Use no_offset_view in indexing AbstractRanges (#199)

* The util method _unwrap has been replaced by no_offset_view

* no_offset_view is a no-op for AbstractUnitRanges with axes of type Base.OneTo

Author: jishnub

src/OffsetArrays.jl

@@ -353,18 +353,18 @@ end
 
 for OR in [:IIUR, :IdOffsetRange]
     for R in [:StepRange, :StepRangeLen, :LinRange, :UnitRange]
-        @eval @propagate_inbounds Base.getindex(r::$R, s::$OR) = OffsetArray(r[_unwrap(s)], axes(s))
+        @eval @propagate_inbounds Base.getindex(r::$R, s::$OR) = OffsetArray(r[no_offset_view(s)], axes(s))
     end
 
     # this method is needed for ambiguity resolution
     @eval @propagate_inbounds Base.getindex(r::StepRangeLen{T,<:Base.TwicePrecision,<:Base.TwicePrecision}, s::$OR) where T =
-    OffsetArray(r[_unwrap(s)], axes(s))
+    OffsetArray(r[no_offset_view(s)], axes(s))
 end
 
 #= Integer UnitRanges may return an appropriate AbstractUnitRange{<:Integer}, as the result may be used in indexing, and
 indexing is faster with ranges =#
-@propagate_inbounds Base.getindex(r::UnitRange{<:Integer}, s::IdOffsetRange) = IdOffsetRange(r[_unwrap(s)] .- s.offset, s.offset)
-@propagate_inbounds Base.getindex(r::UnitRange{<:Integer}, s::IIUR) = IdentityUnitRange(r[_unwrap(s)])
+@propagate_inbounds Base.getindex(r::UnitRange{<:Integer}, s::IdOffsetRange) = IdOffsetRange(r[no_offset_view(s)] .- s.offset, s.offset)
+@propagate_inbounds Base.getindex(r::UnitRange{<:Integer}, s::IIUR) = IdentityUnitRange(r[no_offset_view(s)])
 
 function Base.show(io::IO, r::OffsetRange)
     show(io, r.parent)
@@ -435,7 +435,6 @@ julia> A
 ```
 """
 no_offset_view(A::OffsetArray) = no_offset_view(parent(A))
-no_offset_view(a::AbstractUnitRange) = UnitRange(a)
 if isdefined(Base, :IdentityUnitRange)
     no_offset_view(a::Base.Slice) = Base.Slice(UnitRange(a))  # valid only if Slice is distinguished from IdentityUnitRange
     no_offset_view(S::SubArray) = view(parent(S), map(no_offset_view, parentindices(S))...)
@@ -445,7 +444,10 @@ no_offset_view(i::Number) = i
 no_offset_view(A::AbstractArray) = _no_offset_view(axes(A), A)
 _no_offset_view(::Tuple{}, A::AbstractArray{T,0}) where T = A
 _no_offset_view(::Tuple{<:Base.OneTo,Vararg{<:Base.OneTo}}, A::AbstractArray) = A
+# the following method is needed for ambiguity resolution
+_no_offset_view(::Tuple{<:Base.OneTo,Vararg{<:Base.OneTo}}, A::AbstractUnitRange) = A
 _no_offset_view(::Any, A::AbstractArray) = OffsetArray(A, Origin(1))
+_no_offset_view(::Any, A::AbstractUnitRange) = UnitRange(A)
 
 ####
 # work around for segfault in searchsorted*

src/utils.jl

@@ -71,6 +71,3 @@ function _checkindices(N::Integer, indices, label)
     throw_argumenterror(N, indices, label) = throw(ArgumentError(label*" $indices are not compatible with a $(N)D array"))
     N == length(indices) || throw_argumenterror(N, indices, label)
 end
-
-_unwrap(r::IdOffsetRange) = r.parent .+ r.offset
-_unwrap(r::IdentityUnitRange) = r.indices

test/runtests.jl

@@ -1446,6 +1446,9 @@ Base.getindex(x::PointlessWrapper, i...) = x.parent[i...]
     @test axes(noffax, 1) == 1:10   # ideally covered by the above, but current it isn't
     @test isa(noffax, AbstractUnitRange)
 
+    r = Base.OneTo(4)
+    @test OffsetArrays.no_offset_view(r) isa typeof(r)
+
     # SubArrays
     A = reshape(1:12, 3, 4)
     V = view(A, OffsetArrays.IdentityUnitRange(2:3), OffsetArrays.IdentityUnitRange(2:3))