JuliaGraphs / Graphs.jl

An optimized graphs package for the Julia programming language
http://juliagraphs.org/Graphs.jl/
Other
455 stars 88 forks source link

`transitiveclosure` only works on `SimpleDiGraph`s #245

Open gdalle opened 1 year ago

gdalle commented 1 year ago

The guilty lines of code:

https://github.com/JuliaGraphs/Graphs.jl/blob/53cb58152a19ca62fa01439300948c30035dca6a/src/digraph/transitivity.jl#L105-L108

Is there a reason why:

First spotted by https://github.com/JuliaGraphs/SimpleWeightedGraphs.jl/issues/10

jlapeyre commented 1 year ago

Maybe the only method in the package for transitiveclosure! requires SimpleDiGraph so the author did not want to provide a method that would throw an error when calling the inner function. But Graphs is intended to be extended outside the package. So if someone implements a graph type and a method for transitiveclosure! then they would not get transitiveclosure for free.

simonschoelly commented 1 year ago

That function is kind of similar to other graph operators that we have, such as blockdiag in that they relay on add_edge! to add edges - which is difficult if we need to have some kind of metadata to construct an edge.

That was probably the reason that it was restricted to SimpleDiGraph - or this function was written at a time when there was no abstract graph interface.

Using transitiveclosure on an undirected graph would just convert all connected components into complete graphs - so the usefulness of that case is limited.

gdalle commented 1 year ago

Yes but it could be a directed AbstractGraph at least

gdalle commented 11 months ago

Linked to #281