Fix bug in is_arranged with variable length Tuples

Author: mtfishman

src/abstractdatagraph.jl

@@ -289,9 +289,9 @@ function get(graph::AbstractDataGraph, vertex, default)
 end
 
 function getindex(graph::AbstractDataGraph, edge::AbstractEdge)
-  is_edge_arranged = is_arranged(graph, edge)
-  data = edge_data(graph)[arrange(is_edge_arranged, edge)]
-  return reverse_data_direction(is_edge_arranged, graph, data)
+  is_edge_arranged_ = is_edge_arranged(graph, edge)
+  data = edge_data(graph)[arrange(is_edge_arranged_, edge)]
+  return reverse_data_direction(is_edge_arranged_, graph, data)
 end
 
 # Support syntax `g[v1 => v2]`
@@ -300,9 +300,9 @@ function getindex(graph::AbstractDataGraph, edge::Pair)
 end
 
 function get(graph::AbstractDataGraph, edge::AbstractEdge, default)
-  is_edge_arranged = is_arranged(graph, edge)
-  data = get(edge_data(graph), arrange(is_edge_arranged, edge), default)
-  return reverse_data_direction(is_edge_arranged, graph, data)
+  is_edge_arranged_ = is_edge_arranged(graph, edge)
+  data = get(edge_data(graph), arrange(is_edge_arranged_, edge), default)
+  return reverse_data_direction(is_edge_arranged_, graph, data)
 end
 
 function get(graph::AbstractDataGraph, edge::Pair, default)
@@ -332,9 +332,9 @@ function setindex!(graph::AbstractDataGraph, data, vertex)
 end
 
 function setindex!(graph::AbstractDataGraph, data, edge::AbstractEdge)
-  is_edge_arranged = is_arranged(graph, edge)
-  arranged_edge = arrange(is_edge_arranged, edge)
-  arranged_data = reverse_data_direction(is_edge_arranged, graph, data)
+  is_edge_arranged_ = is_edge_arranged(graph, edge)
+  arranged_edge = arrange(is_edge_arranged_, edge)
+  arranged_data = reverse_data_direction(is_edge_arranged_, graph, data)
   set!(edge_data(graph), arranged_edge, arranged_data)
   return graph
 end

src/arrange.jl

@@ -5,34 +5,46 @@
 # stored in directed AbstractDataGraph types (which by default returns nothing,
 # indicating not to automatically store data in both directions)
 # TODO: Use `Graphs.is_ordered`? https://juliagraphs.org/Graphs.jl/v1.7/core_functions/core/#Graphs.is_ordered-Tuple{AbstractEdge}
-@traitfn function is_arranged(graph::AbstractDataGraph::IsDirected, src, dst)
-  return true
-end
-
-@traitfn function is_arranged(graph::AbstractDataGraph::(!IsDirected), src, dst)
+function is_arranged(src, dst)
   if !hasmethod(isless, typeof.((src, dst)))
-    src_hash = hash(src)
-    dst_hash = hash(dst)
-    if (src_hash == dst_hash) && (src ≠ dst)
-      @warn "Hash collision when arranging vertices to extract edge data. Setting or extracting data may be ill-defined."
-    end
-    return isless(src_hash, dst_hash)
+    return is_arranged_by_hash(src, dst)
   end
   return isless(src, dst)
 end
 
-@traitfn function is_arranged(graph::AbstractDataGraph::(!IsDirected), t1::Tuple, t2::Tuple)
+function is_arranged_by_hash(src, dst)
+  src_hash = hash(src)
+  dst_hash = hash(dst)
+  if (src_hash == dst_hash) && (src ≠ dst)
+    @warn "Hash collision when arranging vertices to extract edge data. Setting or extracting data may be ill-defined."
+  end
+  return isless(src_hash, dst_hash)
+end
+
+# https://github.com/JuliaLang/julia/blob/v1.8.5/base/tuple.jl#L470-L482
+is_arranged(::Tuple{}, ::Tuple{}) = false
+is_arranged(::Tuple{}, ::Tuple) = true
+is_arranged(::Tuple, ::Tuple{}) = false
+
+function is_arranged(t1::Tuple, t2::Tuple)
   a, b = t1[1], t2[1]
-  return is_arranged(graph, a, b) ||
-         (isequal(a, b) && is_arranged(graph, Base.tail(t1), Base.tail(t2)))
+  return is_arranged(a, b) || (isequal(a, b) && is_arranged(Base.tail(t1), Base.tail(t2)))
+end
+
+@traitfn function is_edge_arranged(graph::AbstractDataGraph::IsDirected, src, dst)
+  return true
+end
+
+@traitfn function is_edge_arranged(graph::AbstractDataGraph::(!IsDirected), src, dst)
+  return is_arranged(src, dst)
 end
 
-function is_arranged(graph::AbstractDataGraph, edge::AbstractEdge)
-  return is_arranged(graph, src(edge), dst(edge))
+function is_edge_arranged(graph::AbstractDataGraph, edge::AbstractEdge)
+  return is_edge_arranged(graph, src(edge), dst(edge))
 end
 
 function arrange(graph::AbstractDataGraph, edge::AbstractEdge)
-  return arrange(is_arranged(graph, edge), edge)
+  return arrange(is_edge_arranged(graph, edge), edge)
 end
 
 function arrange(is_arranged::Bool, edge::AbstractEdge)

test/runtests.jl

@@ -5,6 +5,8 @@ using NamedGraphs
 using Suppressor
 using Test
 
+using DataGraphs: is_arranged
+
 @testset "DataGraphs.jl" begin
   @testset "Examples" begin
     examples_path = joinpath(pkgdir(DataGraphs), "examples")
@@ -15,6 +17,25 @@ using Test
     end
   end
 
+  @testset "is_arranged" begin
+    for (a, b) in [
+      (1, 2),
+      ([1], [2]),
+      ([1, 2], [2, 1]),
+      ([1, 2], [2]),
+      ([2], [2, 1]),
+      ((1,), (2,)),
+      ((1, 2), (2, 1)),
+      ((1, 2), (2,)),
+      ((2,), (2, 1)),
+      ("X", 1),
+      (("X",), (1, 2)),
+    ]
+      @test is_arranged(a, b)
+      @test !is_arranged(b, a)
+    end
+  end
+
   @testset "Basics" begin
     g = grid((4,))
     dg = DataGraph{<:Any,String,Symbol}(g)