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

Export named graphs from Catlab.Graphs #856

Merged
merged 2 commits into from Sep 29, 2023
Merged

Export named graphs from Catlab.Graphs #856

merged 2 commits into from Sep 29, 2023

Conversation

epatters
Copy link
Member

CC @slwu89

@slwu89
Copy link
Member

slwu89 commented Sep 29, 2023

Export all the things! Thanks. After you merge this I'll update my #850 before requesting review.

@epatters
Copy link
Member Author

epatters commented Sep 29, 2023

Now that vertex_name and edge_name are part of the official API, we should update GraphvizGraphs and GraphCategories to use those by default, which will work for named graphs while preserving the old behavior for unnamed graphs.

@epatters epatters merged commit a8e1539 into main Sep 29, 2023
13 checks passed
@epatters epatters deleted the export-named-graphs branch September 29, 2023 00:16
@slwu89
Copy link
Member

slwu89 commented Sep 29, 2023

Ok, happy to do that, as it'll help figure out how to adjust #850.

@slwu89
Copy link
Member

slwu89 commented Sep 29, 2023

@epatters I'm actually not sure what's the best way forward on updating GraphvizGraphs to use vertex_name and edge_name. Currently there's a lot of flexibility, as the argument node_labels can be boolean or a symbol referring to a subpart. For example, if we pass a NamedGraph to to_graphviz we have the option of printing the vertex ID, a subpart associated to vertices, or nothing. It would seem that using vertex_name in place of the method node_label for example, would mean we are forced to use the vname subpart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants