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

transitiveclosure only works on SimpleDiGraphs #245

Open
gdalle opened this issue Apr 6, 2023 · 4 comments
Open

transitiveclosure only works on SimpleDiGraphs #245

gdalle opened this issue Apr 6, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@gdalle
Copy link
Member

gdalle commented Apr 6, 2023

The guilty lines of code:

function transitiveclosure(g::DiGraph, selflooped=false)
copyg = copy(g)
return transitiveclosure!(copyg, selflooped)
end

Is there a reason why:

  • the function dispatches on a concrete graph type
  • it doesn't support undirected graphs?

First spotted by JuliaGraphs/SimpleWeightedGraphs.jl#10

@jlapeyre
Copy link
Contributor

jlapeyre commented Apr 6, 2023

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
Copy link
Member

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
Copy link
Member Author

gdalle commented Jun 16, 2023

Yes but it could be a directed AbstractGraph at least

@gdalle gdalle added the enhancement New feature or request label Jun 16, 2023
@gdalle
Copy link
Member Author

gdalle commented Sep 14, 2023

Linked to #281

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants