Repository: invenia/NamedDims.jl
Pull request #15 (2019-04-24 13:39:08)

Cleanup
This redoes the name from combine to unify
and also fixes an inconstient naming of `names_a`
`sourcewalk` is really handy
http://mikeinnes.github.io/MacroTools.jl/stable/sourcewalk/

Also it makes another operation run at compile time.
(which was missed because it was in a seperate parallel, PR)


codecov[bot]: # [Codecov](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=h1) Report
> Merging [#15](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=desc) into [master](https://codecov.io/gh/invenia/NamedDims.jl/commit/9dea8581f4e3523ad9cba52c8a35800ef221f599?src=pr&el=desc) will **decrease** coverage by `1.32%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/invenia/NamedDims.jl/pull/15/graphs/tree.svg?width=650&token=M5MW3xa0xM&height=150&src=pr)](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
- Coverage   92.36%   91.03%   -1.33%     
==========================================
  Files           7        7              
  Lines         144      145       +1     
==========================================
- Hits          133      132       -1     
- Misses         11       13       +2
```


| [Impacted Files](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/broadcasting.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/15/diff?src=pr&el=tree#diff-c3JjL2Jyb2FkY2FzdGluZy5qbA==) | `80% <100%> (ø)` | :arrow_up: |
| [src/name\_core.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/15/diff?src=pr&el=tree#diff-c3JjL25hbWVfY29yZS5qbA==) | `88.33% <62.5%> (-3.2%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=footer). Last update [9dea858...45b4612](https://codecov.io/gh/invenia/NamedDims.jl/pull/15?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/broadcasting.jl b/src/broadcasting.jl
index d6e2a38..26c7fe1 100644
--- a/src/broadcasting.jl
+++ b/src/broadcasting.jl
@@ -17,7 +17,7 @@ broadcasted_names(bc::Base.Broadcast.Broadcasted) = broadcasted_names(bc.args...
 function broadcasted_names(a, bs...)
     a_name = broadcasted_names(a)
     b_name = broadcasted_names(bs...)
-    combine_names_longest(a_name, b_name)
+    unify_names_longest(a_name, b_name)
 end
 broadcasted_names(a::AbstractArray) = names(a)
 broadcasted_names(a) = tuple()
diff --git a/src/name_core.jl b/src/name_core.jl
index a3d99c9..3d402bc 100644
--- a/src/name_core.jl
+++ b/src/name_core.jl
@@ -137,27 +137,27 @@ function incompatible_dimension_error(names_a, names_b)
 end
 
 """
-    combine_names(a, b)
+    unify_names(a, b)
 
 Produces the merged set of names for tuples of names `a` and `b`,
-or an error if it is not possibly to combine them.
-Two tuples of names can be combined they are the same length
+or an error if it is not possibly to unify them.
+Two tuples of names can be unified they are the same length
 and if for each position the names are either the same, or one is a wildcard (`:_`).
 When combining wildcard with non-wildcard the resulting name is the non-wildcard.
 (This is somewhat like the very simplest case of unification in e.g prolog).
 
 For example:
- - `(:a, :b)` and `(:a, :b)` can be combined to give `(:a, :b)`
- - similarly: `(:a, :_)` and `(:a, :b)` can also be combined to give `(:a, :b)`
- - `(:a, :b)` and `(:b, :c)` can not be combined as the names at each position to not match.
- - `(:a, :b)` and `(:a, :b, :c)` can not be combined as they have different lengths
+ - `(:a, :b)` and `(:a, :b)` can be unified to give `(:a, :b)`
+ - similarly: `(:a, :_)` and `(:a, :b)` can also be unified to give `(:a, :b)`
+ - `(:a, :b)` and `(:b, :c)` cannot be unified as the names at each position to not match.
+ - `(:a, :b)` and `(:a, :b, :c)` cannot be unified as they have different lengths
 
 This is the type of name combination used for binary array operations.
 Where the dimensions of both arrays must be the same.
 """
-function combine_names(names_a, names_b)
+function unify_names(names_a, names_b)
     # 0-Allocations if inputs are the same
-    # 0-Allocation, if has a `:_` see  `@btime (()->combine_names((:a, :b), (:a, :_)))()`
+    # 0-Allocation, if has a `:_` see  `@btime (()->unify_names((:a, :b), (:a, :_)))()`
 
     names_a === names_b && return names_a
 
@@ -179,10 +179,10 @@ function combine_names(names_a, names_b)
 end
 
 """
-    combine_names_longest(a, b)
+    unify_names_longest(a, b)
 
-This is the same as [`combine_names`](@ref), but with the equal length requirement removed.
-It combines the names up to the length of the shortest, and takes the named from the longest
+This is the same as [`unify_names`](@ref), but with the equal length requirement removed.
+It unifies the names up to the length of the shortest, and takes the named from the longest
 for the remainder.
 It can also be considered as padding the shorter of the two given tuples of names
 with trailing wildcards (`:_`).
@@ -190,24 +190,25 @@ This is the type of name combination used for broadcating array operations.
 Where the smaller (dimensionally) array is broadcast against the longer, repeating it for
 all entries in the missing trailing dimensions.
 """
-combine_names_longest(names, ::Tuple{}) = names
-combine_names_longest(::Tuple{}, names) = names
-combine_names_longest(::Tuple{}, ::Tuple{}) = tuple()
-function combine_names_longest(a_names, b_names)
-    # 1 Allocation: @btime (()-> combine_names_longest((:a,:b), (:a,)))()
-
-    length(a_names) == length(b_names) && return combine_names(a_names, b_names)
-    long, short = length(a_names) > length(b_names) ? (a_names, b_names) : (b_names, a_names)
+unify_names_longest(names, ::Tuple{}) = names
+unify_names_longest(::Tuple{}, names) = names
+unify_names_longest(::Tuple{}, ::Tuple{}) = tuple()
+function unify_names_longest(names_a, names_b)
+    # 0 Allocations: @btime (()-> unify_names_longest((:a,:b), (:a,)))()
+
+    length(names_a) == length(names_b) && return unify_names(names_a, names_b)
+    long, short = length(names_a) > length(names_b) ? (names_a, names_b) : (names_b, names_a)
     short_names = identity_namedtuple(short)
-    return ntuple(length(long)) do ii
+    ret = ntuple(length(long)) do ii
         a = getfield(long, ii)
         b = get(short_names, ii, :_)
         a === :_ && return b
         b === :_ && return a
         a === b && return a
-
-        incompatible_dimension_error(a_names, b_names)
+        return false  # mismatch occured, we mark this with a nonSymbol result
     end
+    ret isa Tuple{Vararg{Symbol}} || incompatible_dimension_error(names_a, names_b)
+    return compile_time_return_hack(ret)
 end
 
 
diff --git a/src/wrapper_array.jl b/src/wrapper_array.jl
index addb364..f7f4124 100644
--- a/src/wrapper_array.jl
+++ b/src/wrapper_array.jl
@@ -53,7 +53,7 @@ NamedDimsArray(orig::AbstractVector, name::Symbol) = NamedDimsArray(orig, (name,
 # Name-asserting constructor (like renaming, but only for wildcards (`:_`).)
 NamedDimsArray{L}(orig::NamedDimsArray{L}) where L = orig
 function NamedDimsArray{L}(orig::NamedDimsArray{old_names, T, N, A}) where {L, old_names, T, N, A}
-    new_names = combine_names(L, old_names)
+    new_names = unify_names(L, old_names)
     return NamedDimsArray{new_names, T, N, A}(parent(orig))
 end
 


Test patch:
diff --git a/test/name_core.jl b/test/name_core.jl
index 0aba361..05cc89b 100644
--- a/test/name_core.jl
+++ b/test/name_core.jl
@@ -1,8 +1,8 @@
 using NamedDims
 using NamedDims:
     names,
-    combine_names,
-    combine_names_longest,
+    unify_names,
+    unify_names_longest,
     order_named_inds,
     permute_dimnames,
     remaining_dimnames_from_indexing,
@@ -32,28 +32,28 @@ using Test
     end
 end
 
-@testset "combine_names/combine_names_longest" begin
-    @test_throws DimensionMismatch combine_names((:a,), (:a, :b,))
+ @testset "unify_names/unify_names_longest" begin
+    @test_throws DimensionMismatch unify_names((:a,), (:a, :b,))
 
-    @test combine_names_longest((:a,), (:a, :b,)) == (:a, :b)
-    @test combine_names_longest((:a,), (:a, :_)) == (:a, :_)
-    @test combine_names_longest((:a, :b), (:a, :_, :c)) == (:a, :b, :c)
+    @test unify_names_longest((:a,), (:a, :b,)) == (:a, :b)
+    @test unify_names_longest((:a,), (:a, :_)) == (:a, :_)
+    @test unify_names_longest((:a, :b), (:a, :_, :c)) == (:a, :b, :c)
 
-    @test_throws DimensionMismatch combine_names_longest((:a, :b, :c), (:b, :a))
+    @test_throws DimensionMismatch unify_names_longest((:a, :b, :c), (:b, :a))
 
-    for combine in (combine_names, combine_names_longest)
-        @test combine((:a,), (:a,)) == (:a,)
-        @test combine((:a, :b), (:a, :b)) == (:a, :b)
-        @test combine((:a, :_), (:a, :b)) == (:a, :b)
-        @test combine((:a, :_), (:a, :_)) == (:a, :_)
+    for unify in (unify_names, unify_names_longest)
+        @test unify((:a,), (:a,)) == (:a,)
+        @test unify((:a, :b), (:a, :b)) == (:a, :b)
+        @test unify((:a, :_), (:a, :b)) == (:a, :b)
+        @test unify((:a, :_), (:a, :_)) == (:a, :_)
 
-        @test combine((:a, :b, :c), (:_, :_, :_)) == (:a, :b, :c)
-        @test combine((:a, :_, :c), (:_, :b, :_)) == (:a, :b, :c)
-        @test combine((:_, :_, :_), (:_, :_, :_)) == (:_, :_, :_)
+        @test unify((:a, :b, :c), (:_, :_, :_)) == (:a, :b, :c)
+        @test unify((:a, :_, :c), (:_, :b, :_)) == (:a, :b, :c)
+        @test unify((:_, :_, :_), (:_, :_, :_)) == (:_, :_, :_)
 
-        @test_throws DimensionMismatch combine((:a,), (:b,))
-        @test_throws DimensionMismatch combine((:a,:b), (:b, :a))
-        @test_throws DimensionMismatch combine((:a, :b, :c), (:_, :_, :d))
+        @test_throws DimensionMismatch unify((:a,), (:b,))
+        @test_throws DimensionMismatch unify((:a,:b), (:b, :a))
+        @test_throws DimensionMismatch unify((:a, :b, :c), (:_, :_, :d))
     end
 end