Repository: invenia/NamedDims.jl
Pull request #13 (2019-04-24 09:23:24)

Add renaming, and related constructor

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

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

```diff
@@            Coverage Diff             @@
##           master      #13      +/-   ##
==========================================
- Coverage   93.49%   92.12%   -1.37%     
==========================================
  Files           6        6              
  Lines         123      127       +4     
==========================================
+ Hits          115      117       +2     
- Misses          8       10       +2
```


| [Impacted Files](https://codecov.io/gh/invenia/NamedDims.jl/pull/13?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/wrapper\_array.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/13/diff?src=pr&el=tree#diff-c3JjL3dyYXBwZXJfYXJyYXkuamw=) | `81.25% <50%> (-4.47%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/invenia/NamedDims.jl/pull/13?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/13?src=pr&el=footer). Last update [7efc8be...d84eac7](https://codecov.io/gh/invenia/NamedDims.jl/pull/13?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 2e57046..f62765b 100644
--- a/README.md
+++ b/README.md
@@ -9,11 +9,13 @@
 `NamedDimsArray` is a zero-cost abstraction to add names to the dimensions of an array.
 
 ## Core functionality:
+
 For `nda = NamedDimsArrays{(:x,:y,:z)}(rand(10,20,30))`.
 
  - Unwrapping: `parent(nda)`: returns the underlying `AbstractArray` that is wrapped by the `NamedDimsArray`
  - Indexing: `nda[y=2]`: the same as `nda[x=:, y=2, z=:]` which is the same as `nda[:,2,:]`
  - Functions taking a dims arg: `sum(nda; dims=:y)` is the same as `sum(nda; dims=2)`
+ - Renaming: `rename(nda, new_names)` returns a new `NamedDimsArray` with the `new_names` but still wrapping the same data.
 
 ### Dimensionally Safe Operations
 
@@ -48,6 +50,39 @@ Currently, if you have more than one wildcard dimension name,
 functionality for referring to dimensions by name will not work.
 See [issue #8](https://github.com/invenia/NamedDims.jl/issues/8).
 
+## Usage
+### Writing functions that accept `NamedDimsArray`s or `AbstractArray`s
+
+It is a common desire to be able to write code that anyone can call,
+whether they are using `NamedDimsArray`s or not.
+While also being able to use `NamedDimsArray`s internally in its definition;
+and also getting the assertion when a `NamedDimsArray` _is_  passed in, that it has the
+expected dimensions.
+The way to do this is to call the `NamedDimsArray` constructor, with the expected names
+within the function.
+As in the following example:
+
+```
+function total_variance(data::AbstractMatrix)
+    n_data = NamedDimsArray(data, (:times,:locations))
+    location_variance = var(n_data; dims=:times)  # calculate variance at each location
+    return sum(location_variance; dims=:locations)  # total them
+end
+```
+
+
+If this function is given (say) a `Matrix`, then it will apply the names to it in `n_data`.
+Thus the function will just work on unnamed types.
+If `data` is a `NamedDimsArray`, with incompatible names an error will be thrown.
+For example if it `data` was mistakenly transposed and so had the dimension names:
+`(:locations, :times)` instead of `(:times, :locations)`.
+If `data` was partially named, e.g. `(:_, :locations)`, then that name would be allowed to be
+combined with the named from the constructor; yielding `n_data` with the expected names:
+`(:times, :locations)`.
+This pattern allows both assertions of correctness (for named inputs),
+and convenience and compatibility (for unnamed input).
+And since `NamedDimsArray` is a zero-cost abstraction, this will basically compile out of existence,
+most of the time.
 
 ## Extending support for more functions
 There are two common things to do to make a function support `NamedDimsArray`s.
diff --git a/src/NamedDims.jl b/src/NamedDims.jl
index 25c102c..a50fdd8 100644
--- a/src/NamedDims.jl
+++ b/src/NamedDims.jl
@@ -3,7 +3,7 @@ using Base: @propagate_inbounds
 using LinearAlgebra
 using Statistics
 
-export NamedDimsArray, dim
+export NamedDimsArray, dim, rename
 
 include("name_core.jl")
 include("wrapper_array.jl")
diff --git a/src/functions_dims.jl b/src/functions_dims.jl
index 34614ea..169e31c 100644
--- a/src/functions_dims.jl
+++ b/src/functions_dims.jl
@@ -1,5 +1,15 @@
 # This file is for functions that explictly mess with the dimensions of a NameDimsArray
 
+"""
+    rename(nda::NamedDimsArray, names)
+
+Returns a new `NameDimsArray` with the given dimension `names`.
+`rename` outright replaces the names; while still wrapping the same backing array.
+Unlike the constructor, it does not require that new names are compatible
+with the old names (though you do still need to match the number of dimensions).
+"""
+rename(nda::NamedDimsArray, names) = NamedDimsArray(parent(nda), names)
+
 function Base.dropdims(nda::NamedDimsArray; dims)
     numerical_dims = dim(nda, dims)
     data = dropdims(parent(nda); dims=numerical_dims)
diff --git a/src/name_core.jl b/src/name_core.jl
index fa35271..a3d99c9 100644
--- a/src/name_core.jl
+++ b/src/name_core.jl
@@ -128,6 +128,14 @@ function order_named_inds(dimnames::Tuple; named_inds...)
     return inds
 end
 
+"""
+    incompatible_dimension_error(names_a, names_b)
+Throws a `DimensionMismatch` explaining that these dimension names are not compatible.
+"""
+function incompatible_dimension_error(names_a, names_b)
+    throw(DimensionMismatch("Incompatible dimension names: $names_a \neq $names_b"))
+end
+
 """
     combine_names(a, b)
 
@@ -155,8 +163,7 @@ function combine_names(names_a, names_b)
 
     # Error message should not include names until it is thrown, as othrwise
     # the interpolation allocates and slows everything down a lot.
-    err_msg = "Attempted to combine arrays with incompatible dimension names. "
-    length(names_a) != length(names_b) && throw(DimensionMismatch(err_msg * "$names_a ≠ $names_b."))
+    length(names_a) != length(names_b) && incompatible_dimension_error(names_a, names_b)
 
     ret = ntuple(length(names_a)) do ii  # remove :_ wildcards
         a = getfield(names_a, ii)
@@ -167,7 +174,7 @@ function combine_names(names_a, names_b)
 
         return false  # mismatch occured, we mark this with a nonSymbol result
     end
-    ret isa Tuple{Vararg{Symbol}} || throw(DimensionMismatch(err_msg * "$names_a ≠ $names_b."))
+    ret isa Tuple{Vararg{Symbol}} || incompatible_dimension_error(names_a, names_b)
     return compile_time_return_hack(ret)
 end
 
@@ -199,8 +206,7 @@ function combine_names_longest(a_names, b_names)
         b === :_ && return a
         a === b && return a
 
-        err_msg = "Attempted to combine arrays with incompatible dimension names. "
-        throw(DimensionMismatch(err_msg * "$a_names ≠ $b_names."))
+        incompatible_dimension_error(a_names, b_names)
     end
 end
 
diff --git a/src/wrapper_array.jl b/src/wrapper_array.jl
index 89df970..addb364 100644
--- a/src/wrapper_array.jl
+++ b/src/wrapper_array.jl
@@ -19,6 +19,16 @@ x = x[observations=15, features=2]  # 2nd feature in 15th observation.
 `NamedDimsArray`s are normally a (near) zero-cost abstraction.
 They generally resolve most dimension name related operations at compile
 time.
+
+
+The `NamedDimsArray` constructor takes a list of names as `Symbol`s,
+one per dimension, and an array to wrap.
+If the array being wrapped is a `NamedDimsArray` already then the new names
+are combined with the existing names -- to replace wildcards (`:_`).
+This will throw an error if the new names are not compatible with the old names
+(i.e. if the nonwildcards do not match).
+To assign new names to a `NamedDimsArray` without regard to compatibility with the old names
+see `rename`(@ref).
 """
 struct NamedDimsArray{L, T, N, A<:AbstractArray{T, N}} <: AbstractArray{T, N}
     # `L` is for labels, it should be an `NTuple{N, Symbol}`
@@ -34,9 +44,19 @@ function NamedDimsArray{L}(orig::AbstractArray{T, N}) where {L, T, N}
     end
     return NamedDimsArray{L, T, N, typeof(orig)}(orig)
 end
+
 function NamedDimsArray(orig::AbstractArray{T, N}, names::NTuple{N, Symbol}) where {T, N}
     return NamedDimsArray{names}(orig)
 end
+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)
+    return NamedDimsArray{new_names, T, N, A}(parent(orig))
+end
+
 
 parent_type(::Type{<:NamedDimsArray{L, T, N, A}}) where {L, T, N, A} = A
 Base.parent(x::NamedDimsArray) = x.data


Test patch:
diff --git a/test/functions_dims.jl b/test/functions_dims.jl
index b40664f..b8a0884 100644
--- a/test/functions_dims.jl
+++ b/test/functions_dims.jl
@@ -2,6 +2,14 @@ using NamedDims
 using NamedDims: names
 using Test
 
+@testset "rename" begin
+    nda = NamedDimsArray{(:a, :b, :c, :d)}(ones(10,1,1,20))
+    new_nda = rename(nda, (:w, :x, :y, :z))
+
+    @test names(new_nda) === (:w, :x, :y, :z)
+    @test parent(new_nda) === parent(nda)
+end
+
 @testset "dropdims" begin
     nda = NamedDimsArray{(:a, :b, :c, :d)}(ones(10, 1, 1, 20))
 
diff --git a/test/wrapper_array.jl b/test/wrapper_array.jl
index 127ad7c..ade5403 100644
--- a/test/wrapper_array.jl
+++ b/test/wrapper_array.jl
@@ -4,6 +4,7 @@ using SparseArrays
 using Test
 
 
+
 @testset "get the parent array that was wrapped" begin
     orig = [1 2; 3 4]
     @test parent(NamedDimsArray(orig, (:x, :y))) === orig
@@ -15,6 +16,19 @@ end
 end
 
 
+@testset "Name-asserting constructor" begin
+    orig_full = NamedDimsArray(ones(3,4,5), (:a, :b, :c))
+    @test names(NamedDimsArray(orig_full, (:a, :b, :c))) == (:a, :b, :c)
+    @test names(NamedDimsArray(orig_full, (:a, :b, :_))) == (:a, :b, :c)
+    @test_throws DimensionMismatch NamedDimsArray(orig_full, (:a, :b, :wrong))
+    @test_throws DimensionMismatch NamedDimsArray(orig_full, (:c, :a, :b))
+
+    orig_partial = NamedDimsArray(ones(3,4,5), (:a, :_, :c))
+    @test names(NamedDimsArray(orig_partial, (:a, :b, :c))) == (:a, :b, :c)
+    @test names(NamedDimsArray(orig_partial, (:a, :_, :c))) == (:a, :_, :c)
+end
+
+
 @testset "getindex" begin
     nda = NamedDimsArray([10 20; 30 40], (:x, :y))