Broadcasted Base.getindex

[filchristou]

Taking the example in docs

julia> colors = MetaGraph(Graph(), VertexData = String, EdgeData = Symbol, graph_data = "graph_of_colors")
Meta graph based on a SimpleGraph{Int64}(0, Vector{Int64}[]) with vertex labels of type Symbol, vertex metadata of type String, edge metadata of type Symbol, graph metadata given by "graph_of_colors", and default weight 1.0

julia> colors[:red] = "warm";

julia> colors[:yellow] = "warm";

julia> colors[:blue] = "cool";

wouldn't we want to support something like this ?

julia> colors[[:red, :yellow]]
ERROR: KeyError: key [:red, :blue] not found
Stacktrace:
 [1] getindex(h::Dict{Symbol, Tuple{Int64, String}}, key::Vector{Symbol})
   @ Base ./dict.jl:498
 [2] getindex(meta_graph::MetaUndirectedGraph{Int64, Symbol, SimpleGraph{Int64}, String, Symbol, String, MetaGraphsNext.var"#3#5", Float64}, label::Vector{Symbol})
   @ MetaGraphsNext ~/.julia/packages/MetaGraphsNext/lf5y4/src/dict_utils.jl:16
 [3] top-level scope
   @ REPL[15]:1

I was expecting to get the following

julia> getindex.([colors], [:red, :blue])
2-element Vector{String}:
 "warm"
 "cool"

I think the implementation would also be rather easy by dispatching to something like Base.getindex(mg::MetaGraphs{Label}, vec::Vector{Label})

I could eventually file a PR if interested.

[bramtayl]

Hmm, I'm not super convinced we need this. getindex.([colors], [:red, :blue]) works as you pointed out, and I've been basically thinking of these as Dicts, and this doesn't work for a Dict:

julia> x = Dict("a" => 1, "b" => 2)
Dict{String, Int64} with 2 entries:
  "b" => 2
  "a" => 1

julia> x[["a", "b"]]
ERROR: KeyError: key ["a", "b"] not found
Stacktrace:
 [1] getindex(h::Dict{String, Int64}, key::Vector{String})
   @ Base .\dict.jl:498
 [2] top-level scope
   @ REPL[2]:1

Is there a particular use case you had in mind?

[filchristou]

I see.

Do you worry that it will have any unwanted implications ? Having a fully concrete type MetaGraph I think these are minimized.
Actually... now I can imagine an edge cases where the user passes in Label=Any, and then uses a label that is a vector. Although I would argue this would be bad practice, indeed it will not work appropriately.

The motivation behind is that when I use MetaGraphsNext.jl I end up having long (not easy to read) expressions and I wanted to mitigated that.
Most of the times though it's just the conversion between label_for and code_for to blame.

For example all Graphs.jl functions work with T<Integer and to get out data we need Label.
So I end up doing acrobatics like:

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

to just get the data of all the nodes that go into the :blue node.

Ideally this should be just

colors[inneighbors(colors, :blue)]

this demands dispatching Graphs.inneighbors(g::MetaGraphs{Label}, lab::Label) and implementing this issue.

[filchristou]

this demands dispatching Graphs.inneighbors(g::MetaGraphs{Label}, lab::Label) and implementing this issue.

sorry, that's not quite true. This also requires that Base.getindex works differently for Integers.
It's already recommended not to have Integer Label.
If we make that a hard rule, this will work.

[gdalle]

For example all Graphs.jl functions work with T<Integer and to get out data we need Label.

This is actually the reason behind my alternate implementation in MetaDataGraphs.jl: for my personal use, graph[label] should return the integer code and not the metadata, to make integration with Graphs.jl easier. But I agree that returning the metadata also makes sense.

[gdalle]

this demands dispatching Graphs.inneighbors(g::MetaGraphs{Label}, lab::Label) and implementing this issue.

Unfortunately, even if we extend inneighbors this way, we're just kicking the can further down the hill. For instance, if you want to apply Dijkstra's algorithm, or any other function from Graphs.jl that is not exactly inneighbors, you will still need the conversion to a code. So it's not enough to overload the Graphs.jl interface with labels, we would need to overload the whole library. As long as Graphs.jl relies on integer vertices (which may well be forever), it doesn't make much sense to me.

[gdalle]

Hmm, I'm not super convinced we need this. getindex.([colors], [:red, :blue]) works as you pointed out, and I've been basically thinking of these as Dicts, and this doesn't work for a Dict

Agreed, and I think the discussion above shows that the real issue is not the dict syntax but the interactions between labels, code and data

[gdalle]

For example all Graphs.jl functions work with T<Integer and to get out data we need Label.

This is actually the reason behind my alternate implementation in MetaDataGraphs.jl: for my personal use, graph[label] should return the integer code and not the metadata, to make integration with Graphs.jl easier. But I agree that returning the metadata also makes sense.

Moving this specific discussion to #44

[gdalle]

Partially improved by #50, and the discussion continues in #44, so I'm closing this specific issue since we won't override the dict behavior