Skip to content

Commit 3b5b11b

Browse files
timholyjishnub
andauthored
Make no_offset_view inferrable (#189)
This changes the implementation so that the axis *value*s do not affect the type of the output. Technically this might be viewed as a breaking change, but only at the level of the returned types, not at the level of returned values. I don't think that really counts as breaking. Closes #198 Also fixes a new Adapt ambiguity in our tests Co-authored-by: Jishnu Bhattacharya <[email protected]>
1 parent 47f5614 commit 3b5b11b

File tree

2 files changed

+49
-10
lines changed

2 files changed

+49
-10
lines changed

src/OffsetArrays.jl

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -434,16 +434,18 @@ julia> A
434434
2 4 6
435435
```
436436
"""
437-
function no_offset_view(A::AbstractArray)
438-
if Base.has_offset_axes(A)
439-
OffsetArray(A, Origin(1))
440-
else
441-
A
442-
end
443-
end
444-
445437
no_offset_view(A::OffsetArray) = no_offset_view(parent(A))
438+
no_offset_view(a::AbstractUnitRange) = UnitRange(a)
439+
if isdefined(Base, :IdentityUnitRange)
440+
no_offset_view(a::Base.Slice) = Base.Slice(UnitRange(a)) # valid only if Slice is distinguished from IdentityUnitRange
441+
no_offset_view(S::SubArray) = view(parent(S), map(no_offset_view, parentindices(S))...)
442+
end
446443
no_offset_view(a::Array) = a
444+
no_offset_view(i::Number) = i
445+
no_offset_view(A::AbstractArray) = _no_offset_view(axes(A), A)
446+
_no_offset_view(::Tuple{}, A::AbstractArray{T,0}) where T = A
447+
_no_offset_view(::Tuple{<:Base.OneTo,Vararg{<:Base.OneTo}}, A::AbstractArray) = A
448+
_no_offset_view(::Any, A::AbstractArray) = OffsetArray(A, Origin(1))
447449

448450
####
449451
# work around for segfault in searchsorted*

test/runtests.jl

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ end
4141
end
4242

4343
@testset "IdOffsetRange" begin
44-
44+
4545
function check_indexed_by(r, rindx)
4646
for i in rindx
4747
r[i]
@@ -1403,6 +1403,7 @@ end
14031403
Base.parent(x::PointlessWrapper) = x.parent
14041404
Base.size(x::PointlessWrapper) = size(parent(x))
14051405
Base.axes(x::PointlessWrapper) = axes(parent(x))
1406+
Base.getindex(x::PointlessWrapper, i...) = x.parent[i...]
14061407

14071408
@testset "no offset view" begin
14081409
# OffsetArray fallback
@@ -1415,21 +1416,56 @@ Base.axes(x::PointlessWrapper) = axes(parent(x))
14151416
@inferred no_offset_view(O2)
14161417

14171418
P = PointlessWrapper(A)
1418-
@test no_offset_view(P) P
1419+
@test @inferred(no_offset_view(P)) === P
1420+
@test @inferred(no_offset_view(A)) === A
1421+
a0 = reshape([1])
1422+
@test @inferred(no_offset_view(a0)) === a0
1423+
a0v = view(a0)
1424+
@test @inferred(no_offset_view(a0v)) === a0v
14191425

14201426
# generic fallback
14211427
A = collect(reshape(1:12, 3, 4))
14221428
N = NegativeArray(A)
14231429
@test N[-3, -4] == 1
14241430
V = no_offset_view(N)
14251431
@test collect(V) == A
1432+
A = reshape(view([5], 1, 1))
1433+
@test no_offset_view(A) == A
14261434

14271435
# bidirectional
14281436
B = BidirectionalVector([1, 2, 3])
14291437
pushfirst!(B, 0)
14301438
OB = OffsetArrays.no_offset_view(B)
14311439
@test axes(OB, 1) == 1:4
14321440
@test collect(OB) == 0:3
1441+
1442+
# issue #198
1443+
offax = axes(OffsetVector(1:10, -5), 1)
1444+
noffax = OffsetArrays.no_offset_view(offax)
1445+
@test noffax == -4:5
1446+
@test axes(noffax, 1) == 1:10 # ideally covered by the above, but current it isn't
1447+
@test isa(noffax, AbstractUnitRange)
1448+
1449+
# SubArrays
1450+
A = reshape(1:12, 3, 4)
1451+
V = view(A, OffsetArrays.IdentityUnitRange(2:3), OffsetArrays.IdentityUnitRange(2:3))
1452+
if collect(V) == [5 8; 6 9] # julia 1.0 has a bug here
1453+
@test OffsetArrays.no_offset_view(V) == [5 8; 6 9]
1454+
end
1455+
V = view(A, OffsetArrays.IdentityUnitRange(2:3), 2)
1456+
@test V != [5;6]
1457+
if collect(V) == [5;6]
1458+
@test OffsetArrays.no_offset_view(V) == [5;6]
1459+
end
1460+
O = OffsetArray(A, -1:1, 0:3)
1461+
V = view(O, 0:1, 1:2)
1462+
@test V == OffsetArrays.no_offset_view(V) == [5 8; 6 9]
1463+
r1, r2 = OffsetArrays.IdOffsetRange(1:3, -2), OffsetArrays.IdentityUnitRange(2:3)
1464+
V = view(O, r1, r2)
1465+
@test V != collect(V)
1466+
@test OffsetArrays.no_offset_view(V) == collect(V)
1467+
V = @view O[:,:]
1468+
@test IndexStyle(A) == IndexStyle(O) == IndexStyle(V) == IndexStyle(OffsetArrays.no_offset_view(V)) == IndexLinear()
14331469
end
14341470

14351471
@testset "no nesting" begin
@@ -1490,6 +1526,7 @@ end
14901526
@testset "Adapt" begin
14911527
# We need another storage type, CUDA.jl defines one but we can't use that for CI
14921528
# let's define an appropriate method for SArrays
1529+
Adapt.adapt_storage(::Type{SA}, xs::Array) where SA<:SArray = convert(SA, xs) # ambiguity
14931530
Adapt.adapt_storage(::Type{SA}, xs::AbstractArray) where SA<:SArray = convert(SA, xs)
14941531
arr = OffsetArray(rand(3, 3), -1:1, -1:1)
14951532
s_arr = adapt(SMatrix{3,3}, arr)

0 commit comments

Comments
 (0)