Make all_edges public and give it a real iterator

docs/src/user_interface/named_graphs.md

@@ -27,6 +27,7 @@ section will grow over time.
 ```@docs; canonical=false
 vertices(::AbstractNamedGraph)
 edges(::AbstractNamedGraph)
+all_edges
 neighbors(::AbstractNamedGraph, ::Any)
 Graphs.dijkstra_shortest_paths(::AbstractNamedGraph, ::Any, ::Any)
 subgraph

src/NamedGraphs.jl

@@ -22,7 +22,7 @@ module NamedGraphs
 # exports its own, different, generic functions under those names, so exporting
 # ours would leave both dead after `using Graphs, NamedGraphs`.
 export ⊔, AbstractNamedGraph, NamedDiGraph, NamedEdge, NamedGraph,
-    add_edge, add_edges, add_edges!, add_vertex, add_vertices, boundary_edges,
+    add_edge, add_edges, add_edges!, add_vertex, add_vertices, all_edges, boundary_edges,
     convert_vertextype, default_root_vertex, directed_graph, disjoint_union,
     edge_subgraph, edgeless_graph, empty_graph, forest_cover,
     forest_cover_edge_sequence, in_incident_edges, incident_edges, is_leaf_vertex,

src/abstractnamedgraph.jl

@@ -1348,6 +1348,42 @@ function subgraph_edges(graph::AbstractGraph, subvertices)
     end
 end
 
+"""
+    all_edges(graph::AbstractNamedGraph)
+
+Both directions of each edge of an undirected `graph`, each edge immediately
+followed by its reverse, or just the edges of a directed one. Useful where a
+value belongs to a direction rather than to an edge, such as a message on each
+directed edge.
+
+# Examples
+
+```jldoctest
+julia> using NamedGraphs: all_edges, named_path_digraph, named_path_graph
+
+julia> collect(all_edges(named_path_graph(3)))
+4-element Vector{NamedEdge{Int64}}:
+ 1 => 2
+ 2 => 1
+ 2 => 3
+ 3 => 2
+
+julia> collect(all_edges(named_path_digraph(3)))
+2-element Vector{NamedEdge{Int64}}:
+ 1 => 2
+ 2 => 3
+```
+"""
+function all_edges end
+
+@traitfn function all_edges(g::AbstractNamedGraph::IsDirected)
+    return edges(g)
+end
+
+@traitfn function all_edges(g::AbstractNamedGraph::(!IsDirected))
+    return NamedAllEdgeIter(g)
+end
+
 """
     edge_subgraph(graph::AbstractNamedGraph, edges)
 

src/graphsextensions/abstractgraph.jl

@@ -157,17 +157,6 @@ function _neighbors(graph::AbstractGraph, vertex; dir = :out)
     )
 end
 
-# Returns just the edges of a directed graph,
-# but both edge directions of an undirected graph.
-# TODO: Move to NamedGraphs.jl
-@traitfn function all_edges(g::::IsDirected)
-    return edges(g)
-end
-
-@traitfn function all_edges(g::::(!IsDirected))
-    return Iterators.flatten(Iterators.map(e -> (e, reverse(e)), edges(g)))
-end
-
 # Alternative syntax to `getindex` for getting a subgraph
 # TODO: Should this preserve vertex names by
 # converting to `NamedGraph` if indexed by

src/indicesviews.jl

@@ -42,10 +42,11 @@ Dictionaries.gettokenvalue(vs::NamedVerticesView, token) = decoded_vertex(vs.gra
 # `Graphs.SimpleGraphs.SimpleEdgeIter`: iterates by decoding the edges of
 # `encoded_graph(graph)` and tests membership through `has_edge`.
 # Output of `edges(graph::AbstractNamedGraph)`.
-struct NamedEdgeIter{V, E <: AbstractEdge{V}, G <: AbstractGraph{V}} <: AbstractEdgeIter
+struct NamedEdgeIter{V, E <: AbstractNamedEdge{V}, G <: AbstractNamedGraph{V}} <:
+    AbstractEdgeIter
     graph::G
 end
-function NamedEdgeIter(graph::AbstractGraph)
+function NamedEdgeIter(graph::AbstractNamedGraph)
     return NamedEdgeIter{vertextype(graph), edgetype(graph), typeof(graph)}(graph)
 end
 
@@ -66,3 +67,36 @@ function Base.:(==)(es1::NamedEdgeIter, es2::NamedEdgeIter)
     return all(e -> e in es2, es1)
 end
 Base.show(io::IO, es::NamedEdgeIter) = show(io, collect(es))
+
+# Lazy iterator over both directions of each edge of an undirected named graph.
+# Output of `all_edges(graph)` there. Built as an iterator rather than a
+# `Iterators.flatten` so that `eltype` and `length` survive: flattening drops
+# both, which breaks callers that dispatch on the element type.
+struct NamedAllEdgeIter{V, E <: AbstractNamedEdge{V}, G <: AbstractNamedGraph{V}} <:
+    AbstractEdgeIter
+    graph::G
+end
+function NamedAllEdgeIter(graph::AbstractNamedGraph)
+    return NamedAllEdgeIter{vertextype(graph), edgetype(graph), typeof(graph)}(graph)
+end
+
+Base.eltype(::Type{<:NamedAllEdgeIter{<:Any, E}}) where {E} = E
+Base.length(es::NamedAllEdgeIter) = 2 * ne(es.graph)
+# The state carries the reversed edge still owed for the current forward edge,
+# so each edge is emitted immediately before its reverse.
+function Base.iterate(es::NamedAllEdgeIter)
+    next = iterate(edges(es.graph))
+    isnothing(next) && return nothing
+    edge, edges_state = next
+    return edge, (edges_state, reverse(edge))
+end
+function Base.iterate(es::NamedAllEdgeIter, state)
+    edges_state, owed = state
+    isnothing(owed) || return owed, (edges_state, nothing)
+    next = iterate(edges(es.graph), edges_state)
+    isnothing(next) && return nothing
+    edge, new_state = next
+    return edge, (new_state, reverse(edge))
+end
+Base.in(edge, es::NamedAllEdgeIter) = has_edge(es.graph, edge)
+Base.show(io::IO, es::NamedAllEdgeIter) = show(io, collect(es))

test/test_exports.jl

@@ -14,6 +14,7 @@ using Test: @test, @testset
             :add_edges!,
             :add_vertex,
             :add_vertices,
+            :all_edges,
             :boundary_edges,
             :convert_vertextype,
             :default_root_vertex,

test/test_graphsextensions.jl

@@ -139,12 +139,19 @@ using Test: @test, @test_broken, @test_throws, @testset
     @test !has_edge(g_perm, 3 => 4)
 
     # all_edges
-    g = path_graph(4)
+    g = named_path_graph(4)
     @test issetequal(
         all_edges(g), edgetype(g).([1 => 2, 2 => 1, 2 => 3, 3 => 2, 3 => 4, 4 => 3])
     )
-    g = path_digraph(4)
+    # The undirected branch is an iterator rather than a flatten, so that `eltype`
+    # and `length` survive for callers that dispatch on them.
+    @test eltype(all_edges(g)) === edgetype(g)
+    @test length(all_edges(g)) == 2 * ne(g)
+    @test collect(all_edges(g)) == [e for edge in edges(g) for e in (edge, reverse(edge))]
+    g = named_path_digraph(4)
     @test issetequal(all_edges(g), edgetype(g).([1 => 2, 2 => 3, 3 => 4]))
+    @test eltype(all_edges(g)) === edgetype(g)
+    @test length(all_edges(g)) == ne(g)
 
     # subgraph
     g = subgraph(path_graph(4), 2:4)

test/test_namedgraph.jl

@@ -140,7 +140,16 @@ end
         )
         @test all([e in edges(g) for e in es])
         eg = edge_subgraph(g, es)
-        @test length(edges(eg)) == length(es)
+        # Compare edge sets rather than counts: `has_edge` ignores orientation on
+        # an undirected graph, so a count check passes even if the wrong edge was
+        # removed and another kept.
+        unordered(e) = Set((src(e), dst(e)))
+        @test Set(unordered.(edges(eg))) == Set(unordered.(es))
+        # `subgraph` can store an edge in the opposite orientation to the request,
+        # so requesting one reversed must give the same subgraph.
+        es_reversed = [i == 2 ? reverse(e) : e for (i, e) in enumerate(es)]
+        @test Set(unordered.(edges(edge_subgraph(g, es_reversed)))) ==
+            Set(unordered.(es))
 
         g = NamedGraph(["A", "B", "C", "D", "E"])
         add_edge!(g, "A" => "B")