sbromberger / LightGraphs.jl

An optimized graphs package for the Julia programming language
Other
673 stars 185 forks source link

Fallback for all edge operations #1287

Open matbesancon opened 4 years ago

matbesancon commented 4 years ago

It would be handy for all edge operations to have both f(g, e) and f(g, src(e), dst(e)) defined by default on abstract graphs

sbromberger commented 4 years ago

(Agreed, but since this currently isn't manifesting as incorrect / unintended behavior, I've removed the "bug" tag.)

sbromberger commented 4 years ago

@matbesancon could you start compiling a list of functions that will require new methods?

matbesancon commented 4 years ago

Indeed the bug tag was my mistake, the github mobile interface is a bit of a pain

matbesancon commented 4 years ago

I've mostly noticed it for add_edge! rem_edge! has_edge

birm commented 4 years ago

Do you have an example of code that demonstrates a case where one of those definitions doesn't work?

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

matbesancon commented 4 years ago

bump

matbesancon commented 4 years ago

(just for stalebot)

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

matbesancon commented 4 years ago

go away bot

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

sbromberger commented 3 years ago

@matbesancon did we ever fix this?

matbesancon commented 3 years ago

No I don't think we did. It is done for has_edge: https://github.com/JuliaGraphs/LightGraphs.jl/blob/stable/src/interface.jl#L277

But it is the only function using edges in the interface. So not for all mutation functions adding and removing edges

gdalle commented 3 years ago

Since the discussion is not closed about this, I have a related issue. Would it be possible to access edges based on src and dst, mainly to get their index in the list of all edges? Does it fit in the same batch of potential changes? Just a newbie speaking, so don't mind me too much

sbromberger commented 3 years ago

There is no Lightgraphs-based graph type that I know of that allows global edge indexing (e.g., get_edge(g, 15) returns whatever you determine to be the "15th" edge in the graph).

You could always make a graph type that does this (just define your data type with methods that allow indexing of the edges, and implement the 10 or so API functions that LightGraphs needs, and everything else should just work).

If you don't care about memory usage and you don't make changes to the graph, a collect(edges(g)) might suffice. But you're storing |2E| memory to do that (but with O(1) lookup, so that might be more important).