Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IO-1496][external] Expose the name, type, and edge_ids of Stage objects #690

Merged
merged 16 commits into from
Oct 19, 2023

Conversation

JBWilkie
Copy link
Collaborator

Problem

Stage objects currently don't expose their name, type, or edge_ids

Solution

Write Stage class properties to expose these fields

Changelog

Added the ability to access the name, type, and edge_ids of a Stage object

@linear
Copy link

linear bot commented Oct 18, 2023

IO-1496 Stage is missing name, type, and edges

The Stage object doesn't expose name, type, or edges members.

Add these, as properties with loading if necessary.

Acceptance criteria

  • Accessing name on Stage returns the name as a string
  • Accessing type on Stage returns the type as an Enum/appropriate type
  • Accessing edges on Stage returns the edges as a List

Copy link
Contributor

@owencjones owencjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not approved yet, but this is good. If you DM me when you've considered all the stuff, then I'll approve.

@property
def edges(self) -> List[List[UUID]]:
"""Edge ID, source stage ID, target stage ID."""
edges = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not obligatory, but you could do this with a list comprehension, which are a bit faster, and worth getting used to.

    return [
        [edge.id, edge.source_stage_id, edge.target_stage_id]
        for edge in in self._element.edges
    ]

Optional, but consider it, and also have a think about listcomps in general.

Copy link
Contributor

@owencjones owencjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after PR changes

@JBWilkie JBWilkie merged commit 1ac5d03 into master Oct 19, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants