tests and fixes for lookups and dimensions

src/DimensionalData.jl

@@ -38,7 +38,7 @@ using .Dimensions.Lookups
 using .Dimensions: StandardIndices, DimOrDimType, DimTuple, DimTupleOrEmpty, DimType, AllDims
 import .Lookups: metadata, set, _set, rebuild, basetypeof, 
     order, span, sampling, locus, val, index, bounds, intervalbounds,
-    hasselection, units, SelectorOrInterval
+    hasselection, units, SelectorOrInterval, Begin, End
 import .Dimensions: dims, refdims, name, lookup, kw2dims, hasdim, label, _astuple
 
 import DataAPI.groupby
@@ -56,6 +56,8 @@ export X, Y, Z, Ti, Dim, Coord
 # Selector
 export At, Between, Touches, Contains, Near, Where, All, .., Not, Bins, CyclicBins
 
+export Begin, End
+
 export AbstractDimArray, DimArray
 
 export AbstractDimVector, AbstractDimMatrix, AbstractDimVecOrMat, DimVector, DimMatrix, DimVecOrMat

src/Dimensions/indexing.jl

@@ -14,9 +14,9 @@ for f in (:getindex, :view, :dotview)
         @propagate_inbounds function Base.$f(d::Dimension{<:AbstractArray}, i::SelectorOrInterval)
             Base.$f(d, selectindices(val(d), i))
         end
-        # Everything else (like custom indexing from other packages) passes through to the parent
         @propagate_inbounds function Base.$f(d::Dimension{<:AbstractArray}, i)
-            Base.$f(parent(d), i)
+            x = Base.$f(parent(d), i)
+            x isa AbstractArray ? rebuild(d, x) : x
         end
     end
 end

src/Lookups/Lookups.jl

@@ -63,11 +63,11 @@ include("lookup_traits.jl")
 include("lookup_arrays.jl")
 include("predicates.jl")
 include("selector.jl")
+include("beginend.jl")
 include("indexing.jl")
 include("methods.jl")
 include("utils.jl")
 include("set.jl")
-include("beginend.jl")
 include("show.jl")
 
 end

src/Lookups/indexing.jl

@@ -9,7 +9,12 @@ for f in (:getindex, :view, :dotview)
             rebuild(l; data=Base.$f(parent(l), i))
         # Selector gets processed with `selectindices`
         @propagate_inbounds Base.$f(l::Lookup, i::SelectorOrInterval) = Base.$f(l, selectindices(l, i))
-        # Everything else (like custom indexing from other packages) passes through to the parent
-        @propagate_inbounds Base.$f(l::Lookup, i) = Base.$f(parent(l), i)
+        @propagate_inbounds function Base.$f(l::Lookup, i)
+            x = Base.$f(parent(l), i)
+            x isa AbstractArray ? rebuild(l; data=x) : x
+        end
+        @propagate_inbounds function Base.$f(l::Lookup, i::AbstractBeginEndRange)
+            l[Base.to_indices(l, (i,))...]
+        end
     end
 end

src/Lookups/lookup_traits.jl

@@ -72,7 +72,7 @@ isrev(::Type{<:ForwardOrdered}) = false
 isrev(::Type{<:ReverseOrdered}) = true
 
 """
-   Locus <: LookupTrait
+   Position <: LookupTrait
 
 Abstract supertype of types that indicate the position of index values
 where they represent [`Intervals`](@ref).
@@ -83,49 +83,63 @@ These allow for values array cells to align with the [`Start`](@ref),
 This means they can be plotted with correct axis markers, and allows automatic
 converrsions to between formats with different standards (such as NetCDF and GeoTiff).
 """
-abstract type Locus <: LookupTrait end
+abstract type Position <: LookupTrait end
 
 """
-    Center <: Locus
+    Center <: Position
 
     Center()
 
-Indicates a lookup value is for the center of its corresponding array cell.
+Used to specify lookup values correspond to the center position in an interval.
 """
-struct Center <: Locus end
+struct Center <: Position end
 
 """
-    Begin <: Locus
+    Start <: Position
 
-    Begin()
+    Start()
+
+Used to specify lookup values correspond to the center 
+position of an interval.
+"""
+struct Start <: Position end
 
-Indicates a lookup value is for the start of its corresponding array cell,
-in the direction of the lookup index order.
 """
-struct Begin <: Locus end
+    Begin <: Position
 
-const Start = Begin
+    Begin()
 
+Used to specify the `begin` index of a `Dimension` axis. 
+as regular `begin` will not work with named dimensions.
 """
-    End <: Locus
+struct Begin <: Position end
+
+"""
+    End <: Position
 
     End()
 
-Indicates a lookup value is for the end of its corresponding array cell,
-in the direction of the lookup index order.
+Used to specify the `end` index of aa `Dimension` axis, 
+as regular `end` will not work with named dimensions.
+
+Also ysed to specify lookup values correspond to the center 
+position of an interval.
 """
-struct End <: Locus end
+struct End <: Position end
 
 """
-    AutoLocus <: Locus
+    AutoPosition <: Position
 
-    AutoLocus()
+    AutoPosition()
 
 Indicates a interval where the index position is not yet known.
 This will be filled with a default value on object construction.
 """
-struct AutoLocus <: Locus end
+struct AutoPosition <: Position end
 
+# Deprecated
+const Locus = Union{AutoPosition,Start,Center,End}
+const AutoLocus = AutoPosition
 
 """
     Sampling <: LookupTrait
@@ -139,7 +153,7 @@ struct NoSampling <: Sampling end
 locus(sampling::NoSampling) = Center()
 
 struct AutoSampling <: Sampling end
-locus(sampling::AutoSampling) = AutoLocus()
+locus(sampling::AutoSampling) = AutoPosition()
 
 """
     Points <: Sampling
@@ -168,7 +182,7 @@ Intervals require a [`Locus`](@ref) of [`Start`](@ref), [`Center`](@ref) or
 struct Intervals{L} <: Sampling
     locus::L
 end
-Intervals() = Intervals(AutoLocus())
+Intervals() = Intervals(AutoPosition())
 
 locus(sampling::Intervals) = sampling.locus
 rebuild(::Intervals, locus) = Intervals(locus)

src/Lookups/selector.jl

@@ -77,7 +77,10 @@ function selectindices(l::Lookup, sel::Not; kw...)
     indices = selectindices(l, sel.skip; kw...)
     return first(to_indices(l, (Not(indices),)))
 end
-selectindices(l::Lookup, i; kw...) = first(Base.to_indices(l, (i,)))
+@inline function selectindices(l::Lookup, sel; kw...)
+    selstr = sprint(show, sel)
+    throw(ArgumentError("Invalid index `$selstr`. Did you mean `At($selstr)`? Use stardard indices, `Selector`s, or `Val` for compile-time `At`."))
+end
 
 """
     At <: IntSelector
@@ -1043,10 +1046,6 @@ function selectindices end
 # @inline selectindices(dim::Lookup, sel::Val) = selectindices(val(dim), At(sel))
 # Standard indices are just returned.
 @inline selectindices(::Lookup, sel::StandardIndices) = sel
-@inline function selectindices(l::Lookup, sel)
-    selstr = sprint(show, sel)
-    throw(ArgumentError("Invalid index `$selstr`. Did you mean `At($selstr)`? Use stardard indices, `Selector`s, or `Val` for compile-time `At`."))
-end
 # Vectors are mapped
 @inline selectindices(lookup::Lookup, sel::Selector{<:AbstractVector}) =
     [selectindices(lookup, rebuild(sel; val=v)) for v in val(sel)]

src/array/indexing.jl

@@ -13,17 +13,13 @@ const SelectorOrStandard = Union{SelectorOrInterval,StandardIndices}
 const DimensionIndsArrays = Union{AbstractArray{<:Dimension},AbstractArray{<:DimTuple}}
 const DimensionalIndices = Union{DimTuple,DimIndices,DimSelectors,Dimension,DimensionIndsArrays}
 const _DimIndicesAmb = Union{AbstractArray{Union{}},DimIndices{<:Integer},DimSelectors{<:Integer}}
+const _DimIndicesAmb2 = Union{Tuple{Dimension,Vararg{Dimension}},AbstractArray{<:Dimension},AbstractArray{<:Tuple{Dimension,Vararg{Dimension}}},DimIndices,DimSelectors,Dimension}
 
 for f in (:getindex, :view, :dotview)
     _f = Symbol(:_, f)
     @eval begin
         if Base.$f === Base.view
-            @eval @propagate_inbounds function Base.$f(A::AbstractDimArray, i::Union{CartesianIndex,CartesianIndices})
-                x = Base.$f(parent(A), i)
-                I = to_indices(A, (i,))
-                rebuildsliced(Base.$f, A, x, I)
-            end
-            @eval @propagate_inbounds function Base.$f(A::AbstractDimArray, i::Integer)
+            @eval @propagate_inbounds function Base.$f(A::AbstractDimVector, i::Integer)
                 x = Base.$f(parent(A), i)
                 I = to_indices(A, (CartesianIndices(A)[i],))
                 rebuildsliced(Base.$f, A, x, I)
@@ -32,23 +28,20 @@ for f in (:getindex, :view, :dotview)
             #### Array getindex/view ###
             # These are needed to resolve ambiguity
             @propagate_inbounds Base.$f(A::AbstractDimArray, i::Integer) = Base.$f(parent(A), i)
-            @propagate_inbounds Base.$f(A::AbstractDimArray, i::CartesianIndex) = Base.$f(parent(A), i)
-            # CartesianIndices
-            @propagate_inbounds Base.$f(A::AbstractDimArray, I::CartesianIndices) =
-                Base.$f(A, to_indices(A, (I,))...)
+            @propagate_inbounds Base.$f(A::AbstractDimVector, i::Integer) = Base.$f(parent(A), i)
         end
-        @propagate_inbounds function Base.$f(A::AbstractDimVector, i)
-            x = Base.$f(parent(A), i)
+        # @propagate_inbounds function Base.$f(A::AbstractDimVector, i)
+        #     x = Base.$f(parent(A), i)
+        #     if $f != view || x isa AbstractArray
+        #         rebuildsliced(Base.$f, A, x, i)
+        #     else
+        #         x
+        #     end
+        # end
+        @propagate_inbounds function Base.$f(A::AbstractDimArray, i1, I...)
+            x = Base.$f(parent(A), i1, I...)
             if $f != view || x isa AbstractArray
-                rebuildsliced(Base.$f, A, x, i)
-            else
-                x
-            end
-        end
-        @propagate_inbounds function Base.$f(A::AbstractDimArray, i1, i2, I...)
-            x = Base.$f(parent(A), i1, i2, I...)
-            if $f != view || x isa AbstractArray
-                rebuildsliced(Base.$f, A, x, to_indices(A, (i1, i2, I...)))
+                rebuildsliced(Base.$f, A, x, to_indices(A, (i1, I...)))
             else
                 x
             end
@@ -63,6 +56,8 @@ for f in (:getindex, :view, :dotview)
         @propagate_inbounds Base.$f(A::AbstractDimArray, i1::SelectorOrStandard, I::SelectorOrStandard...) =
             Base.$f(A, dims2indices(A, (i1, I...))...)
 
+        @propagate_inbounds Base.$f(A::AbstractDimArray, extent::Extents.Extent) =
+            Base.$f(A, dims2indices(A, extent)...)
         @propagate_inbounds Base.$f(A::AbstractBasicDimArray, extent::Extents.Extent) =
             Base.$f(A, dims2indices(A, extent)...)
         # All Dimension indexing modes combined
@@ -74,7 +69,10 @@ for f in (:getindex, :view, :dotview)
         @propagate_inbounds Base.$f(A::AbstractDimVector, i::DimIndices) = $_f(A, i)
         @propagate_inbounds Base.$f(A::AbstractDimVector, i::DimSelectors) = $_f(A, i)
         @propagate_inbounds Base.$f(A::AbstractDimVector, i::_DimIndicesAmb) = $_f(A, i)
-        @propagate_inbounds Base.$f(A::AbstractDimArray, i1::_DimIndicesAmb, I::_DimIndicesAmb...) = $_f(A, i1, I...)
+        @propagate_inbounds Base.$f(A::AbstractDimArray, i1::_DimIndicesAmb, I::_DimIndicesAmb...) = 
+            $_f(A, _simplify_dim_indices(i1, I...)...)
+        @propagate_inbounds Base.$f(A::AbstractDimArray, i1::_DimIndicesAmb2,  I::_DimIndicesAmb2...) = 
+            $_f(A, _simplify_dim_indices(i1, I)...)
 
         # Use underscore methods to minimise ambiguities
         @propagate_inbounds $_f(A::AbstractBasicDimArray, d1::Dimension, ds::Dimension...) =

test/indexing.jl

@@ -42,8 +42,9 @@ end
 @testset "lookup" begin
     @testset "Points" begin
         l = Sampled(2.0:2.0:10, ForwardOrdered(), Regular(2.0), Points(), nothing)
-        @test l[:] == l
-        @test l[1:5] == l
+        @test l[:] == l[Begin:End] == l
+        @test l[Begin:5] == l[1:5] == l
+        @test l[Begin:End] isa typeof(l)
         @test l[1:5] isa typeof(l)
         @test l[[1, 3, 4]] == view(l, [1, 3, 4]) == 
             Base.dotview(l, [1, 3, 4]) ==
@@ -110,9 +111,10 @@ end
 
     @testset "Intervals" begin
         l = Sampled(2.0:2.0:10, ForwardOrdered(), Regular(2.0), Intervals(Start()), nothing)
-        @test l[:] == l
+        @test l[:] == l[Begin:End] == l
         @test l[1:5] == l
         @test l[1:5] isa typeof(l)
+        @test l[Begin:End] isa typeof(l)
         @test l[[1, 3, 4]] == Sampled([2.0, 6.0, 8.0], ForwardOrdered(), Irregular(2.0, 10.0), Intervals(Start()), nothing)
         @test l[Int[]] == Sampled(Float64[], ForwardOrdered(), Irregular(nothing, nothing), Intervals(Start()), nothing)
         @test l[Near(2.1)] == 2.0
@@ -168,7 +170,9 @@ end
     d = X(Sampled(2.0:2.0:10, ForwardOrdered(), Regular(2.0), Points(), nothing))
     @test @inferred d[:] == d
     @test @inferred d[1:5] == d
-    @test @inferred d[1:5] isa typeof(d)
+    @test d[1:5] isa typeof(d)
+    @test @inferred d[Begin:End] == d
+    @test d[Begin:End] isa typeof(d)
     # TODO properly handle index mashing arrays: here Regular should become Irregular
     # @test d[[1, 3, 4]] == X(Sampled([2.0, 6.0, 8.0], ForwardOrdered(), Regular(2.0), Points(), nothing))
     # @test d[[true, false, false, false, true]] == X(Sampled([2.0, 10.0], ForwardOrdered(), Regular(2.0), Points(), nothing))