Skip to content

Commit

Permalink
Fix ambiguities and preserve Integer types
Browse files Browse the repository at this point in the history
  • Loading branch information
jishnub committed Dec 24, 2024
1 parent 25266c5 commit ec278ed
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 20 deletions.
2 changes: 0 additions & 2 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -834,13 +834,11 @@ similar(a::AbstractArray, ::Type{T}, dims::Dims{N}) where {T,N} = Array{T,N}(
to_shape(::Tuple{}) = ()
to_shape(dims::Dims) = dims
to_shape(dims::DimsOrInds) = map(to_shape, dims)::DimsOrInds
to_shape(dims::Tuple{Vararg{Union{Integer, AbstractUnitRange, Colon}}}) = map(to_shape, dims)
# each dimension
to_shape(i::Int) = i
to_shape(i::Integer) = Int(i)
to_shape(r::OneTo) = Int(last(r))
to_shape(r::AbstractUnitRange) = r
to_shape(r::Colon) = r

"""
similar(storagetype, axes)
Expand Down
19 changes: 14 additions & 5 deletions base/reshapedarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,31 @@ reshape

# we collect the vararg indices and only define methods for tuples of indices
reshape(parent::AbstractArray, dims::Union{Integer,Colon,AbstractUnitRange}...) = reshape(parent, dims)
reshape(parent::AbstractArray, dims::Tuple{Vararg{Integer}}) = _reshape(parent, map(Int, dims))
reshape(parent::AbstractArray, dims::Tuple{Vararg{Integer}}) = reshape(parent, map(Int, dims))
# Usually, custom arrays would only need to specialize `reshape(A::MyArray, dims::Tuple{Vararg{Int}})`, and other
# integer types passed as `dims` would be converted to `Int`s
reshape(parent::AbstractArray, dims::Dims) = _reshape(parent, dims)

# Allow missing dimensions with Colon():
# convert axes to sizes using to_shape, and convert colons to sizes using _reshape_uncolon
# Replace `OneTo`s by their lengths, and convert colons to sizes using _reshape_uncolon
# We add a level of indirection to avoid method ambiguities in reshape
reshape(parent::AbstractArray, dims::Tuple{Vararg{Union{Integer,Colon,OneTo}}}) = _reshape_maybecolon(parent, dims)
_reshape_maybecolon(parent::AbstractVector, ::Tuple{Colon}) = parent
_reshape_maybecolon(parent::AbstractArray, dims::Tuple{Vararg{Union{Integer,Colon,OneTo}}}) = reshape(parent, _reshape_uncolon(length(parent), to_shape(dims)))
# helper function analogous to to_shape, but this doesn't cast Integers to Int
# this allows dispatching to a specialized reshape(M::MyArray, dims::Tuple{Vararg{Integer}})
_to_shape(i::OneTo) = length(i)
_to_shape(i::Union{Integer, Colon}) = i
_to_shape(t::Tuple) = map(_to_shape, t)
_reshape_maybecolon(parent::AbstractArray, dims::Tuple{Vararg{Union{Integer,Colon,OneTo}}}) = reshape(parent, _reshape_uncolon(parent, _to_shape(dims)))

@noinline _reshape_throwcolon(dims) = throw(DimensionMismatch(LazyString("new dimensions ", dims,
" may have at most one omitted dimension specified by `Colon()`")))
@noinline _reshape_throwsize(lenA, dims) = throw(DimensionMismatch(LazyString("array size ", lenA,
" must be divisible by the product of the new dimensions ", dims)))

_reshape_uncolon(len, ::Tuple{Colon}) = len
@inline function _reshape_uncolon(len, _dims::Tuple{Vararg{Union{Integer, Colon}}})
_reshape_uncolon(parent::AbstractArray, dims::Tuple) = _reshape_uncolon(length(parent), dims)
_reshape_uncolon(len::Integer, ::Tuple{Colon}) = len
@inline function _reshape_uncolon(len::Integer, _dims::Tuple{Vararg{Union{Integer, Colon}}})
# promote the dims to `Int` at least
dims = map(x -> x isa Colon ? x : promote_type(typeof(x), Int)(x), _dims)
dims isa Tuple{Vararg{Integer}} && return dims
Expand Down
183 changes: 171 additions & 12 deletions test/offsetarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,23 @@

isdefined(Main, :OffsetArrays) || @eval Main include("testhelpers/OffsetArrays.jl")
using .Main.OffsetArrays
import .Main.OffsetArrays: IdOffsetRange
import .Main.OffsetArrays: IdOffsetRange, no_offset_view
using Random
using LinearAlgebra
using Base: IdentityUnitRange
using Test

no_offset_axes(x, d) = no_offset_view(axes(x, d))
no_offset_axes(x) = map(no_offset_view, axes(x))

function same_value(r1, r2)
length(r1) == length(r2) || return false
for (v1, v2) in zip(r1, r2)
v1 == v2 || return false
end
return true
end

if !isdefined(@__MODULE__, :T24Linear)
include("testhelpers/arrayindexingtypes.jl")
end
Expand Down Expand Up @@ -846,6 +857,84 @@ end
a = OffsetArray(4:5, 5:6)
@test reshape(a, :) === a
@test reshape(a, (:,)) === a

A0 = [1 3; 2 4]
A = OffsetArray(A0, (-1,2))

B = reshape(A0, -10:-9, 9:10)
@test isa(B, OffsetArray{Int,2})
@test parent(B) == A0
@test axes(B) == IdentityUnitRange.((-10:-9, 9:10))
B = reshape(A, -10:-9, 9:10)
@test isa(B, OffsetArray{Int,2})
@test axes(B) == IdentityUnitRange.((-10:-9, 9:10))
b = reshape(A, -7:-4)
@test axes(b) == (IdentityUnitRange(-7:-4),)
@test isa(parent(b), Vector{Int})
@test parent(b) == A0[:]
a = OffsetArray(rand(3,3,3), -1:1, 0:2, 3:5)
# Offset axes are required for reshape(::OffsetArray, ::Val) support
b = reshape(a, Val(2))
@test isa(b, OffsetArray{Float64,2})
@test axes(b) == IdentityUnitRange.((-1:1, 1:9))
b = reshape(a, Val(4))
@test isa(b, OffsetArray{Float64,4})
@test axes(b) == (axes(a)..., IdentityUnitRange(1:1))

@test reshape(OffsetArray(-1:0, -1:0), :, 1) == reshape(-1:0, 2, 1)
@test reshape(OffsetArray(-1:2, -1:2), -2:-1, :) == reshape(-1:2, -2:-1, 2)

@test reshape(OffsetArray(-1:0, -1:0), :) == OffsetArray(-1:0, -1:0)
@test reshape(A, :) == reshape(A0, :)

# reshape with one Colon for AbstractArrays
B = reshape(A0, -10:-9, :)
@test B isa OffsetArray{Int,2}
@test parent(B) == A0
@test no_offset_axes(B, 1) == -10:-9
@test axes(B, 2) == axes(A0, 2)

B = reshape(A0, -10:-9, 3:3, :)
@test B isa OffsetArray{Int,3}
@test same_value(A0, B)
@test no_offset_axes(B, 1) == -10:-9
@test no_offset_axes(B, 2) == 3:3
@test axes(B, 3) == 1:2

B = reshape(A0, -10:-9, 3:4, :)
@test B isa OffsetArray{Int,3}
@test same_value(A0, B)
@test no_offset_axes(B, 1) == -10:-9
@test no_offset_axes(B, 2) == 3:4
@test axes(B, 3) == 1:1

# pop the parent
B = reshape(A, size(A))
@test B == A0
B = reshape(A, (Base.OneTo(2), 2))
@test B == A0
B = reshape(A, (2,:))
@test B == A0

# julialang/julia #33614
A = OffsetArray(-1:0, (-2,))
@test reshape(A, :) == A
Arsc = reshape(A, :, 1)
Arss = reshape(A, 2, 1)
@test Arsc[1,1] == Arss[1,1] == -1
@test Arsc[2,1] == Arss[2,1] == 0
@test_throws BoundsError Arsc[0,1]
@test_throws BoundsError Arss[0,1]
A = OffsetArray([-1,0], (-2,))
Arsc = reshape(A, :, 1)
Arsc[1,1] = 5
@test first(A) == 5

Vec64 = zeros(6)
ind_a_64 = 3
ind_a_32 =Int32.(ind_a_64)
@test reshape(Vec64, ind_a_32, :) == reshape(Vec64, ind_a_64, :)

R = reshape(zeros(6), 2, :)
@test R isa Matrix
@test axes(R) == (1:2, 1:3)
Expand All @@ -856,6 +945,87 @@ end
@test axes(R) == (2:3, 1:3)
R = reshape(zeros(6,1), 2:3, :)
@test axes(R) == (2:3, 1:3)

R = reshape(zeros(6), 1:2, :)
@test axes(R) == (1:2, 1:3)
R = reshape(zeros(6,1), 1:2, :)
@test axes(R) == (1:2, 1:3)

# reshape works even if the parent doesn't have 1-based indices
# this works even if the parent doesn't support the reshape
r = OffsetArray(IdentityUnitRange(0:1), -1)
@test reshape(r, 2) == 0:1
@test reshape(r, (2,)) == 0:1
@test reshape(r, :) == OffsetArray(0:1, -1:0)
@test reshape(r, (:,)) == OffsetArray(0:1, -1:0)

@test reshape(ones(2:3, 4:5), (2, :)) == ones(2,2)

# more than one colon is not allowed
@test_throws Exception reshape(ones(3:4, 4:5, 1:2), :, :, 2)
@test_throws Exception reshape(ones(3:4, 4:5, 1:2), :, 2, :)

A = OffsetArray(rand(4, 4), -1, -1);
B = reshape(A, (2, :))
@test axes(B, 1) == 1:2
@test axes(B, 2) == 1:8

# reshape with one single colon becomes a `vec`
A = OffsetArray(rand(4, 4), -1, -1)
@test reshape(A, (:, )) == vec(A)
@test reshape(A, :) == vec(A)

A0 = [1 3; 2 4]
A = reshape(A0, 2:3, 4:5)
@test axes(A) == Base.IdentityUnitRange.((2:3, 4:5))
@test reshape(A, axes(parent(A))..., :) == reshape(A0, axes(A0)..., :)

B = reshape(A0, -10:-9, 9:10)
@test isa(B, OffsetArray{Int,2})
@test parent(B) == A0
@test axes(B) == Base.IdentityUnitRange.((-10:-9, 9:10))

@testset "BigInt reshape" begin
struct MyBigFill{T,N} <: AbstractArray{T,N}
val :: T
axes :: NTuple{N,Base.OneTo{BigInt}}
end
MyBigFill(val, sz::Tuple{}) = MyBigFill{typeof(val),0}(val, sz)
MyBigFill(val, sz::Tuple{Vararg{Integer}}) = MyBigFill(val, map(Base.OneTo{BigInt}, sz))
Base.size(M::MyBigFill) = map(length, M.axes)
Base.axes(M::MyBigFill) = M.axes
function Base.getindex(M::MyBigFill{<:Any,N}, ind::Vararg{Integer,N}) where {N}
checkbounds(M, ind...)
M.val
end
function Base.isassigned(M::MyBigFill{<:Any,N}, ind::Vararg{BigInt,N}) where {N}
checkbounds(M, ind...)
true
end
function Base.reshape(M::MyBigFill, ind::NTuple{N,BigInt}) where {N}
length(M) == prod(ind) || throw(ArgumentError("length mismatch in reshape"))
MyBigFill(M.val, ind)
end
Base.reshape(M::MyBigFill, ind::Tuple{}) = MyBigFill(M.val, ind)

M = MyBigFill(4, (2, 3))
O = OffsetArray(M)
@test vec(O) isa MyBigFill
@test vec(O) == vec(M)
@test reshape(O, :, big(2)) == reshape(M, :, big(2))
@test reshape(O, :, big(2)) isa MyBigFill
@test reshape(O, axes(M)) isa MyBigFill
@test reshape(O, axes(M)) == reshape(M, axes(M))
@test reshape(O, axes(M,1), :) isa MyBigFill
@test reshape(O, axes(M,1), :) == reshape(M, axes(M,1), :)
@test reshape(O, axes(M,1), big(1), :) isa MyBigFill
@test reshape(O, axes(M,1), big(1), :) == reshape(M, axes(M,1), big(1), :)

M = MyBigFill(4, (1,1))
O = OffsetArray(M)
@test reshape(O) isa MyBigFill
@test reshape(O) == reshape(M)
end
end

@testset "stack" begin
Expand Down Expand Up @@ -924,14 +1094,3 @@ end
b = sum(a, dims=1)
@test b[begin] == sum(r)
end

@testset "reshape" begin
A0 = [1 3; 2 4]
A = reshape(A0, 2:3, 4:5)
@test axes(A) == Base.IdentityUnitRange.((2:3, 4:5))

B = reshape(A0, -10:-9, 9:10)
@test isa(B, OffsetArray{Int,2})
@test parent(B) == A0
@test axes(B) == Base.IdentityUnitRange.((-10:-9, 9:10))
end
1 change: 0 additions & 1 deletion test/testhelpers/OffsetArrays.jl
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,6 @@ _reshape_nov(A, inds) = _reshape(no_offset_view(A), inds)
# And for non-offset axes, we can just return a reshape of the parent directly
Base.reshape(A::OffsetArray, inds::Tuple{Integer,Vararg{Integer}}) = _reshape_nov(A, inds)
Base.reshape(A::OffsetArray, inds::Dims) = _reshape_nov(A, inds)

# permutedims in Base does not preserve axes, and can not be fixed in a non-breaking way
# This is a stopgap solution
Base.permutedims(v::OffsetVector) = reshape(v, (1, axes(v, 1)))
Expand Down

0 comments on commit ec278ed

Please sign in to comment.