Repository: invenia/NamedDims.jl (julia)

Issue:
Ambiguity on 1.5: Adjoint{...,Vector} * NamedDimsArray{...,Vector}
Julia 1.5 seems to cause extra method ambiguities, for instance this:
```julia
julia> using NamedDims

julia> rand(3)' * NamedDimsArray(rand(3), :a)
ERROR: MethodError: *(::LinearAlgebra.Adjoint{Float64,Array{Float64,1}}, ::NamedDimsArray{(:a,),Float64,1,Array{Float64,1}}) is ambiguous. Candidates:
  *(u::LinearAlgebra.Adjoint{T,var"#s814"} where var"#s814"<:(AbstractArray{T,1} where T), v::AbstractArray{T,1}) where T<:Number in LinearAlgebra at /Applications/Julia-1.5.app/Contents/Resources/julia/share/julia/stdlib/v1.5/LinearAlgebra/src/adjtrans.jl:267
  *(a::LinearAlgebra.Adjoint{var"#s16",var"#s15"} where var"#s15"<:(AbstractArray{T,1} where T) where var"#s16", b::NamedDimsArray{L,T,1,var"#s17"} where var"#s17"<:AbstractArray{T,1}) where {L, T} in NamedDims at /Users/me/.julia/packages/NamedDims/p7e4G/src/functions_math.jl:48
Possible fix, define
  *(::LinearAlgebra.Adjoint{T,var"#s15"} where var"#s15"<:(AbstractArray{T,1} where T), ::NamedDimsArray{L,T,1,var"#s17"} where var"#s17"<:AbstractArray{T,1}) where {L, T<:Number}
Stacktrace:
 [1] top-level scope at REPL[2]:1

julia> [ones(2,2), ones(2,2)]' * NamedDimsArray([ones(2,2), ones(2,2)], :a)
2×2 Array{Float64,2}:
 4.0  4.0
 4.0  4.0
```

xref https://github.com/mcabbott/AxisKeys.jl/issues/17 

Edit: also here: https://github.com/JuliaCI/NanosoldierReports/blob/master/pkgeval/by_date/2020-05/09/logs/NamedDims/1.6.0-DEV-b828bbc426.log#L129

Pull request description:
Closes #112
Makes #128 redundant

Cause is:
https://github.com/JuliaLang/julia/pull/36679
 removed a defintiion of matmul with `Adjoint{<:Any}` and replaced it with one with `Adjoint{<:Number}`
we were winning that ambiguity fight before, but now it ties.
This PR changes us to match.

Patch:
diff --git a/Project.toml b/Project.toml
index 1125a3e..ea64c9c 100644
--- a/Project.toml
+++ b/Project.toml
@@ -1,7 +1,7 @@
 name = "NamedDims"
 uuid = "356022a1-0364-5f58-8944-0da4b18d706f"
 authors = ["Invenia Technical Computing Corporation"]
-version = "0.2.24"
+version = "0.2.25"
 
 [deps]
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
diff --git a/src/functions_math.jl b/src/functions_math.jl
index b11b991..8454295 100644
--- a/src/functions_math.jl
+++ b/src/functions_math.jl
@@ -44,7 +44,7 @@ function Base.:*(a::NamedDimsArray{L,T,2,<:CoVector}, b::AbstractVector) where {
 end
 
 # Using `CoVector` results in Method ambiguities; have to define more specific methods.
-for A in (Adjoint{<:Any, <:AbstractVector}, Transpose{<:Real, <:AbstractVector{<:Real}})
+for A in (Adjoint{<:Number, <:AbstractVector}, Transpose{<:Real, <:AbstractVector{<:Real}})
     @eval function Base.:*(a::$A, b::NamedDimsArray{L,T,1,<:AbstractVector{T}}) where {L,T}
         return *(a, parent(b))
     end
diff --git a/src/name_core.jl b/src/name_core.jl
index 5d23b9f..566951c 100644
--- a/src/name_core.jl
+++ b/src/name_core.jl
@@ -84,7 +84,7 @@ function expand_dimnames(dimnames::Tuple, name::Symbol)
     if dim_noerror(dimnames, name) > 0  # name in dimnames, but optimised
         return dimnames
     else
-        return (dimnames..., name)
+        return compile_time_return_hack((dimnames..., name))
     end
 end
 
@@ -98,15 +98,17 @@ function expand_dimnames(dimnames::Tuple, name::Integer)
     else
         extra_length = name - length(dimnames)
         new_dimnames = ntuple(i->:_, extra_length)
-        return (dimnames..., new_dimnames...)
+        return compile_time_return_hack((dimnames..., new_dimnames...))
     end
 end
 
 function expand_dimnames(dimnames::Tuple, names)
-    expand_dimnames(expand_dimnames(dimnames, names[1]), names[2:end])
+    return expand_dimnames(
+        expand_dimnames(dimnames, first(names)),
+        Base.tail(names),
+    )
 end
 
-
 """
     permute_dimnames(dimnames, perm)
 


Test patch:
diff --git a/test/name_core.jl b/test/name_core.jl
index 6fd7be9..7952117 100644
--- a/test/name_core.jl
+++ b/test/name_core.jl
@@ -15,7 +15,6 @@ using NamedDims:
     remaining_dimnames_after_dropping
 using Test
 
-
 @testset "dim" begin
     @testset "small case" begin
         @test dim((:x, :y), :x) == 1
@@ -32,11 +31,7 @@ end
 @testset "allocations: dim" begin
     @test 0 == @ballocated (()->dim((:a, :b), :b))()
     @test 0 == @ballocated (()->dim_noerror((:a, :b, :c), :c))()
-    if VERSION >= v"1.1"
-        @test 0 == @ballocated (()->dim((:a,:b), (:a,:b)))()
-    else
-        @test_broken 0 == @ballocated (()->dim((:a,:b), (:a,:b)))()
-    end
+    @test_modern 0 == @ballocated (()->dim((:a,:b), (:a,:b)))()
 end
 
 
@@ -79,27 +74,14 @@ end
 end
 @testset "allocations: unify_names_*" begin
     for unify in (unify_names, unify_names_longest, unify_names_shortest)
-        if VERSION >= v"1.1"
-            @test 0 == @ballocated (()->$unify((:a, :b), (:a, :_)))()
-        else
-            @test_broken 0 == @ballocated (()->$unify((:a, :b), (:a, :_)))()
-        end
-    end
-    if VERSION >= v"1.1"
-        @test 0 == @ballocated (()->unify_names_longest((:a, :b), (:a, :_, :c)))()
-        @test 0 == @ballocated (()->unify_names_shortest((:a, :b), (:a, :_, :c)))()
-        @test 0 == @ballocated (()->unify_names_longest((:a,), (:a, :b), (:a, :_, :c)))()
-        @test 0 == @ballocated (()->unify_names_shortest((:a,), (:a, :b), (:a, :_, :c)))()
-        @test 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :_)))()
-        @test 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :c)))()
-    else
-        @test_broken 0 == @ballocated (()->unify_names_longest((:a, :b), (:a, :_, :c)))()
-        @test_broken 0 == @ballocated (()->unify_names_shortest((:a, :b), (:a, :_, :c)))()
-        @test_broken 0 == @ballocated (()->unify_names_longest((:a,), (:a, :b), (:a, :_, :c)))()
-        @test_broken 0 == @ballocated (()->unify_names_shortest((:a,), (:a, :b), (:a, :_, :c)))()
-        @test_broken 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :_)))()
-        @test_broken 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :c)))()
+        @test_modern 0 == @ballocated (()->$unify((:a, :b), (:a, :_)))()
     end
+    @test_modern 0 == @ballocated (()->unify_names_longest((:a, :b), (:a, :_, :c)))()
+    @test_modern 0 == @ballocated (()->unify_names_shortest((:a, :b), (:a, :_, :c)))()
+    @test_modern 0 == @ballocated (()->unify_names_longest((:a,), (:a, :b), (:a, :_, :c)))()
+    @test_modern 0 == @ballocated (()->unify_names_shortest((:a,), (:a, :b), (:a, :_, :c)))()
+    @test_modern 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :_)))()
+    @test_modern 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :c)))()
     @test 0 == @ballocated (()->names_are_unifiable((:a, :b), (:a, :b)))()
 end
 
@@ -131,13 +113,13 @@ end
     @testset "names" begin
         @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), :x))()
         @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), :z))()
-        @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), (:x, :z)))()
+        @test_modern 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), (:x, :z)))()
     end
 
     @testset "numbers et al" begin
         @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), 1))()
         @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), 5))()
-        @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), (1, 5)))()
+        @test_modern 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), (1, 5)))()
         @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), :))()
         @test 0 == @ballocated (()->NamedDims.expand_dimnames((:x, :y), ()))()
     end
@@ -203,9 +185,7 @@ end
     @test_throws BoundsError permute_dimnames((:a, :b), (1, 0))
 end
 @testset "allocations: permute_dimnames" begin
-    if VERSION >= v"1.1"
-        @test 0 == @ballocated permute_dimnames((:a,:b,:c), (1,3,2))
-    end
+    @test_modern 0 == @ballocated permute_dimnames((:a,:b,:c), (1,3,2))
 end
 
 
diff --git a/test/runtests.jl b/test/runtests.jl
index e2038b4..91006cd 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -3,6 +3,8 @@ using BenchmarkTools
 using SparseArrays
 using Test
 
+include("test_helpers.jl")
+
 const testfiles = (
     "name_core.jl",
     "wrapper_array.jl",
diff --git a/test/test_helpers.jl b/test/test_helpers.jl
new file mode 100644
index 0000000..c299521
--- /dev/null
+++ b/test/test_helpers.jl
@@ -0,0 +1,15 @@
+"""
+    @test_modern
+
+Use this to mark tests that should pass on modern (>=v1.1) versions of julia,
+but should be marked as `@test_broken` on Julia 1.0.
+For more complicated cases, or where it shouldn't be defined at all on julia versions,
+you can use `if VERSION` directly.
+"""
+macro test_modern(expr...)
+    if VERSION >= v"1.1"
+        return :(@test $(expr...))
+    else
+        return :(@test_broken $(expr...))
+    end
+end