Fix infinite method call in forest_cover_edge_sequence; add tests.

Project.toml

@@ -1,6 +1,6 @@
 name = "NamedGraphs"
 uuid = "678767b0-92e7-4007-89e4-4527a8725b19"
-version = "0.11.3"
+version = "0.11.4"
 authors = ["Matthew Fishman <mfishman@flatironinstitute.org>, Joseph Tindall <jtindall@flatironinstitute.org> and contributors"]
 
 [workspace]

src/NamedGraphs.jl

@@ -10,6 +10,7 @@ include("abstractnamededge.jl")
 include("namededge.jl")
 include("abstractnamedgraph.jl")
 include("abstractgraphindices.jl")
+include("similar_graph.jl")
 include("decorate.jl")
 include("simplecycles.jl")
 include("shortestpaths.jl")

src/abstractnamedgraph.jl

@@ -1,13 +1,12 @@
-using .GraphsExtensions: GraphsExtensions, all_edges, directed_graph, incident_edges,
-    partition_vertices, rem_edges, rem_edges!, rem_vertices, rename_vertices,
-    similar_simplegraph, subgraph
+using .GraphsExtensions: GraphsExtensions, all_edges, directed_graph, empty_graph,
+    incident_edges, partition_vertices, rem_edges, rem_edges!, rem_vertices,
+    rename_vertices, similar_graph, subgraph
 using Dictionaries: set!
-using Graphs.SimpleGraphs: SimpleDiGraph, SimpleEdge
-using Graphs: Graphs, AbstractGraph, AbstractSimpleGraph, IsDirected, a_star, add_edge!,
-    adjacency_matrix, bfs_parents, boruvka_mst, connected_components, degree, edges,
-    has_path, indegree, induced_subgraph, inneighbors, is_connected, is_cyclic, kruskal_mst,
-    ne, neighborhood, neighborhood_dists, nv, outdegree, prim_mst, rem_edge!,
-    spfa_shortest_paths, vertices, weights
+using Graphs: Graphs, AbstractGraph, AbstractSimpleGraph, IsDirected, SimpleDiGraph,
+    SimpleEdge, SimpleGraph, a_star, add_edge!, adjacency_matrix, bfs_parents, boruvka_mst,
+    connected_components, degree, edges, has_path, indegree, induced_subgraph, inneighbors,
+    is_connected, is_cyclic, kruskal_mst, ne, neighborhood, neighborhood_dists, nv,
+    outdegree, prim_mst, rem_edge!, spfa_shortest_paths, vertices, weights
 using SimpleTraits: SimpleTraits, @traitfn, Not
 
 abstract type AbstractNamedGraph{V} <: AbstractGraph{V} end
@@ -51,42 +50,6 @@ Graphs.edgetype(::Type{<:AbstractNamedGraph}) = not_implemented()
 GraphsExtensions.convert_vertextype(::Type{V}, g::AbstractNamedGraph{V}) where {V} = g
 GraphsExtensions.convert_vertextype(::Type, g::AbstractNamedGraph) = not_implemented()
 
-function similar_graph(graph::AbstractGraph)
-    newgraph = similar_graph(graph, vertices(graph))
-    add_edges!(newgraph, edges(graph))
-    return newgraph
-end
-
-# Construct `GenericNamedGraph` as a fallback.
-@traitfn function similar_graph(
-        graph::AbstractGraph::(!IsDirected),
-        vertices
-    )
-    V = eltype(vertices)
-    return NamedGraph{V}(vertices)
-end
-@traitfn function similar_graph(
-        graph::AbstractGraph::IsDirected,
-        vertices
-    )
-    V = eltype(vertices)
-    return NamedDiGraph{V}(vertices)
-end
-
-# Passing a type as a first argument attempts to call a constructor. Should be overloaded
-# if the constructor doesnt exist for a given `AbstractGraph` concrete type.
-similar_graph(T::Type{<:AbstractGraph}) = similar_graph(T, vertextype(T)[])
-similar_graph(T::Type{<:AbstractGraph}, vertices) = T(vertices)
-
-# If `T <: AbstractSimpleGraph`, then we defer to `GraphsExtensions.similar_simplegraph`.
-function similar_graph(T::Type{<:AbstractSimpleGraph}, vertices = 0)
-    return similar_simplegraph(T, vertices)
-end
-
-edgeless_graph(graph::AbstractGraph) = rem_edges(graph, edges(graph))
-
-empty_graph(graph::AbstractGraph) = rem_vertices(graph, vertices(graph))
-
 Base.copy(graph::AbstractNamedGraph) = copyto!(similar_graph(graph), graph)
 
 function Graphs.merge_vertices!(
@@ -585,7 +548,7 @@ end
 
 # TODO: Implement an edgelist version
 function induced_subgraph_from_vertices(graph::AbstractGraph, subvertices)
-    subgraph = similar_graph(graph, collect(subvertices))
+    subgraph = similar_graph(graph, subvertices)
     add_edges!(subgraph, subgraph_edges(graph, subvertices))
     return subgraph, nothing
 end
@@ -613,23 +576,3 @@ function edge_subgraph_namedgraph(graph, edgelist)
     g = rem_edges!(g, setdiff(edges(g), edgelist))
     return g
 end
-
-@traitfn function GraphsExtensions.directed_graph(graph::AbstractNamedGraph::(!IsDirected))
-    digraph = similar_graph(directed_graph_type(graph), vertices(graph))
-    add_edges!(digraph, all_edges(graph))
-    return digraph
-end
-
-@traitfn function GraphsExtensions.undirected_graph(graph::AbstractNamedGraph::IsDirected)
-    undigraph = similar_graph(undirected_graph_type(graph), vertices(graph))
-    for e in edges(graph)
-        has_edge(undigraph, e) && continue
-        add_edge!(undigraph, e)
-    end
-    return undigraph
-end
-
-function GraphsExtensions.forest_cover_edge_sequence(graph::AbstractNamedGraph; kwargs...)
-    dummy_graph = add_edges!(NamedGraph(vertices(graph)), edges(graph))
-    return GraphsExtensions.forest_cover_edge_sequence(dummy_graph; kwargs...)
-end

src/lib/GraphsExtensions/src/abstractgraph.jl

@@ -26,17 +26,26 @@ convert_vertextype(::Type, ::AbstractGraph) = not_implemented()
 convert_vertextype(::Type{V}, G::Type{<:AbstractGraph{V}}) where {V} = G
 convert_vertextype(::Type, ::Type{<:AbstractGraph}) = not_implemented()
 
-# ==================================== similar_simplegraph =============================== #
+# ==================================== similar_graph ===================================== #
 
-function similar_simplegraph(graph::AbstractGraph)
-    newgraph = similar_simplegraph(graph, vertices(graph))
+# Generic for any `AbstractGraph`.
+function similar_graph(graph::AbstractGraph)
+    newgraph = similar_graph(graph, vertices(graph))
     add_edges!(newgraph, edges(graph))
     return newgraph
 end
 
+similar_graph(graph::AbstractGraph, vertices) = similar_simplegraph(graph, vertices)
+
+# Don't know a sensible fallback in `Graphs.jl` for a non-`Base.OneTo` vertex collection.
+function similar_simplegraph(graph::AbstractGraph, vertices)
+    return throw(MethodError(similar_graph, (typeof(graph), typeof(vertices))))
+end
+
 function similar_simplegraph(graph::AbstractGraph, vertices::Base.OneTo)
-    return similar_simplegraph(graph, length(vertices))
+    return similar_graph(graph, length(vertices))
 end
+
 # To be specialized (optional, has following fallback)
 @traitfn function similar_simplegraph(
         graph::AbstractGraph::(!IsDirected),
@@ -55,16 +64,61 @@ end
     return new_graph
 end
 
-similar_simplegraph(T::Type{<:AbstractSimpleGraph}) = T()
-similar_simplegraph(T::Type{<:AbstractSimpleGraph}, vertices) = T(vertices)
-function similar_simplegraph(T::Type{<:AbstractSimpleGraph}, vertices::Base.OneTo)
-    return similar_simplegraph(T, length(vertices))
+# Type domain versions attempt to call a constructor.
+similar_graph(T::Type{<:AbstractGraph}) = T()
+similar_graph(T::Type{<:AbstractGraph}, vertices) = T(vertices)
+function similar_graph(T::Type{<:AbstractSimpleGraph}, vertices::Base.OneTo)
+    return similar_graph(T, length(vertices))
+end
+similar_graph(T::Type{<:AbstractSimpleGraph}) = similar_graph(T, 0)
+
+# =============================== `similar_dataless_grap` ================================ #
+# This function behaves much the same as `similar_graph`, but should strictly return a
+# a similar graph type that has no notion of data (in the abstract sense).
+
+function similar_dataless_graph(graph::AbstractGraph)
+    dataless_graph = similar_dataless_graph(graph, vertices(graph))
+    add_edges!(dataless_graph, edges(graph))
+    return dataless_graph
+end
+
+function similar_dataless_graph(graph::AbstractGraph, vertices)
+    return similar_dataless_simplegraph(graph, vertices)
+end
+
+function similar_dataless_simplegraph(graph::AbstractGraph, vertices::Base.OneTo)
+    return similar_dataless_simplegraph(graph, length(vertices))
+end
+
+# Fallbacks
+@traitfn function similar_dataless_simplegraph(graph::AbstractGraph::!(IsDirected), nv::Int)
+    return SimpleGraph(nv)
+end
+
+@traitfn function similar_dataless_simplegraph(graph::AbstractGraph::IsDirected, nv::Int)
+    return SimpleDiGraph(nv)
 end
-similar_simplegraph(T::Type{<:AbstractSimpleGraph}, nvertices::Int) = T(nvertices)
+
+edgeless_graph(graph::AbstractGraph) = rem_edges(graph, edges(graph))
+empty_graph(graph::AbstractGraph) = rem_vertices(graph, vertices(graph))
 
 @traitfn directed_graph(graph::::IsDirected) = graph
 @traitfn undirected_graph(graph::::(!IsDirected)) = graph
 
+@traitfn function directed_graph(graph::AbstractGraph::(!IsDirected))
+    digraph = similar_graph(directed_graph_type(graph), vertices(graph))
+    add_edges!(digraph, all_edges(graph))
+    return digraph
+end
+@traitfn function undirected_graph(graph::AbstractGraph::IsDirected)
+    undigraph = similar_graph(undirected_graph_type(graph), vertices(graph))
+    for e in edges(graph)
+        has_edge(undigraph, e) && continue
+        add_edge!(undigraph, e)
+    end
+    return undigraph
+end
+
 # Similar to `eltype`, but `eltype` doesn't work on types
 vertextype(::Type{<:AbstractGraph{V}}) where {V} = V
 vertextype(graph::AbstractGraph) = vertextype(typeof(graph))

src/lib/GraphsExtensions/src/simplegraph.jl

@@ -20,28 +20,7 @@ function convert_vertextype(vertextype::Type, graph::SimpleDiGraph)
 end
 
 directed_graph_type(G::Type{<:SimpleGraph}) = SimpleDiGraph{vertextype(G)}
-# TODO: Use traits to make this more general.
 undirected_graph_type(G::Type{<:SimpleGraph}) = G
 
-# TODO: Use traits to make this more general.
 directed_graph_type(G::Type{<:SimpleDiGraph}) = G
 undirected_graph_type(G::Type{<:SimpleDiGraph}) = SimpleGraph{vertextype(G)}
-
-@traitfn function directed_graph(graph::AbstractSimpleGraph::(!IsDirected))
-    digraph = similar_simplegraph(directed_graph_type(graph), vertices(graph))
-    add_edges!(digraph, all_edges(graph))
-    return digraph
-end
-
-# Must have the same argument name as:
-# @traitfn undirected_graph(graph::::(!IsDirected))
-# to avoid method overwrite warnings, see:
-# https://github.com/mauro3/SimpleTraits.jl#method-overwritten-warnings
-@traitfn function undirected_graph(graph::AbstractSimpleGraph::IsDirected)
-    undigraph = similar_simplegraph(undirected_graph_type(graph), vertices(graph))
-    for e in edges(graph)
-        has_edge(undigraph, e) && continue
-        add_edge!(undigraph, e)
-    end
-    return undigraph
-end

src/lib/GraphsExtensions/src/trees_and_forests.jl

@@ -1,5 +1,5 @@
 using .GraphsExtensions: random_bfs_tree, rem_edges, undirected_graph
-using Graphs: IsDirected, bfs_tree, connected_components, edges, edgetype
+using Graphs: AbstractGraph, IsDirected, bfs_tree, connected_components, edges, edgetype
 using SimpleTraits: SimpleTraits, @traitfn, Not
 
 abstract type SpanningTreeAlgorithm end
@@ -47,9 +47,12 @@ end
 # Given an undirected graph g with vertex set V, build a set of forests (each with vertex set V) which covers all edges in g
 # (see https://en.wikipedia.org/wiki/Arboricity) We do not find the minimum but our tests show this algorithm performs well
 function forest_cover(g::AbstractGraph; spanning_tree = spanning_tree)
-    edges_collected = edgetype(g)[]
+    g = similar_dataless_graph(g)
+    g_reduced = g
+
     remaining_edges = edges(g)
-    g_reduced = rem_edges(g, edges_collected)
+    edges_collected = empty(remaining_edges)
+
     forests = typeof(g)[]
     while !isempty(remaining_edges)
         g_reduced_spanning_forest = spanning_forest(g_reduced; spanning_tree)

src/lib/GraphsExtensions/test/runtests.jl

@@ -9,14 +9,14 @@ using NamedGraphs.GraphGenerators: binary_arborescence
 using NamedGraphs.GraphsExtensions: TreeGraph, add_edge, add_edges, add_edges!,
     add_vertices!, all_edges, arrange_edge, arranged_edges, child_edges, child_vertices,
     convert_vertextype, degrees, directed_graph, directed_graph_type, disjoint_union,
-    distance_to_leaves, has_edges, has_leaf_neighbor, has_vertices, incident_edges,
-    indegrees, is_arborescence, is_arranged, is_binary_arborescence, is_cycle_graph,
-    is_ditree, is_edge_arranged, is_leaf_edge, is_leaf_vertex, is_path_graph,
-    is_root_vertex, is_rooted, is_self_loop, leaf_vertices, minimum_distance_to_leaves,
-    next_nearest_neighbors, non_leaf_edges, outdegrees, permute_vertices, rem_edge,
-    rem_edges, rem_edges!, rename_vertices, root_vertex, similar_simplegraph, subgraph,
-    tree_graph_node, undirected_graph, undirected_graph_type, vertextype,
-    vertices_at_distance, ⊔
+    distance_to_leaves, forest_cover_edge_sequence, has_edges, has_leaf_neighbor,
+    has_vertices, incident_edges, indegrees, is_arborescence, is_arranged,
+    is_binary_arborescence, is_cycle_graph, is_ditree, is_edge_arranged, is_leaf_edge,
+    is_leaf_vertex, is_path_graph, is_root_vertex, is_rooted, is_self_loop, leaf_vertices,
+    minimum_distance_to_leaves, next_nearest_neighbors, non_leaf_edges, outdegrees,
+    permute_vertices, rem_edge, rem_edges, rem_edges!, rename_vertices, root_vertex,
+    similar_dataless_graph, similar_graph, subgraph, tree_graph_node, undirected_graph,
+    undirected_graph_type, vertextype, vertices_at_distance, ⊔
 using NamedGraphs: NamedDiGraph, NamedEdge, NamedGraph
 using Test: @test, @test_broken, @test_throws, @testset
 
@@ -482,20 +482,31 @@ using Test: @test, @test_broken, @test_throws, @testset
     @test_throws ErrorException root_vertex(g)
     @test_throws MethodError root_vertex(binary_tree(3))
 
-    # similar_simplegraph
+    # similar_graph
     g = path_graph(4)
 
-    @test similar_simplegraph(g) isa typeof(g)
-    @test similar_simplegraph(g) == g
-    @test similar_simplegraph(typeof(g)) isa typeof(g)
-    @test similar_simplegraph(typeof(g)) == typeof(g)()
-    @test isempty(edges(similar_simplegraph(typeof(g))))
-    @test isempty(vertices(similar_simplegraph(typeof(g))))
+    @test similar_graph(g) isa typeof(g)
+    @test similar_graph(g) == g
+    @test similar_graph(typeof(g)) isa typeof(g)
+    @test similar_graph(typeof(g)) == typeof(g)()
+    @test isempty(edges(similar_graph(typeof(g))))
+    @test isempty(vertices(similar_graph(typeof(g))))
 
-    @test similar_simplegraph(g, vertices(g)) == typeof(g)(4)
-    @test similar_simplegraph(typeof(g), vertices(g)) == typeof(g)(4)
-    @test isempty(edges(similar_simplegraph(g, vertices(g))))
-    @test isempty(edges(similar_simplegraph(typeof(g), vertices(g))))
+    @test similar_graph(g, vertices(g)) == typeof(g)(4)
+    @test similar_graph(typeof(g), vertices(g)) == typeof(g)(4)
+    @test isempty(edges(similar_graph(g, vertices(g))))
+    @test isempty(edges(similar_graph(typeof(g), vertices(g))))
+
+    # similar_dataless_graph
+    g = path_graph(4)
+
+    @test similar_dataless_graph(g) isa SimpleGraph
+    @test similar_dataless_graph(SimpleDiGraph(4)) isa SimpleDiGraph
+    @test similar_dataless_graph(g) == g
+
+    @test similar_dataless_graph(SimpleDiGraph(4), 2) isa SimpleDiGraph
+    @test similar_dataless_graph(g, vertices(g)) == typeof(g)(4)
+    @test isempty(edges(similar_dataless_graph(g, vertices(g))))
 
     # add_edge
     g = SimpleGraph(4)
@@ -539,6 +550,11 @@ using Test: @test, @test_broken, @test_throws, @testset
     @test only(next_nearest_neighbors(g, 1)) == 3
     @test issetequal(vertices_at_distance(g, 5, 3), [2, 8])
 
+    g = NamedGraph(path_graph(2))
+    @test forest_cover_edge_sequence(g; root_vertex = (g) -> 1) ==
+        [NamedEdge(2, 1), NamedEdge(1, 2)]
+    @test forest_cover_edge_sequence(g; root_vertex = (g) -> 2) ==
+        [NamedEdge(1, 2), NamedEdge(2, 1)]
     @testset "arrange" begin
         @testset "is_arranged, is_edge_arranged" begin
             for (a, b) in [

src/lib/PartitionedGraphs/src/partitionedgraph.jl

@@ -122,9 +122,7 @@ function Graphs.rem_vertex!(pg::PartitionedGraph{V}, vertex::V) where {V}
     rem_vertex!(pg.graph, vertex)
 
     # If the super-vertex is now empty, remove it from the quotient graph
-    if !haskey(pg.partitioned_vertices, qv)
-        rem_vertex!(pg.quotient_graph, qv)
-    end
+    qv ∈ keys(pg.partitioned_vertices) || rem_vertex!(pg.quotient_graph, qv)
 
     return pg
 end

src/lib/PartitionedGraphs/src/quotientvertex.jl

@@ -79,7 +79,9 @@ function Graphs.vertices(g::AbstractGraph, quotientvertex::QuotientVertex)
     qv = parent(quotientvertex)
 
     pvs = partitioned_vertices(g)
-    haskey(pvs, qv) || throw(ArgumentError("Quotient vertex $quotientvertex not in graph"))
+
+    # Can't use `haskey` since it is not defined on vectors.
+    qv ∈ keys(pvs) || throw(ArgumentError("Quotient vertex $quotientvertex not in graph"))
 
     return pvs[qv]
 end
@@ -88,7 +90,8 @@ function Graphs.vertices(g::AbstractGraph, quotientvertices::QuotientVertices)
 end
 
 function has_quotientvertex(g::AbstractGraph, quotientvertex::QuotientVertex)
-    return haskey(partitioned_vertices(g), parent(quotientvertex))
+    # Can't use `haskey` since it is not defined on vectors.
+    return parent(quotientvertex) ∈ keys(partitioned_vertices(g))
 end
 
 Graphs.nv(g::AbstractGraph, sv::QuotientVertex) = length(vertices(g, sv))