Repository: invenia/NamedDims.jl
Pull request #69 (2019-10-02 22:18:27)

newaxis
After:
```
julia> const newaxis = [CartesianIndex()];

julia> rand(2,2)[:, newaxis, :] |> size
(2, 1, 2)

julia> NamedDimsArray(rand(2,2), (:a,:b))[:, newaxis, :] |> names
(:a, :_, :b)
```

codecov[bot]: # [Codecov](https://codecov.io/gh/invenia/NamedDims.jl/pull/69?src=pr&el=h1) Report
> Merging [#69](https://codecov.io/gh/invenia/NamedDims.jl/pull/69?src=pr&el=desc) into [master](https://codecov.io/gh/invenia/NamedDims.jl/commit/2f355d7ac11f1e9bee4ecfe35a57eab74363be2a?src=pr&el=desc) will **increase** coverage by `0.17%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master      #69      +/-   ##
==========================================
+ Coverage   86.72%   86.89%   +0.17%     
==========================================
  Files           8        8              
  Lines         226      229       +3     
==========================================
+ Hits          196      199       +3     
  Misses         30       30
```


| [Impacted Files](https://codecov.io/gh/invenia/NamedDims.jl/pull/69?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/name\_core.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/69/diff?src=pr&el=tree#diff-c3JjL25hbWVfY29yZS5qbA==) | `95.38% <100%> (+0.22%)` | :arrow_up: |

------

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

oxinabox: I have not reviewed this yet
mcabbott: No rush. Another test case before I forget… which doesn’t error, but why should it have name `:x` not `:z`, perhaps it should have neither?
```
julia> nda = NamedDimsArray(rand(1:9, 3,3,3), (:x, :y, :z));

julia> nda[rand(3,3,3) .> 0.5]
17-element NamedDimsArray{(:x,),Int64,1,Array{Int64,1}}:
```
oxinabox: > Another test case before I forget… which doesn’t error, but why should it have name :x not :z, perhaps it should have neither?

Yeah, I think it should have neither. Maybe open issue for that one?
oxinabox: Thanks!
Can you bump the patch version so I can tag a release after I merge
oxinabox: Thanks

Patch:
diff --git a/Project.toml b/Project.toml
index 4b133c4..e70f939 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.6"
+version = "0.2.7"
 
 [deps]
 LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
diff --git a/src/name_core.jl b/src/name_core.jl
index f575a61..f93b3b0 100644
--- a/src/name_core.jl
+++ b/src/name_core.jl
@@ -242,7 +242,7 @@ is_noninteger_type(::Type{<:Integer}) = false
 is_noninteger_type(::Any) = true
 
 """
-    remaining_dimnames_from_indexing(dimnames::Tuple, inds...)
+    remaining_dimnames_from_indexing(dimnames::Tuple, inds)
 Given a tuple of dimension names
 and a set of index expressesion e.g `1, :, 1:3, [true, false]`,
 determine which are not dropped.
@@ -251,13 +251,23 @@ Dimensions indexed with scalars are dropped
 @generated function remaining_dimnames_from_indexing(dimnames::Tuple, inds)
     # 0-Allocation see:
     # `@btime (()->remaining_dimnames_from_indexing((:a, :b, :c), (:,390,:)))()``
-    ind_types = inds.parameters
-    kept_dims = findall(is_noninteger_type, ind_types)
-    keep_names = [:(getfield(dimnames, $ii)) for ii in kept_dims]
+    keep_names = []
+    dim_num = 0
+    for type in inds.parameters
+        if type <: Integer
+            dim_num += 1
+        elseif type <: CartesianIndex
+            dim_num += type.parameters[1]
+        elseif type == Array{CartesianIndex{0},1}
+            push!(keep_names, QuoteNode(:_))
+        else
+            dim_num += 1
+            push!(keep_names, :(getfield(dimnames, $dim_num)) )
+        end
+    end
     return Expr(:call, :compile_time_return_hack, Expr(:tuple, keep_names...))
 end
 
-
 """
     remaining_dimnames_after_dropping(dimnames::Tuple, dropped_dims)
 Given a tuple of dimension names, and either a collection of dimensions,


Test patch:
diff --git a/test/name_core.jl b/test/name_core.jl
index b896042..8e37400 100644
--- a/test/name_core.jl
+++ b/test/name_core.jl
@@ -69,8 +69,11 @@ end
     @test remaining_dimnames_from_indexing((:a, :b, :c), (10,20,30)) == tuple()
     @test remaining_dimnames_from_indexing((:a, :b, :c), (10, :,30)) == (:b,)
     @test remaining_dimnames_from_indexing((:a, :b, :c), (1:1, [true], [20])) == (:a, :b, :c)
-end
 
+    @test remaining_dimnames_from_indexing((:a, :b, :c), (:, [CartesianIndex()], :, :)) == (:a, :_, :b, :c)
+    @test remaining_dimnames_from_indexing((:a, :b, :c), (1, [CartesianIndex()], 2, :)) == (:_, :c)
+    @test remaining_dimnames_from_indexing((:a, :b, :c), (CartesianIndex(1,1), :)) == (:c,)
+end
 
 @testset "remaining_dimnames_after_dropping" begin
     @test remaining_dimnames_after_dropping((:a, :b, :c), 1) == (:b, :c)
diff --git a/test/wrapper_array.jl b/test/wrapper_array.jl
index 5712981..d85cfa6 100644
--- a/test/wrapper_array.jl
+++ b/test/wrapper_array.jl
@@ -20,13 +20,13 @@ end
 
 
 @testset "Name-asserting constructor" begin
-    orig_full = NamedDimsArray(ones(3,4,5), (:a, :b, :c))
+    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))
+    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
@@ -35,33 +35,50 @@ end
 @testset "getindex" begin
     nda = NamedDimsArray([10 20; 30 40], (:x, :y))
 
-    @test nda[x=1, y=1] == nda[y=1, x=1] == nda[1, 1] == nda[CartesianIndex(1,1)] == 10
-    @test nda[y=end,x=end] == nda[end, end] == 40
+    @test nda[x=1, y=1] == nda[y=1, x=1] == nda[1, 1] == nda[CartesianIndex(1, 1)] == 10
+    @test nda[y=end, x=end] == nda[end, end] == 40
 
     # Unspecified dims become slices
     @test nda[y=1] == nda[y=1, x=:] == nda[:, 1] == [10; 30]
 
+    @test_broken nda[CartesianIndex(1), 1] == nda[1, 1]
+
     @testset "name preservation" begin
-        @test names(nda[y=1]) == (:x,)
+        @test names(nda[y=1]) == (:x, )
         @test names(nda[y=1:1]) == (:x, :y)
     end
 
     # https://github.com/invenia/NamedDims.jl/issues/8
     @testset "with multiple-wildcards" begin
-        nda_mw = NamedDimsArray{(:_,:_,:c)}(ones(10,20,30));
-        @test nda_mw[c=2] == ones(10,20)
+        nda_mw = NamedDimsArray{(:_, :_, :c)}(ones(10, 20, 30));
+        @test nda_mw[c=2] == ones(10, 20)
         @test names(nda_mw[c=2]) == (:_, :_)
     end
+
+    @testset "newaxis" begin
+        newaxis = [CartesianIndex()];
+
+        @test names(nda[:, newaxis, :]) == (:x, :_, :y)
+        @test size(nda[:, newaxis, :]) == (2, 1, 2)
+
+        @test names(nda[1, newaxis, 1]) == (:_, )
+        @test size(nda[1, newaxis, 1]) == (1, )
+
+        @test names(nda[CartesianIndex(1, 1), newaxis]) == (:_, )
+        @test size(nda[CartesianIndex(1, 1), newaxis]) == (1, )
+        @test nda[CartesianIndex(1, 1), newaxis][1] == nda[1, 1]
+    end
 end
 
 
+
 @testset "views" begin
     nda = NamedDimsArray([10 20; 30 40], (:x, :y))
 
     @test @view(nda[y=1]) == @view(nda[y=1, x=:]) == @view(nda[:, 1]) == [10; 30]
 
     @testset "name preservation" begin
-        @test names(nda[y=1]) == (:x,)
+        @test names(nda[y=1]) == (:x, )
         @test names(nda[y=1:1]) == (:x, :y)
     end
 end
@@ -83,7 +100,7 @@ end
         nda[1, 1] = 100
         @test nda == [100 20; 30 40]
 
-        nda[1,:] .= 1000
+        nda[1, :] .= 1000
         @test nda == [1000 1000; 30 40]
     end
 end
@@ -92,7 +109,7 @@ end
     nda = NamedDimsArray([10 20; 30 40], (:x, :y))
     @test IndexStyle(typeof(nda)) == IndexLinear()
 
-    sparse_nda = NamedDimsArray(spzeros(4,2), (:x, :y))
+    sparse_nda = NamedDimsArray(spzeros(4, 2), (:x, :y))
     @test IndexStyle(typeof(sparse_nda)) == IndexCartesian()
 end
 
@@ -103,10 +120,10 @@ end
     @test length(nda) == 6
 
     @test axes(nda) == (1:3, 1:2)
-    @test axes(nda,:x) == (1:3) == axes(nda, 1)
+    @test axes(nda, :x) == (1:3) == axes(nda, 1)
 
-    @test size(nda) == (3,2)
-    @test size(nda,:x) == 3 == size(nda, 1)
+    @test size(nda) == (3, 2)
+    @test size(nda, :x) == 3 == size(nda, 1)
 end
 
 @testset "similar" begin
@@ -116,7 +133,7 @@ end
         ndb = similar(nda)
         @test parent(ndb) !== parent(nda)
         @test eltype(ndb) == Float64
-        @test size(ndb) == (10, 20 ,30, 40)
+        @test size(ndb) == (10, 20 , 30, 40)
         @test names(ndb) == (:a, :b, :c, :d)
     end
     @testset "eltype" begin
@@ -145,8 +162,8 @@ end
         ndb = similar(nda, Float64, (w=11, x=22))
         @test parent(ndb) !== parent(nda)
         @test eltype(ndb) == Float64
-        @test size(ndb) == (11,22)
-        @test names(ndb) == (:w, :x,)
+        @test size(ndb) == (11, 22)
+        @test names(ndb) == (:w, :x)
     end
 end