From b99adc8462ee981199d5f4fd0b73db1ed6e0287b Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Thu, 19 Oct 2023 17:03:34 -0400
Subject: [PATCH 01/14] Added forest cover function

---
 src/traversals/dfs.jl             | 29 +++++++++++++++++++++++++++++
 test/test_forests.jl              | 30 ++++++++++++++++++++++++++++++
 test/test_namedgraphgenerators.jl |  1 -
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 test/test_forests.jl

diff --git a/src/traversals/dfs.jl b/src/traversals/dfs.jl
index 44033fc..5169cf3 100644
--- a/src/traversals/dfs.jl
+++ b/src/traversals/dfs.jl
@@ -25,3 +25,32 @@ end
 function dfs_parents(graph::AbstractNamedGraph, vertex; kwargs...)
   return _dfs_parents(graph, vertex; kwargs...)
 end
+
+#Given a graph, split it into its connected components, construct a spanning tree over each of them
+# and take the union (adding in edges between the trees to recover a connected graph)
+function spanning_forest(g::AbstractNamedGraph)
+  components = connected_components(g)
+  return reduce(
+    union,
+    NamedGraph[
+      undirected_graph(bfs_tree(g[g_comp], first(g_comp))) for g_comp in components
+    ],
+  )
+end
+
+#Given a 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 build_forest_cover(g::AbstractNamedGraph)
+  edges_collected = edgetype(g)[]
+  remaining_edges = edges(g)
+  forests = NamedGraph[]
+  while !isempty(remaining_edges)
+    g_reduced = rem_edges(g, edges_collected)
+    g_reduced_spanning_forest = spanning_forest(g_reduced)
+    push!(edges_collected, edges(g_reduced_spanning_forest)...)
+    push!(forests, g_reduced_spanning_forest)
+    setdiff!(remaining_edges, edges(g_reduced_spanning_forest))
+  end
+
+  return forests
+end
diff --git a/test/test_forests.jl b/test/test_forests.jl
new file mode 100644
index 0000000..9937e40
--- /dev/null
+++ b/test/test_forests.jl
@@ -0,0 +1,30 @@
+using Test
+using Graphs
+using NamedGraphs
+using NamedGraphs:
+  decorate_graph_edges,
+  decorate_graph_vertices,
+  hexagonal_lattice_graph,
+  triangular_lattice_graph,
+  build_forest_cover
+
+@testset "Test Forest Cover" begin
+  gs = [
+    named_grid((6, 1)),
+    named_grid((3, 3, 3)),
+    hexagonal_lattice_graph(6, 6),
+    named_comb_tree((4, 4)),
+    named_grid((10, 10)),
+    triangular_lattice_graph(5, 5; periodic=true),
+  ]
+  for g in gs
+    forest_cover = build_forest_cover(g)
+    cover_edges = reduce(vcat, edges.(forest_cover))
+    @test issetequal(cover_edges, edges(g))
+    for f in forest_cover
+      trees = NamedGraph[f[vs] for vs in connected_components(f)]
+      @test all(is_tree.(trees))
+      @test issetequal(vertices(f), vertices(g))
+    end
+  end
+end
diff --git a/test/test_namedgraphgenerators.jl b/test/test_namedgraphgenerators.jl
index 7eb7813..6b086e7 100644
--- a/test/test_namedgraphgenerators.jl
+++ b/test/test_namedgraphgenerators.jl
@@ -30,7 +30,6 @@ using Random
   @test dims[1] > dims[2]
 
   g = triangular_lattice_graph(2, 1)
-  @show g
   dims = maximum(vertices(g))
   @test dims[1] > dims[2]
 

From 9f4ba2388f3e2033f29bbcf2938851c3b8aef64c Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Fri, 20 Oct 2023 10:57:25 -0400
Subject: [PATCH 02/14] Added option for specifying graph constructor in
 spanning_forest

---
 examples/test.jl      | 14 ++++++++++++++
 src/traversals/dfs.jl | 26 +++++++++++++++-----------
 test/test_forests.jl  |  7 +------
 3 files changed, 30 insertions(+), 17 deletions(-)
 create mode 100644 examples/test.jl

diff --git a/examples/test.jl b/examples/test.jl
new file mode 100644
index 0000000..7b850d7
--- /dev/null
+++ b/examples/test.jl
@@ -0,0 +1,14 @@
+using Test
+using NamedGraphs
+using NamedGraphs:
+  add_edges,
+  add_edges!,
+  rem_edges,
+  rem_edges!,
+  hexagonal_lattice_graph,
+  triangular_lattice_graph
+using Graphs
+
+g = hexagonal_lattice_graph(6, 6; periodic=false)
+
+g = triangular_lattice_graph(7, 7; periodic=true)
diff --git a/src/traversals/dfs.jl b/src/traversals/dfs.jl
index 5169cf3..c5fefe9 100644
--- a/src/traversals/dfs.jl
+++ b/src/traversals/dfs.jl
@@ -28,25 +28,29 @@ end
 
 #Given a graph, split it into its connected components, construct a spanning tree over each of them
 # and take the union (adding in edges between the trees to recover a connected graph)
-function spanning_forest(g::AbstractNamedGraph)
-  components = connected_components(g)
-  return reduce(
-    union,
-    NamedGraph[
-      undirected_graph(bfs_tree(g[g_comp], first(g_comp))) for g_comp in components
-    ],
-  )
+function spanning_forest(
+  g::AbstractNamedGraph;
+  tree_constructor=g ->
+    undirected_graph(bfs_tree(g, first(keys(sort(eccentricities(g); rev=true))))),
+)
+  component_vertices = connected_components(g)
+  return reduce(union, (tree_constructor(g[vs]) for vs in component_vertices))
 end
 
-#Given a graph g with vertex set V, build a set of forests (each with vertex set V) which covers all edges in g
+#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 build_forest_cover(g::AbstractNamedGraph)
+function build_forest_cover(
+  g::AbstractNamedGraph;
+  tree_constructor=g ->
+    undirected_graph(bfs_tree(g, first(keys(sort(eccentricities(g); rev=true))))),
+)
+  @assert !NamedGraphs.is_directed(g)
   edges_collected = edgetype(g)[]
   remaining_edges = edges(g)
   forests = NamedGraph[]
   while !isempty(remaining_edges)
     g_reduced = rem_edges(g, edges_collected)
-    g_reduced_spanning_forest = spanning_forest(g_reduced)
+    g_reduced_spanning_forest = spanning_forest(g_reduced; tree_constructor)
     push!(edges_collected, edges(g_reduced_spanning_forest)...)
     push!(forests, g_reduced_spanning_forest)
     setdiff!(remaining_edges, edges(g_reduced_spanning_forest))
diff --git a/test/test_forests.jl b/test/test_forests.jl
index 9937e40..0acdd16 100644
--- a/test/test_forests.jl
+++ b/test/test_forests.jl
@@ -1,12 +1,7 @@
 using Test
 using Graphs
 using NamedGraphs
-using NamedGraphs:
-  decorate_graph_edges,
-  decorate_graph_vertices,
-  hexagonal_lattice_graph,
-  triangular_lattice_graph,
-  build_forest_cover
+using NamedGraphs: hexagonal_lattice_graph, triangular_lattice_graph, build_forest_cover
 
 @testset "Test Forest Cover" begin
   gs = [

From 720d641bbe2da4df9e1a8bc7c82414b072543cab Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Fri, 20 Oct 2023 11:05:45 -0400
Subject: [PATCH 03/14] Deleted rogue file

---
 examples/test.jl | 14 --------------
 1 file changed, 14 deletions(-)
 delete mode 100644 examples/test.jl

diff --git a/examples/test.jl b/examples/test.jl
deleted file mode 100644
index 7b850d7..0000000
--- a/examples/test.jl
+++ /dev/null
@@ -1,14 +0,0 @@
-using Test
-using NamedGraphs
-using NamedGraphs:
-  add_edges,
-  add_edges!,
-  rem_edges,
-  rem_edges!,
-  hexagonal_lattice_graph,
-  triangular_lattice_graph
-using Graphs
-
-g = hexagonal_lattice_graph(6, 6; periodic=false)
-
-g = triangular_lattice_graph(7, 7; periodic=true)

From 7c2571f24f26fec63cc19c6bc629fe13325ce916 Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Tue, 24 Oct 2023 15:41:34 -0400
Subject: [PATCH 04/14] Added proper test of DFS sort so that it justs tests
 for a valid solution, not a specific one

---
 test/test_namedgraph.jl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/test_namedgraph.jl b/test/test_namedgraph.jl
index b52d9a8..52510a5 100644
--- a/test/test_namedgraph.jl
+++ b/test/test_namedgraph.jl
@@ -557,6 +557,8 @@ end
     add_edge!(g, "D" => "G")
     add_edge!(g, "E" => "G")
     t = topological_sort_by_dfs(g)
-    @test t == ["C", "B", "E", "A", "D", "G", "F"]
+    for e in edges(g)
+      @test findfirst(x -> x == src(e), t) < findfirst(x -> x == dst(e), t) 
+    end
   end
 end

From 4b87693b42efdf66dc019838fedc7a350850177c Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Tue, 24 Oct 2023 16:03:35 -0400
Subject: [PATCH 05/14] Fixed mincut tests, to allow for degenrate solutions

---
 test/test_namedgraph.jl | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/test/test_namedgraph.jl b/test/test_namedgraph.jl
index 52510a5..7885f05 100644
--- a/test/test_namedgraph.jl
+++ b/test/test_namedgraph.jl
@@ -390,17 +390,16 @@ end
     g = NamedGraph(path_graph(4), ["A", "B", "C", "D"])
 
     part1, part2, flow = GraphsFlows.mincut(g, "A", "D")
-    @test issetequal(part1, ["A"])
-    @test issetequal(part2, ["B", "C", "D"])
+    @test "A" ∈ part1
+    @test "D" ∈ part2
     @test flow == 1
 
     part1, part2 = mincut_partitions(g, "A", "D")
-    @test issetequal(part1, ["A"])
-    @test issetequal(part2, ["B", "C", "D"])
+    @test "A" ∈ part1
+    @test "D" ∈ part2
 
     part1, part2 = mincut_partitions(g)
-    @test issetequal(part1, ["B", "C", "D"])
-    @test issetequal(part2, ["A"])
+    @test issetequal(vcat(part1, part2), vertices(g))
 
     weights_dict = Dict{Tuple{String,String},Float64}()
     weights_dict["A", "B"] = 3
@@ -411,17 +410,17 @@ end
 
     for weights in (weights_dict, weights_dictionary)
       part1, part2, flow = GraphsFlows.mincut(g, "A", "D", weights)
-      @test issetequal(part1, ["A", "B"])
-      @test issetequal(part2, ["C", "D"])
+      @test issetequal(part1, ["A", "B"]) || issetequal(part1, ["C", "D"])
+      @test issetequal(vcat(part1, part2), vertices(g))
       @test flow == 2
 
       part1, part2 = mincut_partitions(g, "A", "D", weights)
-      @test issetequal(part1, ["A", "B"])
-      @test issetequal(part2, ["C", "D"])
+      @test issetequal(part1, ["A", "B"]) || issetequal(part1, ["C", "D"])
+      @test issetequal(vcat(part1, part2), vertices(g))
 
       part1, part2 = mincut_partitions(g, weights)
-      @test issetequal(part1, ["C", "D"])
-      @test issetequal(part2, ["A", "B"])
+      @test issetequal(part1, ["A", "B"]) || issetequal(part1, ["C", "D"])
+      @test issetequal(vcat(part1, part2), vertices(g))
     end
   end
   @testset "dijkstra" begin
@@ -558,7 +557,7 @@ end
     add_edge!(g, "E" => "G")
     t = topological_sort_by_dfs(g)
     for e in edges(g)
-      @test findfirst(x -> x == src(e), t) < findfirst(x -> x == dst(e), t) 
+      @test findfirst(x -> x == src(e), t) < findfirst(x -> x == dst(e), t)
     end
   end
 end

From b34511adb40a0056df6dd13dd3ae0573803cda2e Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Wed, 25 Oct 2023 16:48:24 -0400
Subject: [PATCH 06/14] Better spanning tree interface. More tests. File
 Separation

---
 src/NamedGraphs.jl                  |  1 +
 src/traversals/trees_and_forests.jl | 43 +++++++++++++++++++++++++++
 test/test_forests.jl                | 25 ----------------
 test/test_trees_and_forests.jl      | 46 +++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 25 deletions(-)
 create mode 100644 src/traversals/trees_and_forests.jl
 delete mode 100644 test/test_forests.jl
 create mode 100644 test/test_trees_and_forests.jl

diff --git a/src/NamedGraphs.jl b/src/NamedGraphs.jl
index e5985fb..8de2717 100644
--- a/src/NamedGraphs.jl
+++ b/src/NamedGraphs.jl
@@ -113,6 +113,7 @@ include("distance.jl")
 include("distances_and_capacities.jl")
 include(joinpath("steiner_tree", "steiner_tree.jl"))
 include(joinpath("traversals", "dfs.jl"))
+include(joinpath("traversals", "trees_and_forests.jl"))
 include("namedgraph.jl")
 include(joinpath("generators", "named_staticgraphs.jl"))
 include(joinpath("Graphs", "generators", "staticgraphs.jl"))
diff --git a/src/traversals/trees_and_forests.jl b/src/traversals/trees_and_forests.jl
new file mode 100644
index 0000000..8486360
--- /dev/null
+++ b/src/traversals/trees_and_forests.jl
@@ -0,0 +1,43 @@
+default_root_vertex(g) = last(findmax(eccentricities(g)))
+
+function spanning_tree(
+  g::AbstractNamedGraph; root_vertex=default_root_vertex(g), alg::String="BFS"
+)
+  @assert !NamedGraphs.is_directed(g)
+  if alg == "BFS"
+    return undirected_graph(bfs_tree(g, root_vertex))
+  elseif alg == "RandomBFS"
+    return undirected_graph(random_bfs_tree(g, root_vertex))
+  elseif alg == "DFS"
+    return undirected_graph(dfs_tree(g, root_vertex))
+  else
+    error("Algorithm not current supported")
+  end
+end
+
+#Given a graph, split it into its connected components, construct a spanning tree over each of them
+# and take the union.
+function spanning_forest(
+  g::AbstractNamedGraph; spanning_tree_function=g -> spanning_tree(g)
+)
+  return reduce(union, (spanning_tree_function(g[vs]) for vs in connected_components(g)))
+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 build_forest_cover(
+  g::AbstractNamedGraph; spanning_tree_function=g -> spanning_tree(g)
+)
+  edges_collected = edgetype(g)[]
+  remaining_edges = edges(g)
+  forests = NamedGraph[]
+  while !isempty(remaining_edges)
+    g_reduced = rem_edges(g, edges_collected)
+    g_reduced_spanning_forest = spanning_forest(g_reduced; spanning_tree_function)
+    push!(edges_collected, edges(g_reduced_spanning_forest)...)
+    push!(forests, g_reduced_spanning_forest)
+    setdiff!(remaining_edges, edges(g_reduced_spanning_forest))
+  end
+
+  return forests
+end
diff --git a/test/test_forests.jl b/test/test_forests.jl
deleted file mode 100644
index 0acdd16..0000000
--- a/test/test_forests.jl
+++ /dev/null
@@ -1,25 +0,0 @@
-using Test
-using Graphs
-using NamedGraphs
-using NamedGraphs: hexagonal_lattice_graph, triangular_lattice_graph, build_forest_cover
-
-@testset "Test Forest Cover" begin
-  gs = [
-    named_grid((6, 1)),
-    named_grid((3, 3, 3)),
-    hexagonal_lattice_graph(6, 6),
-    named_comb_tree((4, 4)),
-    named_grid((10, 10)),
-    triangular_lattice_graph(5, 5; periodic=true),
-  ]
-  for g in gs
-    forest_cover = build_forest_cover(g)
-    cover_edges = reduce(vcat, edges.(forest_cover))
-    @test issetequal(cover_edges, edges(g))
-    for f in forest_cover
-      trees = NamedGraph[f[vs] for vs in connected_components(f)]
-      @test all(is_tree.(trees))
-      @test issetequal(vertices(f), vertices(g))
-    end
-  end
-end
diff --git a/test/test_trees_and_forests.jl b/test/test_trees_and_forests.jl
new file mode 100644
index 0000000..1a76e9d
--- /dev/null
+++ b/test/test_trees_and_forests.jl
@@ -0,0 +1,46 @@
+using Test
+using Graphs
+using NamedGraphs
+using NamedGraphs:
+  hexagonal_lattice_graph, triangular_lattice_graph, build_forest_cover, spanning_tree
+
+@testset "Test Spanning Trees" begin
+  gs = [
+    named_grid((6, 1)),
+    named_grid((3, 3, 3)),
+    hexagonal_lattice_graph(6, 6),
+    named_comb_tree((4, 4)),
+    named_grid((10, 10)),
+    triangular_lattice_graph(5, 5; periodic=true),
+  ]
+  algs = ["BFS", "DFS", "RandomBFS"]
+  for g in gs
+    for alg in algs
+      s_tree = spanning_tree(g; alg)
+      @test is_tree(s_tree)
+      @test Set(vertices(s_tree)) == Set(vertices(g))
+      @test issubset(Set(edges(s_tree)), Set(edges(g)))
+    end
+  end
+end
+
+@testset "Test Forest Cover" begin
+  gs = [
+    named_grid((6, 1)),
+    named_grid((3, 3, 3)),
+    hexagonal_lattice_graph(6, 6),
+    named_comb_tree((4, 4)),
+    named_grid((10, 10)),
+    triangular_lattice_graph(5, 5; periodic=true),
+  ]
+  for g in gs
+    forest_cover = build_forest_cover(g)
+    cover_edges = reduce(vcat, edges.(forest_cover))
+    @test issetequal(cover_edges, edges(g))
+    @test all(issetequal(vertices(f), vertices(g)) for f in forest_cover)
+    for f in forest_cover
+      trees = NamedGraph[f[vs] for vs in connected_components(f)]
+      @test all(is_tree.(trees))
+    end
+  end
+end

From fbc3b905a6da46dc01c9b8b8d48ba08a432f6e65 Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Thu, 26 Oct 2023 16:11:59 -0400
Subject: [PATCH 07/14] Removed redundant functions in dfs.jl

---
 src/traversals/dfs.jl | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/src/traversals/dfs.jl b/src/traversals/dfs.jl
index c5fefe9..44033fc 100644
--- a/src/traversals/dfs.jl
+++ b/src/traversals/dfs.jl
@@ -25,36 +25,3 @@ end
 function dfs_parents(graph::AbstractNamedGraph, vertex; kwargs...)
   return _dfs_parents(graph, vertex; kwargs...)
 end
-
-#Given a graph, split it into its connected components, construct a spanning tree over each of them
-# and take the union (adding in edges between the trees to recover a connected graph)
-function spanning_forest(
-  g::AbstractNamedGraph;
-  tree_constructor=g ->
-    undirected_graph(bfs_tree(g, first(keys(sort(eccentricities(g); rev=true))))),
-)
-  component_vertices = connected_components(g)
-  return reduce(union, (tree_constructor(g[vs]) for vs in component_vertices))
-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 build_forest_cover(
-  g::AbstractNamedGraph;
-  tree_constructor=g ->
-    undirected_graph(bfs_tree(g, first(keys(sort(eccentricities(g); rev=true))))),
-)
-  @assert !NamedGraphs.is_directed(g)
-  edges_collected = edgetype(g)[]
-  remaining_edges = edges(g)
-  forests = NamedGraph[]
-  while !isempty(remaining_edges)
-    g_reduced = rem_edges(g, edges_collected)
-    g_reduced_spanning_forest = spanning_forest(g_reduced; tree_constructor)
-    push!(edges_collected, edges(g_reduced_spanning_forest)...)
-    push!(forests, g_reduced_spanning_forest)
-    setdiff!(remaining_edges, edges(g_reduced_spanning_forest))
-  end
-
-  return forests
-end

From 89d968cba8ec8232c29cbbc80c24e4496819e58e Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Thu, 26 Oct 2023 16:34:14 -0400
Subject: [PATCH 08/14] Better algorithm specification

---
 src/traversals/trees_and_forests.jl | 42 ++++++++++++++++++-----------
 test/test_trees_and_forests.jl      | 16 +++++------
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/src/traversals/trees_and_forests.jl b/src/traversals/trees_and_forests.jl
index 8486360..8362652 100644
--- a/src/traversals/trees_and_forests.jl
+++ b/src/traversals/trees_and_forests.jl
@@ -1,33 +1,43 @@
+abstract type SpanningTreeAlgorithm end
+
+struct BFS <: SpanningTreeAlgorithm end
+struct RandomBFS <: SpanningTreeAlgorithm end
+struct DFS <: SpanningTreeAlgorithm end
+
+default_spanning_tree_alg() = BFS()
+
 default_root_vertex(g) = last(findmax(eccentricities(g)))
 
+function spanning_tree(g::AbstractNamedGraph; root_vertex=default_root_vertex(g))
+  return spanning_tree(default_spanning_tree_alg(), g; root_vertex)
+end
+
+function spanning_tree(::BFS, g::AbstractNamedGraph; root_vertex=default_root_vertex(g))
+  @assert !NamedGraphs.is_directed(g)
+  return undirected_graph(bfs_tree(g, root_vertex))
+end
+
 function spanning_tree(
-  g::AbstractNamedGraph; root_vertex=default_root_vertex(g), alg::String="BFS"
+  ::RandomBFS, g::AbstractNamedGraph; root_vertex=default_root_vertex(g)
 )
   @assert !NamedGraphs.is_directed(g)
-  if alg == "BFS"
-    return undirected_graph(bfs_tree(g, root_vertex))
-  elseif alg == "RandomBFS"
-    return undirected_graph(random_bfs_tree(g, root_vertex))
-  elseif alg == "DFS"
-    return undirected_graph(dfs_tree(g, root_vertex))
-  else
-    error("Algorithm not current supported")
-  end
+  return undirected_graph(random_bfs_tree(g, root_vertex))
+end
+
+function spanning_tree(::DFS, g::AbstractNamedGraph; root_vertex=default_root_vertex(g))
+  @assert !NamedGraphs.is_directed(g)
+  return undirected_graph(dfs_tree(g, root_vertex))
 end
 
 #Given a graph, split it into its connected components, construct a spanning tree over each of them
 # and take the union.
-function spanning_forest(
-  g::AbstractNamedGraph; spanning_tree_function=g -> spanning_tree(g)
-)
+function spanning_forest(g::AbstractNamedGraph; spanning_tree_function=spanning_tree)
   return reduce(union, (spanning_tree_function(g[vs]) for vs in connected_components(g)))
 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 build_forest_cover(
-  g::AbstractNamedGraph; spanning_tree_function=g -> spanning_tree(g)
-)
+function forest_cover(g::AbstractNamedGraph; spanning_tree_function=spanning_tree)
   edges_collected = edgetype(g)[]
   remaining_edges = edges(g)
   forests = NamedGraph[]
diff --git a/test/test_trees_and_forests.jl b/test/test_trees_and_forests.jl
index 1a76e9d..222d794 100644
--- a/test/test_trees_and_forests.jl
+++ b/test/test_trees_and_forests.jl
@@ -2,7 +2,7 @@ using Test
 using Graphs
 using NamedGraphs
 using NamedGraphs:
-  hexagonal_lattice_graph, triangular_lattice_graph, build_forest_cover, spanning_tree
+  hexagonal_lattice_graph, triangular_lattice_graph, forest_cover, spanning_tree
 
 @testset "Test Spanning Trees" begin
   gs = [
@@ -13,10 +13,10 @@ using NamedGraphs:
     named_grid((10, 10)),
     triangular_lattice_graph(5, 5; periodic=true),
   ]
-  algs = ["BFS", "DFS", "RandomBFS"]
+  algs = [NamedGraphs.BFS(), NamedGraphs.DFS(), NamedGraphs.RandomBFS()]
   for g in gs
     for alg in algs
-      s_tree = spanning_tree(g; alg)
+      s_tree = spanning_tree(alg, g)
       @test is_tree(s_tree)
       @test Set(vertices(s_tree)) == Set(vertices(g))
       @test issubset(Set(edges(s_tree)), Set(edges(g)))
@@ -34,12 +34,12 @@ end
     triangular_lattice_graph(5, 5; periodic=true),
   ]
   for g in gs
-    forest_cover = build_forest_cover(g)
-    cover_edges = reduce(vcat, edges.(forest_cover))
+    cover = forest_cover(g)
+    cover_edges = reduce(vcat, edges.(cover))
     @test issetequal(cover_edges, edges(g))
-    @test all(issetequal(vertices(f), vertices(g)) for f in forest_cover)
-    for f in forest_cover
-      trees = NamedGraph[f[vs] for vs in connected_components(f)]
+    @test all(issetequal(vertices(forest), vertices(g)) for forest in cover)
+    for forest in cover
+      trees = NamedGraph[forest[vs] for vs in connected_components(forest)]
       @test all(is_tree.(trees))
     end
   end

From cf9df1a2f99340b6c0bc369f3c2b67d7817cad1e Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Mon, 30 Oct 2023 12:42:08 -0400
Subject: [PATCH 09/14] Added undirected train to spanning tree functions

---
 src/traversals/trees_and_forests.jl | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/traversals/trees_and_forests.jl b/src/traversals/trees_and_forests.jl
index 8362652..3ebe95a 100644
--- a/src/traversals/trees_and_forests.jl
+++ b/src/traversals/trees_and_forests.jl
@@ -12,20 +12,21 @@ function spanning_tree(g::AbstractNamedGraph; root_vertex=default_root_vertex(g)
   return spanning_tree(default_spanning_tree_alg(), g; root_vertex)
 end
 
-function spanning_tree(::BFS, g::AbstractNamedGraph; root_vertex=default_root_vertex(g))
-  @assert !NamedGraphs.is_directed(g)
+@traitfn function spanning_tree(
+  ::BFS, g::AbstractNamedGraph::(!IsDirected); root_vertex=default_root_vertex(g)
+)
   return undirected_graph(bfs_tree(g, root_vertex))
 end
 
-function spanning_tree(
-  ::RandomBFS, g::AbstractNamedGraph; root_vertex=default_root_vertex(g)
+@traitfn function spanning_tree(
+  ::RandomBFS, g::AbstractNamedGraph::(!IsDirected); root_vertex=default_root_vertex(g)
 )
-  @assert !NamedGraphs.is_directed(g)
   return undirected_graph(random_bfs_tree(g, root_vertex))
 end
 
-function spanning_tree(::DFS, g::AbstractNamedGraph; root_vertex=default_root_vertex(g))
-  @assert !NamedGraphs.is_directed(g)
+@traitfn function spanning_tree(
+  ::DFS, g::AbstractNamedGraph::(!IsDirected); root_vertex=default_root_vertex(g)
+)
   return undirected_graph(dfs_tree(g, root_vertex))
 end
 

From 9c77ae585798f45a7fb3aa19d5aadc789aa26eee Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Mon, 30 Oct 2023 15:19:53 -0400
Subject: [PATCH 10/14] Simplified Test Structure. Spanning tree function ->
 spanning_tree

---
 src/traversals/trees_and_forests.jl | 10 ++---
 test/test_trees_and_forests.jl      | 59 +++++++++++------------------
 2 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/src/traversals/trees_and_forests.jl b/src/traversals/trees_and_forests.jl
index 3ebe95a..2fb8e0e 100644
--- a/src/traversals/trees_and_forests.jl
+++ b/src/traversals/trees_and_forests.jl
@@ -30,21 +30,21 @@ end
   return undirected_graph(dfs_tree(g, root_vertex))
 end
 
-#Given a graph, split it into its connected components, construct a spanning tree over each of them
+#Given a graph, split it into its connected components, construct a spanning tree, using the function spanning_tree, over each of them
 # and take the union.
-function spanning_forest(g::AbstractNamedGraph; spanning_tree_function=spanning_tree)
-  return reduce(union, (spanning_tree_function(g[vs]) for vs in connected_components(g)))
+function spanning_forest(g::AbstractNamedGraph; spanning_tree=spanning_tree)
+  return reduce(union, (spanning_tree(g[vs]) for vs in connected_components(g)))
 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::AbstractNamedGraph; spanning_tree_function=spanning_tree)
+function forest_cover(g::AbstractNamedGraph; spanning_tree=spanning_tree)
   edges_collected = edgetype(g)[]
   remaining_edges = edges(g)
   forests = NamedGraph[]
   while !isempty(remaining_edges)
     g_reduced = rem_edges(g, edges_collected)
-    g_reduced_spanning_forest = spanning_forest(g_reduced; spanning_tree_function)
+    g_reduced_spanning_forest = spanning_forest(g_reduced; spanning_tree)
     push!(edges_collected, edges(g_reduced_spanning_forest)...)
     push!(forests, g_reduced_spanning_forest)
     setdiff!(remaining_edges, edges(g_reduced_spanning_forest))
diff --git a/test/test_trees_and_forests.jl b/test/test_trees_and_forests.jl
index 222d794..72c0e45 100644
--- a/test/test_trees_and_forests.jl
+++ b/test/test_trees_and_forests.jl
@@ -4,43 +4,30 @@ using NamedGraphs
 using NamedGraphs:
   hexagonal_lattice_graph, triangular_lattice_graph, forest_cover, spanning_tree
 
-@testset "Test Spanning Trees" begin
-  gs = [
-    named_grid((6, 1)),
-    named_grid((3, 3, 3)),
-    hexagonal_lattice_graph(6, 6),
-    named_comb_tree((4, 4)),
-    named_grid((10, 10)),
-    triangular_lattice_graph(5, 5; periodic=true),
-  ]
-  algs = [NamedGraphs.BFS(), NamedGraphs.DFS(), NamedGraphs.RandomBFS()]
-  for g in gs
-    for alg in algs
-      s_tree = spanning_tree(alg, g)
-      @test is_tree(s_tree)
-      @test Set(vertices(s_tree)) == Set(vertices(g))
-      @test issubset(Set(edges(s_tree)), Set(edges(g)))
-    end
-  end
+gs = [
+  ("Chain", named_grid((6, 1))),
+  ("Cubic Lattice", named_grid((3, 3, 3))),
+  ("Hexagonal Grid", hexagonal_lattice_graph(6, 6)),
+  ("Comb Tree", named_comb_tree((4, 4))),
+  ("Square lattice", named_grid((10, 10))),
+  ("Triangular Grid", triangular_lattice_graph(5, 5; periodic=true)),
+];
+algs = [NamedGraphs.BFS(), NamedGraphs.DFS(), NamedGraphs.RandomBFS()];
+
+@testset "Test Spanning Trees $g_string, $alg" for (g_string, g) in gs, alg in algs
+  s_tree = spanning_tree(alg, g)
+  @test is_tree(s_tree)
+  @test Set(vertices(s_tree)) == Set(vertices(g))
+  @test issubset(Set(edges(s_tree)), Set(edges(g)))
 end
 
-@testset "Test Forest Cover" begin
-  gs = [
-    named_grid((6, 1)),
-    named_grid((3, 3, 3)),
-    hexagonal_lattice_graph(6, 6),
-    named_comb_tree((4, 4)),
-    named_grid((10, 10)),
-    triangular_lattice_graph(5, 5; periodic=true),
-  ]
-  for g in gs
-    cover = forest_cover(g)
-    cover_edges = reduce(vcat, edges.(cover))
-    @test issetequal(cover_edges, edges(g))
-    @test all(issetequal(vertices(forest), vertices(g)) for forest in cover)
-    for forest in cover
-      trees = NamedGraph[forest[vs] for vs in connected_components(forest)]
-      @test all(is_tree.(trees))
-    end
+@testset "Test Forest Cover $g_string" for (g_string, g) in gs
+  cover = forest_cover(g)
+  cover_edges = reduce(vcat, edges.(cover))
+  @test issetequal(cover_edges, edges(g))
+  @test all(issetequal(vertices(forest), vertices(g)) for forest in cover)
+  for forest in cover
+    trees = NamedGraph[forest[vs] for vs in connected_components(forest)]
+    @test all(is_tree.(trees))
   end
 end

From 896deb7631f899df591a3cf2e3fb5a89020a366a Mon Sep 17 00:00:00 2001
From: Joey <joeytindall94@googlemail.com>
Date: Tue, 31 Oct 2023 16:12:33 -0400
Subject: [PATCH 11/14] Moved variables outside global scope in
 test_trees_and_forests.jl

---
 test/test_trees_and_forests.jl | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/test/test_trees_and_forests.jl b/test/test_trees_and_forests.jl
index 72c0e45..d315158 100644
--- a/test/test_trees_and_forests.jl
+++ b/test/test_trees_and_forests.jl
@@ -1,9 +1,11 @@
 using Test
 using Graphs
 using NamedGraphs
-using NamedGraphs:
-  hexagonal_lattice_graph, triangular_lattice_graph, forest_cover, spanning_tree
+using NamedGraphs: forest_cover, spanning_tree
 
+module TestTreesAndForests
+using NamedGraphs
+using NamedGraphs: hexagonal_lattice_graph, triangular_lattice_graph
 gs = [
   ("Chain", named_grid((6, 1))),
   ("Cubic Lattice", named_grid((3, 3, 3))),
@@ -11,17 +13,20 @@ gs = [
   ("Comb Tree", named_comb_tree((4, 4))),
   ("Square lattice", named_grid((10, 10))),
   ("Triangular Grid", triangular_lattice_graph(5, 5; periodic=true)),
-];
-algs = [NamedGraphs.BFS(), NamedGraphs.DFS(), NamedGraphs.RandomBFS()];
+]
+algs = [NamedGraphs.BFS(), NamedGraphs.DFS(), NamedGraphs.RandomBFS()]
+end
+
+@testset "Test Spanning Trees $g_string, $alg" for (g_string, g) in TestTreesAndForests.gs,
+  alg in TestTreesAndForests.algs
 
-@testset "Test Spanning Trees $g_string, $alg" for (g_string, g) in gs, alg in algs
   s_tree = spanning_tree(alg, g)
   @test is_tree(s_tree)
   @test Set(vertices(s_tree)) == Set(vertices(g))
   @test issubset(Set(edges(s_tree)), Set(edges(g)))
 end
 
-@testset "Test Forest Cover $g_string" for (g_string, g) in gs
+@testset "Test Forest Cover $g_string" for (g_string, g) in TestTreesAndForests.gs
   cover = forest_cover(g)
   cover_edges = reduce(vcat, edges.(cover))
   @test issetequal(cover_edges, edges(g))

From f35a1351dc473a0f8d7cf00beb1cdd70aae8d109 Mon Sep 17 00:00:00 2001
From: Joseph Tindall <51231103+JoeyT1994@users.noreply.github.com>
Date: Wed, 1 Nov 2023 11:58:24 -0400
Subject: [PATCH 12/14] Update src/traversals/trees_and_forests.jl

Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>
---
 src/traversals/trees_and_forests.jl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/traversals/trees_and_forests.jl b/src/traversals/trees_and_forests.jl
index 2fb8e0e..7e40ad5 100644
--- a/src/traversals/trees_and_forests.jl
+++ b/src/traversals/trees_and_forests.jl
@@ -8,8 +8,8 @@ default_spanning_tree_alg() = BFS()
 
 default_root_vertex(g) = last(findmax(eccentricities(g)))
 
-function spanning_tree(g::AbstractNamedGraph; root_vertex=default_root_vertex(g))
-  return spanning_tree(default_spanning_tree_alg(), g; root_vertex)
+function spanning_tree(g::AbstractNamedGraph; alg=default_spanning_tree_alg(), root_vertex=default_root_vertex(g))
+  return spanning_tree(alg, g; root_vertex)
 end
 
 @traitfn function spanning_tree(

From 888e80ba2271eec85c97112d1b9183b29d5e13c7 Mon Sep 17 00:00:00 2001
From: Joseph Tindall <51231103+JoeyT1994@users.noreply.github.com>
Date: Wed, 1 Nov 2023 12:22:59 -0400
Subject: [PATCH 13/14] Update test/test_trees_and_forests.jl

Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>
---
 test/test_trees_and_forests.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test_trees_and_forests.jl b/test/test_trees_and_forests.jl
index d315158..ac84342 100644
--- a/test/test_trees_and_forests.jl
+++ b/test/test_trees_and_forests.jl
@@ -20,7 +20,7 @@ end
 @testset "Test Spanning Trees $g_string, $alg" for (g_string, g) in TestTreesAndForests.gs,
   alg in TestTreesAndForests.algs
 
-  s_tree = spanning_tree(alg, g)
+  s_tree = spanning_tree(g; alg)
   @test is_tree(s_tree)
   @test Set(vertices(s_tree)) == Set(vertices(g))
   @test issubset(Set(edges(s_tree)), Set(edges(g)))

From a6ed4f73cf770ad8e16532805bf9f2b95dc7655c Mon Sep 17 00:00:00 2001
From: Joseph Tindall <joeytindall94@googlemail.com>
Date: Wed, 1 Nov 2023 13:52:07 -0400
Subject: [PATCH 14/14] Formatting

---
 src/traversals/trees_and_forests.jl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/traversals/trees_and_forests.jl b/src/traversals/trees_and_forests.jl
index 7e40ad5..de433ee 100644
--- a/src/traversals/trees_and_forests.jl
+++ b/src/traversals/trees_and_forests.jl
@@ -8,7 +8,9 @@ default_spanning_tree_alg() = BFS()
 
 default_root_vertex(g) = last(findmax(eccentricities(g)))
 
-function spanning_tree(g::AbstractNamedGraph; alg=default_spanning_tree_alg(), root_vertex=default_root_vertex(g))
+function spanning_tree(
+  g::AbstractNamedGraph; alg=default_spanning_tree_alg(), root_vertex=default_root_vertex(g)
+)
   return spanning_tree(alg, g; root_vertex)
 end