Indexing is not idempotent using Base.IdentityUnitRange

[jishnub]

This does not satisfy a[ax][i] == a[ax[i]]

julia> a = OffsetVector(3:4, 2:3)
3:4 with indices 2:3

julia> ax = Base.IdentityUnitRange(2:3)
Base.IdentityUnitRange(2:3)

julia> a[ax]
3:4

julia> a[ax][2]
4

julia> a[ax[2]]
3

The issue seems to be that a[ax] results in an UnitRange instead of an OffsetVector, and has different axes from ax. Eg something like this would be worse:

julia> a = OffsetVector(3:4, 10:11)
3:4 with indices 10:11

julia> ax = Base.IdentityUnitRange(10:11)
Base.IdentityUnitRange(10:11)

julia> a[ax]
3:4

julia> a[ax[10]]
3

julia> a[ax][10]
ERROR: BoundsError: attempt to access 2-element UnitRange{Int64} at index [10]
Stacktrace:
 [1] throw_boundserror(::UnitRange{Int64}, ::Int64) at ./abstractarray.jl:541
 [2] getindex(::UnitRange{Int64}, ::Int64) at ./range.jl:646
 [3] top-level scope at REPL[26]:1

[jishnub]

Looks like this happens because of

OffsetArrays.jl/src/OffsetArrays.jl

Line 309 in dc867d5

Base.getindex(a::OffsetRange, r::AbstractRange) = a.parent[r .- a.offsets[1]]

and the fact that Base.IdentityUnitRange gets transformed to an UnitRange upon broadcasting

[johnnychen94]

I'm rewriting the constructor tests in #148. I'll add one test for it!

[jishnub]

Are you fixing this? I wonder what's the best approach? One way to solve this would be

Base.getindex(a::OffsetRange, r::AbstractRange) = OffsetArray(a.parent[r .- a.offsets[1]], axes(r,1))

however this causes tests to fail, as indexing tests explicitly check for UnitRanges. A temporary fix might be to add an extra method for Base.IdentityUnitRange, but this isn't ideal.

[johnnychen94]

Not planned in that PR. I'll tag it as broken.

If you can fix it, please do it by all means. I'm a little busy these days with the new semester starts. The procedure on my side could be a little bit slow.

[timholy]

Ranges are a mess and this is what JuliaLang/julia#30950 is about. I think what happened is that initially we intended to keep AbstractRange with 1-indexing but then we realized rather late in the process that it was really useful to break that requirement, and the process never got finished properly. Now we have to figure out how to fix it in a non-breaking way, which is what made that PR scary and seem like it might be heading down the wrong path.