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

Created on 11 Jul 2016  Â·  17Comments  Â·  Source: JuliaLang/julia

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.

help wanted

Most helpful comment

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

All 17 comments

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?

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?

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.

See #7956

On the other hand, #13739 was merged

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?)

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.

Not desirable, so I support changing it.

@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.

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.)

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?

Should that be interpreted as a special case of FractalArrays?

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

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, ∞).

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

It does seem like we should be more consistent here.

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 https://github.com/JuliaLang/julia/pull/17567 again.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

StefanKarpinski picture StefanKarpinski  Â·  3Comments

dpsanders picture dpsanders  Â·  3Comments

wilburtownsend picture wilburtownsend  Â·  3Comments

ararslan picture ararslan  Â·  3Comments

helgee picture helgee  Â·  3Comments