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

Display public models in output of list command #7816

Closed
wants to merge 11 commits into from

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Jun 7, 2023

resolves #7496

Description

Add public models to the graph and the selection code to enable listing public models and using selection syntax for upstream nodes of public nodes.

Note: There's some noise in the selector_methods.py because I couldn't stand the non-standard node/real_node thing and switched it all to unique_id/node, which doesn't totally confuse people.

Checklist

@gshank gshank requested a review from a team as a code owner June 7, 2023 15:37
@gshank gshank requested a review from aranke June 7, 2023 15:37
@cla-bot cla-bot bot added the cla:yes label Jun 7, 2023
@gshank gshank requested review from MichelleArk and removed request for aranke June 7, 2023 15:38
Copy link
Contributor

@peterallenwebb peterallenwebb left a comment

Choose a reason for hiding this comment

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

Should we consider having the PublicModel class inherit from GraphNode? I'm concerned that introducing a "node that isn't really a node" adds more complexity than we will want to manage. On the other hand, maybe the solution is to use the ManifestOrPublicNode protocol more widely, in which case I'd suggest renaming it to something more evocative.

Another lingering question: Aren't all nodes "graph nodes"? If they aren't, what do we mean by node?

I'm going to approve, since I think these changes do what they say on the tin and don't constrain our options going forward, but I'd be interested in your thoughts.

@gshank
Copy link
Contributor Author

gshank commented Jun 21, 2023

We're not doing it this way now.

@gshank gshank closed this Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-2517] Graph selection for public models from dependency projects
3 participants