UniStuttgart-IKR / AttributeGraphs.jl

MIT License
6 stars 1 forks source link

Precisions on MetaGraphsNext #3

Open gdalle opened 1 year ago

gdalle commented 1 year ago

Hi @filchristou, congrats on the release!

I wanted to give a few details regarding MetaGraphsNext, to help users make informed choices:

Do you think you could update the README to reflect this?

filchristou commented 1 year ago

Hi, thanks for the issue!

1.

In the latest versions, our wrapper is compatible with any AbstractGraph, not just Simple(Di)Graphs

Yes, the docs only mention how MetaGraphs.jl is restricted to Simple(Di)Graph, not MetaGraphsNext.

2.

It is true that labels must be defined, but in the latest version they can be identical to vertex indices

Taken from MetaGraphs docs:

If you don't care about labels at all, using the integer vertex codes as labels may be reasonable. Just keep in mind that labels do not change with vertex deletion, whereas vertex codes get decreased, so the coherence will be broken.

I wouldn't call this support if it can break so easily, except if the docs are outdated ? Also, I guess the user will initially need to provide an integer label for each node matching the Graphs.jl index, which is bothersome.

3.

The properties do not need to be dictionaries,...

What I am referring here is that the underlying structures for vertex and edge properties are Dictionaries, and that is hardcoded (see types for edge_data and vertex_properties): https://github.com/JuliaGraphs/MetaGraphsNext.jl/blob/063399bd04a41c62e796089e0179576628363847/src/metagraph.jl#L27 I guess my text could be misinterpreted, so I will make sure to reflect what I mentioned here.

4.

It is true that it does not play nicely with multigraphs, but we would like it to! The Graphs.jl ecosystem doesn't explicitly support multigraphs either, so this is an ambitous and rather long-term project

Yes, having multigraphs support was the main reason that made me write this package. Since edge_data dictionary keys in MetaGraphsNext are Tuple{Label,Label}, there is no way to support multigraphs, so this was the dealbreaker. The other reasons were:

Frankly, I kinda hate it that I had to write yet another metagraphs package. I really believe that writing few, strong, well supported packages is better for the julia community than having many scattered niche packages. However, I guess some diversity is unavoidable and also good. I hope that eventually many of these packages will be aligned or "merged" to a single OP (overpowered) package.

gdalle commented 1 year ago

Yes, the docs only mention how MetaGraphs.jl is restricted to Simple(Di)Graph, not MetaGraphsNext.

I actually only saw the README, that's what this issue is based on

gdalle commented 1 year ago

I wouldn't call this support if it can break so easily, except if the docs are outdated ? Also, I guess the user will initially need to provide an integer label for each node matching the Graphs.jl index, which is bothersome.

That's a good point. Maybe there could be a subtype of MetaGraph where we don't care about labels and they get updated automatically after vertex deletion?

gdalle commented 1 year ago

What I am referring here is that the underlying structures for vertex and edge properties are Dictionaries, and that is hardcoded (see types for edge_data and vertex_properties):

Right, I hadn't looked at your code, but I do see what you mean! I like your design, I guess theoretically we could include it as an abstract interface for MetaGraph. Or equivalently, include MetaGraph as a more concrete subtype.

gdalle commented 1 year ago

no need for vertex labels, i.e. a more Graphs.jl feeling I think the need for labels can really be alleviated in MetaGraphsNext. If you would like that we could start a discussion there.

Honestly I kinda agree with that, which is also why I started MetaDataGraphs.jl (see #4). There is a bit of a discussion here: https://github.com/JuliaGraphs/MetaGraphsNext.jl/issues/44 Labels give the library a bit of a networkX feeling, and they are useful in lots of situations, but they have their drawbacks. I think the suggestion I made above could be a good middle ground (MetaGraph subtype with automatic labels)?

gdalle commented 1 year ago

Frankly, I kinda hate it that I had to write yet another metagraphs package. I really believe that writing few, strong, well supported packages is better for the julia community than having many scattered niche packages. However, I guess some diversity is unavoidable and also good. I hope that eventually many of these packages will be aligned or "merged" to a single OP (overpowered) package.

Couldn't agree more. I think MetaGraphsNext.jl is well-placed to achieve that, but since we haven't tagged v1.0 there is still a bit of freedom on interface design and internals. I'd love to draw inspiration from your package and improve ours with your help :)

filchristou commented 1 year ago

I'd love to draw inspiration from your package and improve ours with your help :)

sure :)

I made an issue on https://github.com/JuliaGraphs/MetaGraphsNext.jl/issues/59 to continue the discussion for the labels there !