AlgebraicJulia / Catlab.jl

A framework for applied category theory in the Julia language
https://www.algebraicjulia.org
MIT License
599 stars 56 forks source link

Better graphviz support for bipartite graphs #888

Open slwu89 opened 4 months ago

slwu89 commented 4 months ago

Hi @epatters, following up here from the zulip conversation. Before working on PropertyBipartiteGraph and to_graphviz for it, I wanted to ask if you can help explain the design considerations behind the existing PropertyGraph machinery and its interaction with the other graph acsets/to_graphviz system, so that I can replicate as much as possible those designs for the bipartite one. And if there are any essential desiderata for the bipartite one. Thanks!

slwu89 commented 4 months ago

@epatters while looking at this I noticed an apparent oddity, in the BipartiteGraphs module the methods to remove vertices and edges are typed for AbstractBipartiteGraph as opposed to the rest of the getters/setters which are typed for HasBipartiteGraph, see https://github.com/AlgebraicJulia/Catlab.jl/blob/main/src/graphs/BipartiteGraphs.jl#L254-L288

This is relevant because in my current design I follow the version for basic graph property graph. That is, the type of the acset graph in the struct PropertyGraph should be a subtype of HasGraph. If I do something similar for bipartite graphs and make the type of the bipartite acset in BipartitePropertyGraph be a subtype of HasBipartiteGraph, the methods for remove vertices won't "forward" properly, as the type class is too wide for those ones.

I suspect that the intention was to have those methods in the BipartiteGraphs module actually take HasBipartiteGraph as the type of the first argument, but want to confirm with you.