Should reshape() accept Integer as dims rather than only Int?

[davidavdav]

Currently, reshape requires the dimension parameters to be Int

reshape(parent::AbstractArray, dims::Int...) = reshape(parent, dims)

I am not sure if this is required for efficiency reasons, but I wonder if this could be changed to Integer?

In reading binary formatted data I often come across the situation where dimensions of an array are specified as Int32, and then the matrix data that follows has to be reshaped accordingly. Currently I have to escape the dimension specification using Int(), a small effort, but it would be slightly cleaner if that would not be required.

[timholy]

There's a sense in which this is already fixed on julia-0.5:

julia> a = rand(3,4)
3×4 Array{Float64,2}:
 0.835471  0.0993574  0.678461  0.456434
 0.952059  0.217379   0.533439  0.576274
 0.231707  0.4597     0.574938  0.780569

julia> b = reshape(a, 4, 3)
4×3 Array{Float64,2}:
 0.835471   0.217379  0.574938
 0.952059   0.4597    0.456434
 0.231707   0.678461  0.576274
 0.0993574  0.533439  0.780569

julia> b = reshape(a, Int32(4), Int32(3))
4×3 Base.ReshapedArray{Float64,2,Array{Float64,2},Tuple{}}:
 0.835471   0.217379  0.574938
 0.952059   0.4597    0.456434
 0.231707   0.678461  0.576274
 0.0993574  0.533439  0.780569

julia> b.parent === a
true

Of course, it's giving you a ReshapedArray rather than an Array. Probably adding a fallback that does map(Int, dims) would do the trick? Want to give it a whirl?

[davidavdav]

Is this a very recent fix, or a different branch? On Commit f53b00e (5 days old master) I still get an error.

I guess a fallback is always an option, but I understand you'd want that for 0.4 then, right?

[stevengj]

I feel like this has been discussed in another issue, but I can't find it at the moment. Look at all of the uses of the Dims type (== Int...) in Base … if these were changed to Integer... you'd get a huge proliferation of methods.

[stevengj]

See #7956

[stevengj]

On the other hand, #13739 was merged

[martinholters]

If we generally employ the pattern

foo(whatever, dims::Dims) = ... # does the actual work
@inline foo(whatever, dims::Integer...) = foo(whatever, convert(Dims, dims))

would there be the feared huge number of specializations or would they be inlined away? (Or at worst lead to a huge number of specializations of convert only?)

[davidavdav]

Updating to today's master, I can reproduce @timholy's answer. But is it in any way desirable that the type returned by reshape() depends on its second+ argument? reshape(::Array, ::Int, ...) => ::Array, whereas reshape(::Array, ::Int32, ...) => ::ReshapedArray? That sounds a bit hairy to me.

[timholy]

Not desirable, so I support changing it.

[timholy]

@martinholters, that would indeed be the right kind of specialization: you'd get a "short" specialization of foo, along with convert. That would reduce the number of times you need to compile that 100-line version of foo.

[martinholters]

So would there be any harm in using this pattern throughout? (Whether it's worthwhile changing all of Base accordingly is another question, but maybe we could adopt it on a "while you're at it, do this too"-basis, similarly to moving docstrings inline.)

[alanedelman]

This may be another issue, but what is the thinking on dims giving the right answer
if it is a float that happens to equal an integer?

[davidavdav]

Should that be interpreted as a special case of FractalArrays?

[stevengj]

@alanedelman, see #1972, #2293, #4275, #5868, #9776, #10154 for discussion of floating-point dimensions.

[dlfivefifty]

I think converting to Int is not a good idea: for example, one may want to do

julia> reshape(BigInt(1):BigInt(100)^2, BigInt(100), BigInt(100))
ERROR: MethodError: no method matching reshape(::UnitRange{BigInt}, ::BigInt, ::BigInt)
Closest candidates are:
  reshape(::AbstractArray, ::Int64...) at reshapedarray.jl:95
  reshape(::AbstractArray, ::Union{Int64, AbstractUnitRange}...) at reshapedarray.jl:90
  reshape(::AbstractArray, ::Tuple{Vararg{Int64,N}} where N) at reshapedarray.jl:92
  ...

Or in the case of InfiniteArrays.jl, I want to be able to do reshape(1:∞, 1, ∞).

[machakann]

I came across the same problem on Julia 1.0.2. It seems Integer dims parameter other than Int got invalid again. Though this is avoidable if using Tuple, I feel it a little inconsistent.

a = zeros(100);

reshape(a, 10, 10)  # OK

reshape(a, UInt(10), UInt(10))  # MethodError
reshape(a, (UInt(10), UInt(10)))  # OK

reshape(a, Int128(10), Int128(10))  # MethodError
reshape(a, (Int128(10), Int128(10)))  # OK

[StefanKarpinski]

It does seem like we should be more consistent here.

[mbauman]

I think it's fine to be more relaxed here, but we just need to be careful how these methods all layer atop one another so we don't hit #17567 again.