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

ENH: Add function to visualize presentation #380

Merged
merged 6 commits into from
Feb 7, 2021

Conversation

olynch
Copy link
Contributor

@olynch olynch commented Jan 29, 2021

This is just a simple function to draw the generating morphisms in a presentation in graph form.

Questions:

  1. Should this go in another module?
  2. How does one test something like this, especially given that the output may change as we decide to make the style different?
  3. How should data objects look different? Should they be in red, or should they have a square around them?

I will clean up code and add docstrings, but also I would like answers to those questions. Also, I just realized that I need to specialize this to Presentation{Schema}.

The generated graph for Graph looks like this:
image

@epatters
Copy link
Member

epatters commented Jan 29, 2021

Thanks, this will be great to have.

It should probably go in Catlab.Graphics.GraphivzGraphs with names like

to_graphviz_graph(pres::Presentation) = [your code]
to_graphviz(pres::Presentation) = to_graphviz(to_graphviz_graph(pres))

Then users can just call the generic function to_graphviz on presentations to get a quick visualization.

@olynch
Copy link
Contributor Author

olynch commented Jan 29, 2021

This is really handy! These diagrams are so much easier to understand than the presentation!

image

Is there graphviz layout wizardry that I can do to make these have a better layout?

@olynch
Copy link
Contributor Author

olynch commented Jan 30, 2021

This looks better, but is it possible to make the edge labels centered on the edges?
image

@olynch
Copy link
Contributor Author

olynch commented Jan 30, 2021

Ah, maybe the way to do this is by taking the layout from Graphviz, and then outputting the actual figure using Compose or TikZ. Is code for doing stuff like this already in Catlab?

The real cool way of doing this would be for Catlab to output a graph that could be opened by some program and then the layout could be specified manually by dragging and dropping.

@olynch
Copy link
Contributor Author

olynch commented Feb 2, 2021

OK, I think I want to merge this at the current level of functionality, so I'm requesting a review.

@olynch olynch requested a review from epatters February 2, 2021 02:37
Copy link
Member

@epatters epatters left a comment

Choose a reason for hiding this comment

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

I agree, this will be very handy! I left one inline comment.

Also, this function should have a test of some kind. The minimal effort approach is to write a unit test that checks some basic invariant of the Graphviz graph. Another nice thing (not required) to do for graphics stuff is to create a Literate.jl vignette in the docs. For example, see this vignette for the Graphviz wiring diagrams. It doubles as a "visual test" that will also break the build if the code errors. It shouldn't be too much work to just copy the examples you've been playing with.

Re: the possibility of other graphics backends, Catlab actually does support parsing graph layouts from the JSON output of Graphviz; see parse_graphviz. You could pursue this in a follow-up PR if you get the itch.

src/core/Present.jl Outdated Show resolved Hide resolved
@jpfairbanks
Copy link
Member

I agree that Evan's inline comment and then LGTM

@epatters
Copy link
Member

epatters commented Feb 4, 2021

To be clear, I am going to insist on some form of testing, however minimal :)

@epatters
Copy link
Member

epatters commented Feb 7, 2021

Thanks Owen! Merging now.

@epatters epatters merged commit f21c159 into AlgebraicJulia:master Feb 7, 2021
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.

None yet

3 participants