From 33979382338e323ae7fddd1de1a9b44e717817da Mon Sep 17 00:00:00 2001 From: Matthew Fishman Date: Fri, 28 Aug 2026 20:13:42 -0400 Subject: [PATCH 1/2] Give all_edges an iterator so its eltype and length survive Gives `all_edges` on an undirected graph a lazy iterator instead of a flattened lazy `map`, so its element type and length survive. The flatten inferred `eltype` as `Any` and threw on `length`, which broke callers dispatching on the element type. --- src/graphsextensions/abstractgraph.jl | 2 +- src/indicesviews.jl | 32 +++++++++++++++++++++++++++ test/test_graphsextensions.jl | 7 ++++++ test/test_namedgraph.jl | 11 ++++++++- 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/graphsextensions/abstractgraph.jl b/src/graphsextensions/abstractgraph.jl index e28e929..f240734 100644 --- a/src/graphsextensions/abstractgraph.jl +++ b/src/graphsextensions/abstractgraph.jl @@ -165,7 +165,7 @@ end end @traitfn function all_edges(g::::(!IsDirected)) - return Iterators.flatten(Iterators.map(e -> (e, reverse(e)), edges(g))) + return NamedAllEdgeIter(g) end # Alternative syntax to `getindex` for getting a subgraph diff --git a/src/indicesviews.jl b/src/indicesviews.jl index 4c7b0e7..5c4d856 100644 --- a/src/indicesviews.jl +++ b/src/indicesviews.jl @@ -66,3 +66,35 @@ 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 <: AbstractEdge{V}, G <: AbstractGraph{V}} <: AbstractEdgeIter + graph::G +end +function NamedAllEdgeIter(graph::AbstractGraph) + 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)) diff --git a/test/test_graphsextensions.jl b/test/test_graphsextensions.jl index 38f7cc3..9bf565d 100644 --- a/test/test_graphsextensions.jl +++ b/test/test_graphsextensions.jl @@ -143,8 +143,15 @@ using Test: @test, @test_broken, @test_throws, @testset @test issetequal( all_edges(g), edgetype(g).([1 => 2, 2 => 1, 2 => 3, 3 => 2, 3 => 4, 4 => 3]) ) + # 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 = 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) diff --git a/test/test_namedgraph.jl b/test/test_namedgraph.jl index 6ea1115..878de1e 100644 --- a/test/test_namedgraph.jl +++ b/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") From d534bd7d94ec676e6a8303b790266bbf494476fe Mon Sep 17 00:00:00 2001 From: Matthew Fishman Date: Fri, 28 Aug 2026 20:45:25 -0400 Subject: [PATCH 2/2] Narrow all_edges to named graphs and export it `all_edges` never needed a generic graph, so it moves to `NamedGraphs` proper alongside the other named-graph functions, and both edge iterators tighten to named types. Two dependent packages call it, so it is documented and exported rather than reached for as an internal. --- docs/src/user_interface/named_graphs.md | 1 + src/NamedGraphs.jl | 2 +- src/abstractnamedgraph.jl | 36 +++++++++++++++++++++++++ src/graphsextensions/abstractgraph.jl | 11 -------- src/indicesviews.jl | 10 ++++--- test/test_exports.jl | 1 + test/test_graphsextensions.jl | 4 +-- 7 files changed, 47 insertions(+), 18 deletions(-) diff --git a/docs/src/user_interface/named_graphs.md b/docs/src/user_interface/named_graphs.md index 1d92dc5..39147b9 100644 --- a/docs/src/user_interface/named_graphs.md +++ b/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 diff --git a/src/NamedGraphs.jl b/src/NamedGraphs.jl index cd26b5c..9ad7afd 100644 --- a/src/NamedGraphs.jl +++ b/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, diff --git a/src/abstractnamedgraph.jl b/src/abstractnamedgraph.jl index 53a2a96..9e02b02 100644 --- a/src/abstractnamedgraph.jl +++ b/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) diff --git a/src/graphsextensions/abstractgraph.jl b/src/graphsextensions/abstractgraph.jl index f240734..5dd8719 100644 --- a/src/graphsextensions/abstractgraph.jl +++ b/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 NamedAllEdgeIter(g) -end - # Alternative syntax to `getindex` for getting a subgraph # TODO: Should this preserve vertex names by # converting to `NamedGraph` if indexed by diff --git a/src/indicesviews.jl b/src/indicesviews.jl index 5c4d856..eb63352 100644 --- a/src/indicesviews.jl +++ b/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 @@ -71,10 +72,11 @@ Base.show(io::IO, es::NamedEdgeIter) = show(io, collect(es)) # 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 <: AbstractEdge{V}, G <: AbstractGraph{V}} <: AbstractEdgeIter +struct NamedAllEdgeIter{V, E <: AbstractNamedEdge{V}, G <: AbstractNamedGraph{V}} <: + AbstractEdgeIter graph::G end -function NamedAllEdgeIter(graph::AbstractGraph) +function NamedAllEdgeIter(graph::AbstractNamedGraph) return NamedAllEdgeIter{vertextype(graph), edgetype(graph), typeof(graph)}(graph) end diff --git a/test/test_exports.jl b/test/test_exports.jl index 3b9cdca..ffa1bbd 100644 --- a/test/test_exports.jl +++ b/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, diff --git a/test/test_graphsextensions.jl b/test/test_graphsextensions.jl index 9bf565d..ba6c732 100644 --- a/test/test_graphsextensions.jl +++ b/test/test_graphsextensions.jl @@ -139,7 +139,7 @@ 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]) ) @@ -148,7 +148,7 @@ using Test: @test, @test_broken, @test_throws, @testset @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 = path_digraph(4) + 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)