Skip to content

Commit 5835c3b

Browse files
jishnubnsajko
andauthored
Accept more general Integer sizes in reshape (#55521)
This PR generalizes the `reshape` methods to accept `Integer`s instead of `Int`s, and adds a `_reshape_uncolon` method for `Integer` arguments. The current `_reshape_uncolon` method that accepts `Int`s is left unchanged to ensure that the inferred types are not impacted. I've also tried to ensure that most `Integer` subtypes in `Base` that may be safely converted to `Int`s pass through that method. The call sequence would now go like this: ```julia reshape(A, ::Tuple{Vararg{Union{Integer, Colon}}}) -> reshape(A, ::Tuple{Vararg{Integer}}) -> reshape(A, ::Tuple{Vararg{Int}}) (fallback) ``` This lets packages define `reshape(A::CustomArray, ::Tuple{Integer, Vararg{Integer}})` without having to implement `_reshape_uncolon` by themselves (or having to call internal `Base` functions, as in JuliaArrays/FillArrays.jl#373). `reshape` calls involving a `Colon` would convert this to an `Integer` in `Base`, and then pass the `Integer` sizes to the custom method defined in the package. This PR does not resolve issues like #40076 because this still converts `Integer`s to `Int`s in the actual reshaping step. However, `BigInt` sizes that may be converted to `Int`s will work now: ```julia julia> reshape(1:4, big(2), big(2)) 2×2 reshape(::UnitRange{Int64}, 2, 2) with eltype Int64: 1 3 2 4 julia> reshape(1:4, big(1), :) 1×4 reshape(::UnitRange{Int64}, 1, 4) with eltype Int64: 1 2 3 4 ``` Note that the reshape method with `Integer` sizes explicitly converts these to `Int`s to avoid self-recursion (as opposed to calling `to_shape` to carry out the conversion implicitly). In the future, we may want to decide what to do with types or values that can't be converted to an `Int`. --------- Co-authored-by: Neven Sajko <s@purelymail.com>
1 parent e572d23 commit 5835c3b

File tree

4 files changed

+72
-20
lines changed

4 files changed

+72
-20
lines changed

base/reshapedarray.jl

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -121,37 +121,56 @@ reshape
121121

122122
reshape(parent::AbstractArray, dims::IntOrInd...) = reshape(parent, dims)
123123
reshape(parent::AbstractArray, shp::Tuple{Union{Integer,OneTo}, Vararg{Union{Integer,OneTo}}}) = reshape(parent, to_shape(shp))
124+
reshape(parent::AbstractArray, dims::Tuple{Integer, Vararg{Integer}}) = reshape(parent, map(Int, dims))
124125
reshape(parent::AbstractArray, dims::Dims) = _reshape(parent, dims)
125126

126127
# Allow missing dimensions with Colon():
127128
reshape(parent::AbstractVector, ::Colon) = parent
128129
reshape(parent::AbstractVector, ::Tuple{Colon}) = parent
129130
reshape(parent::AbstractArray, dims::Int...) = reshape(parent, dims)
130-
reshape(parent::AbstractArray, dims::Union{Int,Colon}...) = reshape(parent, dims)
131-
reshape(parent::AbstractArray, dims::Tuple{Vararg{Union{Int,Colon}}}) = reshape(parent, _reshape_uncolon(parent, dims))
132-
@inline function _reshape_uncolon(A, dims)
133-
@noinline throw1(dims) = throw(DimensionMismatch(string("new dimensions $(dims) ",
134-
"may have at most one omitted dimension specified by `Colon()`")))
135-
@noinline throw2(A, dims) = throw(DimensionMismatch(string("array size $(length(A)) ",
136-
"must be divisible by the product of the new dimensions $dims")))
137-
pre = _before_colon(dims...)::Tuple{Vararg{Int}}
131+
reshape(parent::AbstractArray, dims::Integer...) = reshape(parent, dims)
132+
reshape(parent::AbstractArray, dims::Union{Integer,Colon}...) = reshape(parent, dims)
133+
reshape(parent::AbstractArray, dims::Tuple{Vararg{Union{Integer,Colon}}}) = reshape(parent, _reshape_uncolon(parent, dims))
134+
135+
@noinline throw1(dims) = throw(DimensionMismatch(LazyString("new dimensions ", dims,
136+
" may have at most one omitted dimension specified by `Colon()`")))
137+
@noinline throw2(lenA, dims) = throw(DimensionMismatch(string("array size ", lenA,
138+
" must be divisible by the product of the new dimensions ", dims)))
139+
140+
@inline function _reshape_uncolon(A, _dims::Tuple{Vararg{Union{Integer, Colon}}})
141+
# promote the dims to `Int` at least
142+
dims = map(x -> x isa Colon ? x : promote_type(typeof(x), Int)(x), _dims)
143+
pre = _before_colon(dims...)
138144
post = _after_colon(dims...)
139145
_any_colon(post...) && throw1(dims)
140-
post::Tuple{Vararg{Int}}
141146
len = length(A)
142-
sz, is_exact = if iszero(len)
143-
(0, true)
147+
_reshape_uncolon_computesize(len, dims, pre, post)
148+
end
149+
@inline function _reshape_uncolon_computesize(len::Int, dims, pre::Tuple{Vararg{Int}}, post::Tuple{Vararg{Int}})
150+
sz = if iszero(len)
151+
0
144152
else
145153
let pr = Core.checked_dims(pre..., post...) # safe product
146-
if iszero(pr)
147-
throw2(A, dims)
148-
end
149-
(quo, rem) = divrem(len, pr)
150-
(Int(quo), iszero(rem))
154+
quo = _reshape_uncolon_computesize_nonempty(len, dims, pr)
155+
convert(Int, quo)
151156
end
152-
end::Tuple{Int,Bool}
153-
is_exact || throw2(A, dims)
154-
(pre..., sz, post...)::Tuple{Int,Vararg{Int}}
157+
end
158+
(pre..., sz, post...)
159+
end
160+
@inline function _reshape_uncolon_computesize(len, dims, pre, post)
161+
pr = prod((pre..., post...))
162+
sz = if iszero(len)
163+
promote(len, pr)[1] # zero of the correct type
164+
else
165+
_reshape_uncolon_computesize_nonempty(len, dims, pr)
166+
end
167+
(pre..., sz, post...)
168+
end
169+
@inline function _reshape_uncolon_computesize_nonempty(len, dims, pr)
170+
iszero(pr) && throw2(len, dims)
171+
(quo, rem) = divrem(len, pr)
172+
iszero(rem) || throw2(len, dims)
173+
quo
155174
end
156175
@inline _any_colon() = false
157176
@inline _any_colon(dim::Colon, tail...) = true

test/abstractarray.jl

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2186,3 +2186,23 @@ end
21862186
copyto!(A, 1, x, 1)
21872187
@test A == axes(A,1)
21882188
end
2189+
2190+
@testset "reshape with Integer sizes" begin
2191+
@test reshape(1:4, big(2), big(2)) == reshape(1:4, 2, 2)
2192+
a = [1 2 3; 4 5 6]
2193+
reshaped_arrays = (
2194+
reshape(a, 3, 2),
2195+
reshape(a, (3, 2)),
2196+
reshape(a, big(3), big(2)),
2197+
reshape(a, (big(3), big(2))),
2198+
reshape(a, :, big(2)),
2199+
reshape(a, (:, big(2))),
2200+
reshape(a, big(3), :),
2201+
reshape(a, (big(3), :)),
2202+
)
2203+
@test allequal(reshaped_arrays)
2204+
for b reshaped_arrays
2205+
@test b isa Matrix{Int}
2206+
@test b.ref === a.ref
2207+
end
2208+
end

test/offsetarray.jl

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,3 +914,14 @@ end
914914
b = sum(a, dims=1)
915915
@test b[begin] == sum(r)
916916
end
917+
918+
@testset "reshape" begin
919+
A0 = [1 3; 2 4]
920+
A = reshape(A0, 2:3, 4:5)
921+
@test axes(A) == Base.IdentityUnitRange.((2:3, 4:5))
922+
923+
B = reshape(A0, -10:-9, 9:10)
924+
@test isa(B, OffsetArray{Int,2})
925+
@test parent(B) == A0
926+
@test axes(B) == Base.IdentityUnitRange.((-10:-9, 9:10))
927+
end

test/testhelpers/OffsetArrays.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ _similar_axes_or_length(AT, ax::I, ::I) where {I} = similar(AT, map(_indexlength
529529

530530
# reshape accepts a single colon
531531
Base.reshape(A::AbstractArray, inds::OffsetAxis...) = reshape(A, inds)
532-
function Base.reshape(A::AbstractArray, inds::Tuple{OffsetAxis,Vararg{OffsetAxis}})
532+
function Base.reshape(A::AbstractArray, inds::Tuple{Vararg{OffsetAxis}})
533533
AR = reshape(no_offset_view(A), map(_indexlength, inds))
534534
O = OffsetArray(AR, map(_offset, axes(AR), inds))
535535
return _popreshape(O, axes(AR), _filterreshapeinds(inds))
@@ -557,6 +557,8 @@ Base.reshape(A::OffsetArray, inds::Tuple{OffsetAxis,Vararg{OffsetAxis}}) =
557557
OffsetArray(_reshape(parent(A), inds), map(_toaxis, inds))
558558
# And for non-offset axes, we can just return a reshape of the parent directly
559559
Base.reshape(A::OffsetArray, inds::Tuple{Union{Integer,Base.OneTo},Vararg{Union{Integer,Base.OneTo}}}) = _reshape_nov(A, inds)
560+
Base.reshape(A::OffsetArray, inds::Tuple{Integer,Vararg{Integer}}) = _reshape_nov(A, inds)
561+
Base.reshape(A::OffsetArray, inds::Tuple{Union{Colon, Integer}, Vararg{Union{Colon, Integer}}}) = _reshape_nov(A, inds)
560562
Base.reshape(A::OffsetArray, inds::Dims) = _reshape_nov(A, inds)
561563
Base.reshape(A::OffsetVector, ::Colon) = A
562564
Base.reshape(A::OffsetVector, ::Tuple{Colon}) = A

0 commit comments

Comments
 (0)