Repository: invenia/NamedDims.jl
Pull request #84 (2019-11-23 11:44:55)

Make keyword indexing fast
This solves the `order_named_inds` problem by just making it generated, I fiddled around but couldn’t make other ways work. Not sure whether the body should then live in src/name_core.jl (as now) or move to src/wrapper_array.jl (where the function of this signature used to live).

Before:
```
julia> nda = NamedDimsArray(rand(2,2), (:x, :y));

julia> @btime $nda[1, 2];
  1.421 ns (0 allocations: 0 bytes)

julia> @btime $nda[x=1, y=2];
  40.455 ns (2 allocations: 64 bytes)
```
After:
```
julia> @btime $nda[x=1, y=2]
  1.420 ns (0 allocations: 0 bytes)
```

Edit: Note that this was the original motivation for #76. Which included a partial fix to get this down from 200ns. 

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

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

```diff
@@            Coverage Diff             @@
##           master      #84      +/-   ##
==========================================
+ Coverage   88.02%   88.38%   +0.35%     
==========================================
  Files           8        8              
  Lines         284      284              
==========================================
+ Hits          250      251       +1     
+ Misses         34       33       -1
```


| [Impacted Files](https://codecov.io/gh/invenia/NamedDims.jl/pull/84?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/NamedDims.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/84/diff?src=pr&el=tree#diff-c3JjL05hbWVkRGltcy5qbA==) | `100% <ø> (ø)` | :arrow_up: |
| [src/wrapper\_array.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/84/diff?src=pr&el=tree#diff-c3JjL3dyYXBwZXJfYXJyYXkuamw=) | `90% <ø> (-0.25%)` | :arrow_down: |
| [src/name\_core.jl](https://codecov.io/gh/invenia/NamedDims.jl/pull/84/diff?src=pr&el=tree#diff-c3JjL25hbWVfY29yZS5qbA==) | `91.01% <100%> (+1.23%)` | :arrow_up: |

------

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


Patch:
diff --git a/src/name_core.jl b/src/name_core.jl
index 7460f1c..c2f8cb5 100644
--- a/src/name_core.jl
+++ b/src/name_core.jl
@@ -111,29 +111,32 @@ Base.@pure function tuple_issubset(lhs::Tuple{Vararg{Symbol,N}}, rhs::Tuple{Vara
     return true
 end
 
-"""
-    order_named_inds(dimnames::Tuple; named_inds...)
 
-Returns the values of the `named_inds`, sorted as per the order they appear in `dimnames`,
-with any missing dimnames, having there value set to `:`.
-An error is thrown if any dimnames are given in `named_inds` that do not occur in `dimnames`.
 """
-order_named_inds(dimnames::Tuple; named_inds...) = order_named_inds(dimnames, named_inds.data)
-
-function order_named_inds(dimnames::Tuple{Vararg{Symbol,N}}, named_inds::NamedTuple) where {N}
-    if !tuple_issubset(keys(named_inds), dimnames)
-        throw(DimensionMismatch("Expected subset of $(dimnames), got $(keys(named_inds))"))
-    end
+    order_named_inds(Val(names); kw...)
+    order_named_inds(Val(names), namedtuple)
 
-    full_inds = ntuple(N) do ii
-        name_ii = dimnames[ii]
-        get(named_inds, name_ii, :)
+Returns the tuple of index values for an array with `names`, when indexed by keywords.
+Any dimensions not fixed are given as `:`, to make a slice.
+An error is thrown if any keywords are used which do not occur in `nda`'s names.
+"""
+order_named_inds(val::Val{L}; kw...) where {L} = order_named_inds(val, kw.data)
+
+@generated function order_named_inds(val::Val{L}, ni::NamedTuple{K}) where {L,K}
+    tuple_issubset(K, L) || throw(DimensionMismatch("Expected subset of $L, got $K"))
+    exs = map(L) do n
+        if Base.sym_in(n, K)
+            qn = QuoteNode(n)
+            :(getfield(ni, $qn))
+        else
+            :(Colon())
+        end
     end
-
-    return full_inds
+    return Expr(:tuple, exs...)
 end
 
 
+
 """
     incompatible_dimension_error(names_a, names_b)
 Throws a `DimensionMismatch` explaining that these dimension names are not compatible.
diff --git a/src/wrapper_array.jl b/src/wrapper_array.jl
index c4404b3..afafbc4 100644
--- a/src/wrapper_array.jl
+++ b/src/wrapper_array.jl
@@ -135,12 +135,11 @@ function Base.similar(
     return similar(a, eltype, dims)
 end
 
-
 ###############################
 # kwargs indexing
 
 """
-    order_named_inds(A, named_inds...)
+    order_named_inds(A; named_inds...)
 
 Returns the indices that have the names and values given by `named_inds`
 sorted into the order expected for the dimension of the array `A`.
@@ -157,7 +156,9 @@ order_named_inds(A; y=5) == (:, 5)
 
 This provides the core indexed lookup for `getindex` and `setindex` on the Array `A`
 """
-order_named_inds(A::AbstractArray; named_inds...) = order_named_inds(dimnames(A); named_inds...)
+function order_named_inds(A::NamedDimsArray{L}; named_inds...) where {L}
+    return order_named_inds(Val{L}(); named_inds...)
+end
 
 ###################
 # getindex / view / dotview


Test patch:
diff --git a/test/name_core.jl b/test/name_core.jl
index fae4be3..74ee78d 100644
--- a/test/name_core.jl
+++ b/test/name_core.jl
@@ -93,23 +93,18 @@ end
 
 
 @testset "order_named_inds" begin
-    @test order_named_inds((:x,)) == (:,)
-    @test order_named_inds((:x,); x=2) == (2,)
+    @test order_named_inds(Val((:x,))) == (:,)
+    @test order_named_inds(Val((:x,)); x=2) == (2,)
 
-    @test order_named_inds((:x, :y,)) == (:, :)
-    @test order_named_inds((:x, :y); x=2) == (2, :)
-    @test order_named_inds((:x, :y); y=2, ) == (:, 2)
-    @test order_named_inds((:x, :y); y=20, x=30) == (30, 20)
-    @test order_named_inds((:x, :y); x=30, y=20) == (30, 20)
+    @test order_named_inds(Val((:x, :y))) == (:, :)
+    @test order_named_inds(Val((:x, :y)); x=2) == (2, :)
+    @test order_named_inds(Val((:x, :y)); y=2, ) == (:, 2)
+    @test order_named_inds(Val((:x, :y)); y=20, x=30) == (30, 20)
+    @test order_named_inds(Val((:x, :y)); x=30, y=20) == (30, 20)
 end
 @testset "allocations: order_named_inds" begin
-    if v"1.1-" <= VERSION <= v"1.2-" # test passes on 1.1, including 1.1.1
-        @test 0 == @allocated (()->order_named_inds((:a, :b, :c); b=1, c=2))()
-        @test 0 == @allocated (()->order_named_inds((:a, :b, :c), (b=1, c=2)))()
-    else
-        @test_broken 0 == @allocated (()->order_named_inds((:a, :b, :c); b=1, c=2))()
-        @test_broken 0 == @allocated (()->order_named_inds((:a, :b, :c), (b=1, c=2)))()
-    end
+    @test 0 == @allocated (()->order_named_inds(Val((:a, :b, :c)); b=1, c=2))()
+    @test 0 == @allocated (()->order_named_inds(Val((:a, :b, :c)), (b=1, c=2)))()
 end
 
 
diff --git a/test/wrapper_array.jl b/test/wrapper_array.jl
index e05c77e..bf56e52 100644
--- a/test/wrapper_array.jl
+++ b/test/wrapper_array.jl
@@ -185,13 +185,8 @@ const cnda = NamedDimsArray([10 20; 30 40], (:x, :y))
     # indexing
     @test 0 == @allocated cnda[1,1]
     @test 0 == @allocated cnda[1,1] = 55
-    if v"1.1-" <= VERSION <= v"1.2-" # tests pass on 1.1, including 1.1.1
-        @test 0 == @allocated cnda[x=1,y=1]
-        @test @allocated(cnda[x=1]) == @allocated(cnda[1,:])
-        @test 0 == @allocated cnda[x=1,y=1] = 66
-    else
-        @test_broken 0 == @allocated cnda[x=1,y=1]
-        @test_broken @allocated(cnda[x=1]) == @allocated(cnda[1,:])
-        @test_broken 0 == @allocated cnda[x=1,y=1] = 66
-    end
+
+    @test 0 == @allocated cnda[x=1, y=1]
+    @test @allocated(cnda[x=1]) == @allocated(cnda[1, :])
+    @test 0 == @allocated cnda[x=1, y=1] = 66
 end