Should graph[label] return the vertex metadata or the integer code?

[gdalle]

• The first option (the current one) makes accessing the data easier
• The second option makes working with Graphs.jl algorithms easier

[filchristou]

and how would you get the vertex metadata ? define a get_prop or something ?

[gdalle]

Yeah that's always the compromise, if you get one fast you get the other one slowly. I would favor get_data if we decide to switch.

I have no strong opinion, I just thought it's a matter worth discussing while we stabilize the API

[filchristou]

so what previously in #39 was done with

getindex.([colors], label_for.([colors], inneighbors(colors, code_for(colors, :blue))))

and I wanted it to be like

colors[inneighbors(colors, :blue)]

with your proposition it will end up looking:

get_data.([colors], label_for.([colors], inneighbors(colors, colors[:blue])))

which is still pretty wordy.
I mean you will have to keep the label_for, right ?
Are or you having any other thoughts ?

[gdalle]

No matter what we do, if we want to use functions from Graphs.jl with labels, it will have to be wordy. I tried to explain it here (#39 (comment)): basically the only alternative would be to reimplement all of Graphs.jl with labels, not juste a few functions like inneighbors

[filchristou]

there is still some space to work before thinking of doing such a mass forwarding which I also find demanding. For example, we could tread getindex(::MetaGraph, ::Integer) differently from getindex(::MetaGraph, ::Label). And if we also implemenent getindex(::MetaGraph, <:Vector{Integer}) something like that will work:

colors[inneighbors(colors, colors[:blue])]

so MetaGraph[::Label] outputs an Integer and MetaGraph[::Integer] outputs the data
ofc as I say here this will require to ban users from using Integer Labels, which I am okey with. Something like UUIDs should be encouraged instead of Integers eitherway.

[gdalle]

For example, we could tread getindex(::MetaGraph, ::Integer) differently from getindex(::MetaGraph, ::Label)

That's an idea I actually like. The only reasonable use of integer labels is to compensate for vertex code updating following deletions or to provide default identifiers (as seen in #37 for instance). Maybe that's worth giving up on for shorter syntax. @bramtayl thoughts?

[bramtayl]

I think I'm ready to fully ban integer labels. Then we can use multiple dispatch to make codes and labels interchangeable, which several people have asked for. We might need to add a note to the documentation that using codes can be more performant in some cases, because it avoids having to look up the label multiple times.

Should graph[label] return the vertex metadata or the integer code?

If we make codes and labels interchangable, then hopefully users won't even have to interact with the codes at all. In that case, I think returning the vertex metadata would be more useful?

We could treat getindex(::MetaGraph, ::Integer) differently from getindex(::MetaGraph, ::Label)

That seems maybe a bit confusing: I'd expect them to return the same thing?

[gdalle]

That seems maybe a bit confusing: I'd expect them to return the same thing?

Then what do you mean by using multiple dispatch on codes and labels?

[bramtayl]

Oh, just so that you could do e.g. inneighbors(graph, 1) or inneighbors(graph, :label1)

[gdalle]

Oh, just so that you could do e.g. inneighbors(graph, 1) or inneighbors(graph, :label1)

The thing is, if we do that, there's no reason to stop there, and we might end up re-implementing all of Graphs.jl with labels instead of codes.
I think the getindex dispatch is elegant, functional (if we ban integer labels) and will substantially reduce code size. But it might be confusing so it needs good docs