Add tests for LinearIndices (#218)

* Add tests for LinearIndices

* update docs to clarify that OffsetArrays require the parent's axes to support idempotent indexing

Author: jishnub

docs/src/internals.md

@@ -44,7 +44,7 @@ type:
 
 ```jldoctest oa
 julia> ax = axes(oa, 2)
-OffsetArrays.IdOffsetRange(5:6)
+OffsetArrays.IdOffsetRange(values=5:6, indices=5:6)
 ```
 
 This has a similar design to `Base.IdentityUnitRange` that `ax[x] == x` always holds.
@@ -61,10 +61,10 @@ This property makes sure that they tend to be their own axes:
 
 ```jldoctest oa
 julia> axes(ax)
-(OffsetArrays.IdOffsetRange(5:6),)
+(OffsetArrays.IdOffsetRange(values=5:6, indices=5:6),)
 
 julia> axes(ax[ax])
-(OffsetArrays.IdOffsetRange(5:6),)
+(OffsetArrays.IdOffsetRange(values=5:6, indices=5:6),)
 ```
 
 This example of indexing is [idempotent](https://en.wikipedia.org/wiki/Idempotence).
@@ -80,7 +80,7 @@ julia> oa2 = OffsetArray([5, 10, 15, 20], 0:3)
  20
 
 julia> ax2 = axes(oa2, 1)
-OffsetArrays.IdOffsetRange(0:3)
+OffsetArrays.IdOffsetRange(values=0:3, indices=0:3)
 
 julia> oa2[2]
 15
@@ -104,6 +104,121 @@ julia> oa2[ax2[2]]
 
     While these behave equivalently now (conversion currently performs coercion), developers are encouraged to "future-proof" their code by choosing the behavior appropriate for each usage.
 
+
+## Wrapping other offset array types
+
+An `OffsetArray` may wrap any subtype of `AbstractArray`, including ones that do not use `1`-based indexing. Such arrays however need to satisfy the fundamental axiom of idempotent indexing for things to work correctly. In other words, an axis of an offset array needs to have the same values as its own axis. This property is built into `OffsetArray`s if the parent uses 1-based indexing, but it's up to the user to ensure the correctness in case a type is to be wrapped that uses offset indices.
+
+We demonstrate this through an example by creating a custom 0-based range type that we wrap in an `OffsetArray`:
+
+```jldoctest zerobasedrange; setup=:(using OffsetArrays)
+julia> struct ZeroBasedRange{T,A<:AbstractRange{T}} <: AbstractRange{T}
+           a :: A
+           function ZeroBasedRange(a::AbstractRange{T}) where {T}
+               @assert !Base.has_offset_axes(a)
+               new{T, typeof(a)}(a)
+           end
+       end;
+
+julia> Base.parent(A::ZeroBasedRange) = A.a;
+
+julia> Base.first(A::ZeroBasedRange) = first(A.a);
+
+julia> Base.length(A::ZeroBasedRange) = length(A.a);
+
+julia> Base.last(A::ZeroBasedRange) = last(A.a);
+
+julia> Base.size(A::ZeroBasedRange) = size(A.a);
+
+julia> Base.axes(A::ZeroBasedRange) = map(x -> 0:x-1, size(A.a));
+
+julia> Base.getindex(A::ZeroBasedRange, i::Int) = A.a[i + 1];
+
+julia> Base.step(A::ZeroBasedRange) = step(A.a);
+
+julia> function Base.show(io::IO, A::ZeroBasedRange)
+           show(io, A.a)
+           print(io, " with indices $(axes(A,1))")
+       end;
+```
+
+This definition of a `ZeroBasedRange` appears to have the correct indices, for example:
+
+```jldoctest zerobasedrange
+julia> z = ZeroBasedRange(1:4)
+1:4 with indices 0:3
+
+julia> z[0]
+1
+
+julia> z[3]
+4
+```
+
+However this does not use idempotent indexing, as the axis of a `ZeroBasedRange` is not its own axis.
+
+```jldoctest zerobasedrange
+julia> axes(z, 1)
+0:3
+
+julia> axes(axes(z, 1), 1)
+Base.OneTo(4)
+```
+
+This will lead to complications in certain functions --- for example `LinearIndices` --- that tend to implictly assume idempotent indexing. In this case the `LinearIndices` of `z` will not match its axis.
+
+```jldoctest zerobasedrange
+julia> LinearIndices(z)
+4-element LinearIndices{1, Tuple{UnitRange{Int64}}}:
+ 1
+ 2
+ 3
+ 4
+```
+
+Wrapping such a type in an `OffsetArray` might lead to unexpected bugs.
+
+```jldoctest zerobasedrange
+julia> zo = OffsetArray(z, 1);
+
+julia> axes(zo, 1)
+OffsetArrays.IdOffsetRange(values=1:4, indices=2:5)
+
+julia> Array(zo)
+ERROR: BoundsError: attempt to access 4-element UnitRange{Int64} at index [5]
+[...]
+```
+
+The `Array` conversion errors despite `zo` having 1-based indices. The function `axes(zo, 1)` hints at the underlying problem --- the values and the indices of the axis are different. We may check that the axis of `zo` is not its own axis:
+
+```jldoctest zerobasedrange
+julia> axes(zo, 1)
+OffsetArrays.IdOffsetRange(values=1:4, indices=2:5)
+
+julia> axes(axes(zo, 1), 1)
+OffsetArrays.IdOffsetRange(values=2:5, indices=2:5)
+```
+
+In this case the bug may be fixed by defining the `axes` of a `ZeroBasedRange` to be idempotent, for example using the `OffsetArrays.IdentityUnitRange` wrapper:
+
+```jldoctest zerobasedrange
+julia> Base.axes(A::ZeroBasedRange) = map(x -> OffsetArrays.IdentityUnitRange(0:x-1), size(A.a))
+
+julia> axes(zo, 1)
+OffsetArrays.IdOffsetRange(values=1:4, indices=1:4)
+```
+
+With this new definition, the values and indices of the axis are identical, which makes indexing idempotent. The conversion to an `Array` works as expected now:
+
+```jldoctest zerobasedrange
+julia> Array(zo)
+4-element Vector{Int64}:
+ 1
+ 2
+ 3
+ 4
+```
+
 ## Caveats
 
 Because `IdOffsetRange` behaves quite differently to the normal `UnitRange` type, there are some
@@ -115,13 +230,13 @@ One such cases is `getindex`:
 julia> Ao = zeros(-3:3, -3:3); Ao[:] .= 1:49;
 
 julia> Ao[-3:0, :] |> axes # the first dimension does not preserve offsets
-(OffsetArrays.IdOffsetRange(1:4), OffsetArrays.IdOffsetRange(-3:3))
+(OffsetArrays.IdOffsetRange(values=1:4, indices=1:4), OffsetArrays.IdOffsetRange(values=-3:3, indices=-3:3))
 
 julia> Ao[-3:0, -3:3] |> axes # neither dimensions preserve offsets
 (Base.OneTo(4), Base.OneTo(7))
 
 julia> Ao[axes(Ao)...] |> axes # offsets are preserved
-(OffsetArrays.IdOffsetRange(-3:3), OffsetArrays.IdOffsetRange(-3:3))
+(OffsetArrays.IdOffsetRange(values=-3:3, indices=-3:3), OffsetArrays.IdOffsetRange(values=-3:3, indices=-3:3))
 
 julia> Ao[:] |> axes # This is linear indexing
 (Base.OneTo(49),)
@@ -138,7 +253,7 @@ julia> Ao[I, 0][1] == Ao[I[1], 0]
 true
 
 julia> ax = axes(Ao, 1) # ax starts at index -3
-OffsetArrays.IdOffsetRange(-3:3)
+OffsetArrays.IdOffsetRange(values=-3:3, indices=-3:3)
 
 julia> Ao[ax, 0][1] == Ao[ax[1], 0]
 true
@@ -164,7 +279,7 @@ julia> a = zeros(3, 3);
 julia> oa = OffsetArray(a, ZeroBasedIndexing());
 
 julia> axes(oa)
-(OffsetArrays.IdOffsetRange(0:2), OffsetArrays.IdOffsetRange(0:2))
+(OffsetArrays.IdOffsetRange(values=0:2, indices=0:2), OffsetArrays.IdOffsetRange(values=0:2, indices=0:2))
 ```
 
 In this example we had to define the action of `to_indices` as the type `ZeroBasedIndexing` did not have a familiar hierarchy. Things are even simpler if we subtype `AbstractUnitRange`, in which case we need to define `first` and `length` for the custom range to be able to use it as an axis:
@@ -182,7 +297,7 @@ julia> Base.length(r::ZeroTo) = r.n + 1
 julia> oa = OffsetArray(zeros(2,2), ZeroTo(1), ZeroTo(1));
 
 julia> axes(oa)
-(OffsetArrays.IdOffsetRange(0:1), OffsetArrays.IdOffsetRange(0:1))
+(OffsetArrays.IdOffsetRange(values=0:1, indices=0:1), OffsetArrays.IdOffsetRange(values=0:1, indices=0:1))
 ```
 
-Note that zero-based indexing may also be achieved using the pre-defined type [`OffsetArrays.Origin`](@ref).
+Note that zero-based indexing may also be achieved using the pre-defined type [`OffsetArrays.Origin`](@ref).

test/runtests.jl

@@ -47,7 +47,7 @@ for Z in [:ZeroBasedRange, :ZeroBasedUnitRange]
     @eval Base.length(A::$Z) = length(A.a)
     @eval Base.last(A::$Z) = last(A.a)
     @eval Base.size(A::$Z) = size(A.a)
-    @eval Base.axes(A::$Z) = map(x -> 0:x-1, size(A.a))
+    @eval Base.axes(A::$Z) = map(x -> IdentityUnitRange(0:x-1), size(A.a))
     @eval Base.getindex(A::$Z, i::Int) = A.a[i + 1]
     @eval Base.step(A::$Z) = step(A.a)
     @eval OffsetArrays.no_offset_view(A::$Z) = A.a
@@ -1023,6 +1023,13 @@ end
     end
 end
 
+@testset "LinearIndexing" begin
+    r = OffsetArray(ZeroBasedRange(3:4), 1);
+    @test LinearIndices(r) == axes(r,1)
+    r = OffsetArray(ZeroBasedRange(3:4), 2);
+    @test LinearIndices(r) == axes(r,1)
+end
+
 @testset "CartesianIndexing" begin
     A0 = [1 3; 2 4]
     A = OffsetArray(A0, (-1,2))