Improve consistency of graph and dataview indexing with Indices; fix some bugs

Author: jack-dunham

src/dataview.jl

@@ -93,7 +93,7 @@ function Base.setindex!(view::VertexOrEdgeDataView{K, V}, data::V, key::K) where
     setindex!(view.graph, data, key)
     return view
 end
-function Base.setindex!(view::EdgeDataView{<:Any, V}, data::V, key::Pair{V, V}) where {V}
+function Base.setindex!(view::EdgeDataView{<:Any, V}, data::V, key::Pair) where {V}
     setindex!(view, data, to_graph_index(view.graph, key))
     return view
 end
@@ -125,31 +125,38 @@ end
 
 Base.keys(dvs::DataViewSlice) = dvs.inds
 
-Base.getindex(dvs::DataViewSlice, key) = dvs.view[key]
-Base.getindex(view::DataViewSlice{K}, key::K) where {K} = view.view[key]
+Base.getindex(view::DataViewSlice, key) = getindex_dataview(view, key)
+Base.getindex(view::DataViewSlice{K}, key::K) where {K} = getindex_dataview(view, key)
+function getindex_dataview(dvs::DataViewSlice, key)
+    isassigned(dvs, key) || throw(IndexError("Dictionary does not contain index: $key"))
+    return dvs.view[key]
+end
 
-Base.isassigned(view::DataViewSlice, key) = key in keys(view)
 Base.isassigned(view::DataViewSlice{K}, key::K) where {K} = key in keys(view)
+Base.isassigned(view::DataViewSlice, key::Pair) = isassigned(view, to_graph_index(view.view.graph, key))
 
 Base.getindex(view::VertexOrEdgeDataView, keys::Indices) = DataViewSlice(view, keys)
 function Base.getindex(view::EdgeDataView, keys::Indices{<:Pair})
     return DataViewSlice(view, Indices(map(k -> to_graph_index(view.graph, k), collect(keys))))
 end
 
+# For method ambiguity
 function Base.setindex!(view::DataViewSlice{K, V}, data::V, key::K) where {K, V}
-    setindex!(view.view, data, key)
-    return view
+    return setindex!_dataview(view, data, key)
 end
-function Base.setindex!(view::DataViewSlice{<:Any, V}, data::V, key::Pair{V, V}) where {V}
-    setindex!(view, data, to_graph_index(view.view.graph, key))
-    return view
+function Base.setindex!(view::DataViewSlice{<:Any, V}, data::V, key::Pair) where {V}
+    return setindex!_dataview(view, data, key)
 end
 
-Base.axes(view::DataViewSlice) = (Base.OneTo(length(keys(view))),)
+function setindex!_dataview(view::DataViewSlice, data, key)
+    isassigned(view, key) || throw(IndexError("Dictionary does not contain index: $key"))
+    setindex!(view.view, data, key)
+    return view
+end
 
-function Base.copyto!(dest::DataViewSlice, bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}})
-    for (i, key) in enumerate(keys(dest))
-        @inbounds dest[key] = bc[i]
+function Base.copyto!(dest::DataViewSlice, bc::Dictionaries.BroadcastedDictionary)
+    for (key, val) in pairs(bc)
+        dest[key] = val
     end
     return dest
 end

src/indexing.jl

@@ -6,18 +6,9 @@ using NamedGraphs:
     to_edges,
     AbstractGraphIndices
 using NamedGraphs.GraphsExtensions: subgraph
-using Dictionaries: AbstractIndices
+using Dictionaries: AbstractIndices, getindices
 
-struct Keys{I, Indices} <: AbstractIndices{I}
-    parent::Indices
-    Keys(parent::GI) where {GI} = new{eltype(GI), GI}(parent)
-end
-
-Base.iterate(keys::Keys, state...) = iterate(keys.parent, state...)
-Base.length(keys::Keys) = length(keys.parent)
-Base.in(i, keys::Keys) = in(i, keys.parent)
-
-# ====================================== getindex! ======================================= #
+# ====================================== getindex ======================================= #
 
 NamedGraphs.get_graph_index(graph::AbstractDataGraph, index) = get_index_data(graph, index)
 # If unknown, treat like  single vertex
@@ -34,19 +25,6 @@ function _get_index_data(graph::AbstractGraph, edge::AbstractEdge)
     return reverse_data_direction(graph, edge, data)
 end
 
-# Can force data retrivial instead of subgraphing by using `Keys`.
-function NamedGraphs.getindex_namedgraph(graph::AbstractDataGraph, keys::Keys)
-    return get_indices_data(graph, to_graph_index(graph, keys.parent))
-end
-
-function get_indices_data(graph::AbstractGraph, vertices::AbstractVertices)
-    return get_vertices_data(graph, vertices)
-end
-
-function get_indices_data(graph::AbstractGraph, edges::AbstractEdges)
-    return get_edges_data(graph, edges)
-end
-
 # ====================================== isassigned ====================================== #
 
 function Base.isassigned(graph::AbstractDataGraph, index)
@@ -149,3 +127,13 @@ end
 function NamedGraphs.to_graph_index(graph::AbstractGraph, vertex::OrdinalSuffixedInteger)
     return to_graph_index(graph, vertices(graph)[vertex])
 end
+
+# ====================================== getindices ====================================== #
+
+Dictionaries.getindices(graph::AbstractDataGraph, inds::Indices) = map(ind -> graph[ind], inds)
+
+# ========================================= view ========================================= #
+
+Base.view(graph::AbstractDataGraph, inds::Indices) = vertex_data(graph)[inds]
+Base.view(graph::AbstractDataGraph, inds::Indices{<:Pair}) = edge_data(graph)[inds]
+Base.view(graph::AbstractDataGraph, inds::Indices{<:AbstractEdge}) = edge_data(graph)[inds]

test/test_basics.jl

@@ -15,7 +15,8 @@ using Dictionaries:
     Dictionary,
     Indices,
     dictionary,
-    unset!
+    unset!,
+    IndexError
 using Graphs:
     add_edge!,
     a_star,
@@ -542,10 +543,23 @@ using Test: @test, @test_broken, @testset
         @test g["b"] == 4
         @test g["c"] == 4
 
-        vertex_data(g)[Indices(["b", "a"])] .= [5, 6]
-        @test g["a"] == 6
-        @test g["b"] == 5
-        @test g["c"] == 4
+        vdv = vertex_data(g)[Indices(["a", "b"])]
+
+        @test isassigned(vdv, "a")
+        @test isassigned(vdv, "b")
+        @test !isassigned(vdv, "c")
+        @test_throws IndexError vdv["c"]
+
+        vertex_data(g)[Indices(["b", "a"])] .= Dictionary(["a", "b"], [1, 2])
+
+        @test g["a"] == 1
+        @test g["b"] == 2
+
+        vdv["a"] = 2
+        vdv["b"] = 1
+
+        @test g["a"] == 2
+        @test g["b"] == 1
 
         unset!(g.edge_data, edgetype(g)("b" => "c"))
         @test !isassigned(g, "b" => "c")
@@ -554,9 +568,10 @@ using Test: @test, @test_broken, @testset
         @test g["a" => "b"] == 4.0
         @test g["b" => "c"] == 4.0
 
-        edge_data(g)[Indices(["a" => "b", "b" => "c"])] .= [1.5, 2.5]
-        @test g["a" => "b"] == 1.5
-        @test g["b" => "c"] == 2.5
+        edv = edge_data(g)[Indices(["a" => "b", "b" => "c"])]
 
+        @test isassigned(edv, "a" => "b")
+        edv["a" => "b"] = 5.0
+        @test g["a" => "b"] == 5.0
     end
 end