Skip to content

Commit

Permalink
deprecate non-sharing forms of reshape. fixes #4211
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Apr 18, 2016
1 parent 8298eb1 commit 0e2d6cc
Show file tree
Hide file tree
Showing 19 changed files with 74 additions and 83 deletions.
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,12 @@ Deprecated or removed

* `scale` is deprecated in favor of either `α*A`, `Diagonal(x)*A`, or `A*Diagonal(x)`. ([#15258])

* `reshape` is now defined to always share data with the original array.
Uses like `reshape(1:10, (2,5))` should be replaced with `collect(1:10, (2,5))` ([#4211]).

[PkgDev]: https://github.com/JuliaLang/PkgDev.jl
<!--- generated by NEWS-update.jl: -->
[#4211]: https://github.com/JuliaLang/julia/issues/4211
[#8036]: https://github.com/JuliaLang/julia/issues/8036
[#8846]: https://github.com/JuliaLang/julia/issues/8846
[#9503]: https://github.com/JuliaLang/julia/issues/9503
Expand Down Expand Up @@ -190,6 +194,7 @@ Deprecated or removed
[#15192]: https://github.com/JuliaLang/julia/issues/15192
[#15242]: https://github.com/JuliaLang/julia/issues/15242
[#15258]: https://github.com/JuliaLang/julia/issues/15258
[#15409]: https://github.com/JuliaLang/julia/issues/15409
[#15550]: https://github.com/JuliaLang/julia/issues/15550
[#15609]: https://github.com/JuliaLang/julia/issues/15609
[#15763]: https://github.com/JuliaLang/julia/issues/15763
6 changes: 0 additions & 6 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,6 @@ similar( a::AbstractArray, T::Type, dims::Integer...) = similar(a, T, dims)
similar( a::AbstractArray, T::Type, dims::DimsInteger) = Array(T, dims...)
similar( a::AbstractArray, T::Type, dims::Dims) = Array(T, dims)

function reshape(a::AbstractArray, dims::Dims)
if prod(dims) != length(a)
throw(ArgumentError("dimensions must be consistent with array size (expected $(length(a)), got $(prod(dims)))"))
end
copy!(similar(a, dims), a)
end
reshape(a::AbstractArray, dims::Int...) = reshape(a, dims)

## from general iterable to any array
Expand Down
17 changes: 13 additions & 4 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,25 @@ _similar_for(c::AbstractArray, T, itr, ::HasShape) = similar(c, T, convert(Dims,
_similar_for(c, T, itr, isz) = similar(c, T)

"""
collect(element_type, collection)
collect(element_type, iterator)
Return an array of type `Array{element_type,1}` of all items in a collection.
Return an array of type `Array{element_type,1}` of all items from an iterator.
"""
collect{T}(::Type{T}, itr) = collect(Generator(T, itr))

collect{T}(::Type{T}, itr::Dims) = collect(Generator(T, itr))

"""
collect(iterator, dims)
Return an array with the given dimensions of all elements from an iterator.
"""
collect(itr, dims::Dims) = collect(IteratorND(itr, dims))

"""
collect(collection)
collect(iterator)
Return an array of all items in a collection. For associative collections, returns Pair{KeyType, ValType}.
Return an array of all items from an iterator. For associative collections, returns Pair{KeyType, ValType}.
"""
collect(itr) = _collect(1:1 #= Array =#, itr, iteratoreltype(itr), iteratorsize(itr))

Expand Down
5 changes: 5 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1037,3 +1037,8 @@ function pmap(f, c...; err_retry=nothing, err_stop=nothing, pids=nothing)

return pmap(p, f, c...)
end

# 4211
@deprecate reshape(a::AbstractArray, dims::Dims) copy!(similar(a, dims), a)
@deprecate reshape{Tv,Ti}(a::SparseMatrixCSC{Tv,Ti}, dims::NTuple{2,Int}) copy!(similar(a, dims), a)
@deprecate reshape(r::Range, dims::Dims) collect(r, dims)
4 changes: 1 addition & 3 deletions base/docs/helpdb/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1572,9 +1572,7 @@ ind2chr
"""
reshape(A, dims)
Create an array with the same data as the given array, but with different dimensions. An
implementation for a particular type of array may choose whether the data is copied or
shared.
Create an array with the same data as the given array, but with different dimensions.
"""
reshape

Expand Down
16 changes: 0 additions & 16 deletions base/sparse/sparsematrix.jl
Original file line number Diff line number Diff line change
Expand Up @@ -172,22 +172,6 @@ function reinterpret{T,Tv,Ti,N}(::Type{T}, a::SparseMatrixCSC{Tv,Ti}, dims::NTup
return SparseMatrixCSC(mS, nS, colptr, rowval, nzval)
end

function reshape{Tv,Ti}(a::SparseMatrixCSC{Tv,Ti}, dims::NTuple{2,Int})
if prod(dims) != length(a)
throw(DimensionMismatch("new dimensions $(dims) must be consistent with array size $(length(a))"))
end
mS,nS = dims
mA,nA = size(a)
numnz = nnz(a)
colptr = Array(Ti, nS+1)
rowval = similar(a.rowval)
nzval = copy(a.nzval)

sparse_compute_reshaped_colptr_and_rowval(colptr, rowval, mS, nS, a.colptr, a.rowval, mA, nA)

return SparseMatrixCSC(mS, nS, colptr, rowval, nzval)
end

## Constructors

copy(S::SparseMatrixCSC) =
Expand Down
19 changes: 8 additions & 11 deletions test/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ import Base: trailingsize
const can_inline = Base.JLOptions().can_inline != 0
function test_scalar_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)
@test A == B
# Test indexing up to 5 dimensions
Expand Down Expand Up @@ -186,23 +186,23 @@ end

function test_vector_indexing{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)
idxs = rand(1:N, 3, 3, 3)
@test B[idxs] == A[idxs] == idxs
@test B[vec(idxs)] == A[vec(idxs)] == vec(idxs)
@test B[:] == A[:] == collect(1:N)
@test B[1:end] == A[1:end] == collect(1:N)
@test B[:,:] == A[:,:] == reshape(1:N, shape[1], prod(shape[2:end]))
@test B[1:end,1:end] == A[1:end,1:end] == reshape(1:N, shape[1], prod(shape[2:end]))
@test B[:,:] == A[:,:] == collect(1:N, (shape[1], prod(shape[2:end])))
@test B[1:end,1:end] == A[1:end,1:end] == collect(1:N, (shape[1], prod(shape[2:end])))
# Test with containers that aren't Int[]
@test B[[]] == A[[]] == []
@test B[convert(Array{Any}, idxs)] == A[convert(Array{Any}, idxs)] == idxs
end

function test_primitives{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)

# last(a)
Expand All @@ -221,9 +221,6 @@ function test_primitives{T}(::Type{T}, shape, ::Type{TestAbstractArray})
@test isassigned(B, length(B) + 1) == false
end

# reshape(a::AbstractArray, dims::Dims)
@test_throws ArgumentError reshape(B, (0, 1))

# copy!(dest::AbstractArray, src::AbstractArray)
@test_throws BoundsError copy!(Array(Int, 10), [1:11...])

Expand Down Expand Up @@ -252,7 +249,7 @@ type UnimplementedArray{T, N} <: AbstractArray{T, N} end

function test_getindex_internals{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)

@test getindex(A) == 1
Expand All @@ -272,7 +269,7 @@ end

function test_setindex!_internals{T}(::Type{T}, shape, ::Type{TestAbstractArray})
N = prod(shape)
A = reshape(1:N, shape)
A = collect(1:N, shape)
B = T(A)

Base.unsafe_setindex!(B, 1)
Expand Down Expand Up @@ -362,7 +359,7 @@ function test_ind2sub(::Type{TestAbstractArray})
n = rand(2:5)
dims = tuple(rand(1:5, n)...)
len = prod(dims)
A = reshape(1:len, dims...)
A = collect(1:len, dims)
I = ind2sub(dims, [1:len...])
for i in 1:len
idx = [ I[j][i] for j in 1:n ]
Expand Down
17 changes: 8 additions & 9 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ a = rand(1, 1, 8, 8, 1)
@test_throws ArgumentError squeeze(a, 6)

sz = (5,8,7)
A = reshape(1:prod(sz),sz...)
A = collect(1:prod(sz), sz)
@test A[2:6] == [2:6;]
@test A[1:3,2,2:4] == cat(2,46:48,86:88,126:128)
@test A[:,7:-3:1,5] == [191 176 161; 192 177 162; 193 178 163; 194 179 164; 195 180 165]
@test A[:,3:9] == reshape(11:45,5,7)
@test A[:,3:9] == collect(11:45, (5,7))
rng = (2,2:3,2:2:5)
tmp = zeros(Int,map(maximum,rng)...)
tmp[rng...] = A[rng...]
Expand Down Expand Up @@ -138,7 +138,7 @@ B[[3,1],[2,4]] = [21 22; 23 24]
B[4,[2,3]] = 7
@test B == [0 23 1 24 0; 11 12 13 14 15; 0 21 3 22 0; 0 7 7 0 0]

@test isequal(reshape(1:27, 3, 3, 3)[1,:], [1, 4, 7, 10, 13, 16, 19, 22, 25])
@test isequal(collect(1:27, (3, 3, 3))[1,:], [1, 4, 7, 10, 13, 16, 19, 22, 25])

a = [3, 5, -7, 6]
b = [4, 6, 2, -7, 1]
Expand Down Expand Up @@ -169,7 +169,7 @@ rt = Base.return_types(setindex!, Tuple{Array{Int32, 3}, UInt8, Vector{Int}, Int

# get
let
A = reshape(1:24, 3, 8)
A = collect(1:24, (3, 8))
x = get(A, 32, -12)
@test x == -12
x = get(A, 14, -12)
Expand Down Expand Up @@ -368,7 +368,7 @@ p = permutedims(s, [2,1])
@test p[2,1]==a[2,3] && p[2,2]==a[3,3]

# of a non-strided subarray
a = reshape(1:60, 3, 4, 5)
a = collect(1:60, (3, 4, 5))
s = sub(a,:,[1,2,4],[1,5])
c = convert(Array, s)
for p in ([1,2,3], [1,3,2], [2,1,3], [2,3,1], [3,1,2], [3,2,1])
Expand Down Expand Up @@ -553,7 +553,7 @@ let
3 3 4 4 3 3 4 4;
3 3 4 4 3 3 4 4]

A = reshape(1:8, 2, 2, 2)
A = collect(1:8, (2, 2, 2))
R = repeat(A, inner = [1, 1, 2], outer = [1, 1, 1])
T = reshape([1:4; 1:4; 5:8; 5:8], 2, 2, 4)
@test R == T
Expand Down Expand Up @@ -623,7 +623,7 @@ a[[true,false,true,false,true]] = 6
a[[true,false,true,false,true]] = [7,8,9]
@test a == [7,2,8,4,9]
@test_throws DimensionMismatch (a[[true,false,true,false,true]] = [7,8,9,10])
A = reshape(1:15, 3, 5)
A = collect(1:15, (3, 5))
@test A[[true, false, true], [false, false, true, true, false]] == [7 10; 9 12]
@test_throws BoundsError A[[true, false], [false, false, true, true, false]]
@test_throws BoundsError A[[true, false, true], [false, true, true, false]]
Expand Down Expand Up @@ -1092,7 +1092,7 @@ a = ones(5,0)
b = sub(a, :, :)
@test mdsum(b) == 0

a = reshape(1:60, 3, 4, 5)
a = collect(1:60, (3, 4, 5))
@test a[CartesianIndex{3}(2,3,4)] == 44
a[CartesianIndex{3}(2,3,3)] = -1
@test a[CartesianIndex{3}(2,3,3)] == -1
Expand Down Expand Up @@ -1353,7 +1353,6 @@ C = copy(B)
@test A == C
@test B == C

@test vec(A) == vec(B) == vec(S)
@test minimum(A) == minimum(B) == minimum(S)
@test maximum(A) == maximum(B) == maximum(S)

Expand Down
2 changes: 1 addition & 1 deletion test/dict.jl
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ end
# show
for d in (Dict("\n" => "\n", "1" => "\n", "\n" => "2"),
[string(i) => i for i = 1:30],
[reshape(1:i^2,i,i) => reshape(1:i^2,i,i) for i = 1:24],
[collect(1:i^2,(i,i)) => collect(1:i^2,(i,i)) for i = 1:24],
[utf8(Char['α':'α'+i;]) => utf8(Char['α':'α'+i;]) for i = (1:10)*10],
Dict("key" => zeros(0, 0)))
for cols in (12, 40, 80), rows in (2, 10, 24)
Expand Down
4 changes: 2 additions & 2 deletions test/examples.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ y = inv(x)

include(joinpath(dir, "ndgrid.jl"))
r = repmat(1:10,1,10)
r1, r2 = ndgrid(1:10, 1:10)
r1, r2 = ndgrid(collect(1:10), collect(1:10))
@test r1 == r
@test r2 == r'
r3, r4 = meshgrid(1:10,1:10)
r3, r4 = meshgrid(collect(1:10),collect(1:10))
@test r3 == r'
@test r4 == r

Expand Down
2 changes: 1 addition & 1 deletion test/fft.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ b = rand(17,14)
b[3:6,9:12] = m4
sm4 = slice(b,3:6,9:12)

m3d = map(Float32,reshape(1:5*3*2, 5, 3, 2))
m3d = map(Float32,collect(1:5*3*2, (5, 3, 2)))
true_fftd3_m3d = Array(Float32, 5, 3, 2)
true_fftd3_m3d[:,:,1] = 17:2:45
true_fftd3_m3d[:,:,2] = -15
Expand Down
2 changes: 1 addition & 1 deletion test/functional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ end
@test collect((i+10j for i=1:2,j=3:4,k=1:1)) == reshape([31 41; 32 42], (2,2,1))

let I = Base.IteratorND(1:27,(3,3,3))
@test collect(I) == reshape(1:27,(3,3,3))
@test collect(I) == collect(1:27,(3,3,3))
@test size(I) == (3,3,3)
@test length(I) == 27
@test eltype(I) === Int
Expand Down
10 changes: 5 additions & 5 deletions test/linalg/matmul.jl
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ C = zeros(Float64,6,6)
@test Base.LinAlg.Ac_mul_B!(C,A,B) == A.'*B

# matrix algebra with subarrays of floats (stride != 1)
A = reshape(map(Float64,1:20),5,4)
A = collect(1.0:20.0, (5,4))
Aref = A[1:2:end,1:2:end]
Asub = sub(A, 1:2:5, 1:2:4)
b = [1.2,-2.5]
Expand All @@ -127,23 +127,23 @@ Asub = sub(Ai, 1:2:5, 1:2:4)
@test Ac_mul_B(Asub, Asub) == Ac_mul_B(Aref, Aref)
@test A_mul_Bc(Asub, Asub) == A_mul_Bc(Aref, Aref)
# issue #15286
let C = zeros(8, 8), sC = sub(C, 1:2:8, 1:2:8), B = reshape(map(Float64,-9:10),5,4)
let C = zeros(8, 8), sC = sub(C, 1:2:8, 1:2:8), B = collect(-9.0:10.0, (5,4))
@test At_mul_B!(sC, A, A) == A'*A
@test At_mul_B!(sC, A, B) == A'*B
end
let Aim = A .- im, C = zeros(Complex128,8,8), sC = sub(C, 1:2:8, 1:2:8), B = reshape(map(Float64,-9:10),5,4) .+ im
let Aim = A .- im, C = zeros(Complex128,8,8), sC = sub(C, 1:2:8, 1:2:8), B = collect(-9.0:10.0, (5,4)) .+ im
@test Ac_mul_B!(sC, Aim, Aim) == Aim'*Aim
@test Ac_mul_B!(sC, Aim, B) == Aim'*B
end


# syrk & herk
A = reshape(1:1503, 501, 3).-750.0
A = collect(1:1503, (501, 3)).-750.0
res = Float64[135228751 9979252 -115270247; 9979252 10481254 10983256; -115270247 10983256 137236759]
@test At_mul_B(A, A) == res
@test A_mul_Bt(A',A') == res
cutoff = 501
A = reshape(1:6*cutoff,2*cutoff,3).-(6*cutoff)/2
A = collect(1:6*cutoff, (2*cutoff,3)).-(6*cutoff)/2
Asub = sub(A, 1:2:2*cutoff, 1:3)
Aref = A[1:2:2*cutoff, 1:3]
@test At_mul_B(Asub, Asub) == At_mul_B(Aref, Aref)
Expand Down
2 changes: 1 addition & 1 deletion test/parallel_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ d = SharedArray(Float64, (2,3))
fn = tempname()
write(fn, 1:30)
sz = (6,5)
Atrue = reshape(1:30, sz)
Atrue = collect(1:30, sz)

S = SharedArray(fn, Int, sz)
@test S == Atrue
Expand Down
12 changes: 6 additions & 6 deletions test/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
@test sum([3]) === 3
@test sum([3.0]) === 3.0

z = reshape(1:16, (2,2,2,2))
z = collect(1:16, (2,2,2,2))
fz = float(z)
@test sum(z) === 136
@test sum(fz) === 136.0
Expand Down Expand Up @@ -167,9 +167,9 @@ prod2(itr) = invoke(prod, Tuple{Any}, itr)
@test maximum(collect(Int16(1):Int16(100))) === Int16(100)
@test maximum(Int32[1,2]) === Int32(2)

@test extrema(reshape(1:24,2,3,4),1) == reshape([(1,2),(3,4),(5,6),(7,8),(9,10),(11,12),(13,14),(15,16),(17,18),(19,20),(21,22),(23,24)],1,3,4)
@test extrema(reshape(1:24,2,3,4),2) == reshape([(1,5),(2,6),(7,11),(8,12),(13,17),(14,18),(19,23),(20,24)],2,1,4)
@test extrema(reshape(1:24,2,3,4),3) == reshape([(1,19),(2,20),(3,21),(4,22),(5,23),(6,24)],2,3,1)
@test extrema(collect(1:24,(2,3,4)),1) == reshape([(1,2),(3,4),(5,6),(7,8),(9,10),(11,12),(13,14),(15,16),(17,18),(19,20),(21,22),(23,24)],1,3,4)
@test extrema(collect(1:24,(2,3,4)),2) == reshape([(1,5),(2,6),(7,11),(8,12),(13,17),(14,18),(19,23),(20,24)],2,1,4)
@test extrema(collect(1:24,(2,3,4)),3) == reshape([(1,19),(2,20),(3,21),(4,22),(5,23),(6,24)],2,3,1)

# any & all

Expand Down Expand Up @@ -275,11 +275,11 @@ end
@test sum(collect(map(UInt8,0:255))) == 32640
@test sum(collect(map(UInt8,254:255))) == 509

A = reshape(map(UInt8, 101:109), (3,3))
A = collect(UInt8(101):UInt8(109), (3,3))
@test @inferred(sum(A)) == 945
@test @inferred(sum(sub(A, 1:3, 1:3))) == 945

A = reshape(map(UInt8, 1:100), (10,10))
A = collect(UInt8(1):UInt8(100), (10,10))
@test @inferred(sum(A)) == 5050
@test @inferred(sum(sub(A, 1:10, 1:10))) == 5050

Expand Down
4 changes: 2 additions & 2 deletions test/reducedim.jl
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ fill!(r, -1.1)
@test_approx_eq sumabs2!(r, Breduc, init=false) safe_sumabs2(Breduc, 1)-1.1

# Small arrays with init=false
A = reshape(1:15, 3, 5)
A = collect(1:15, (3, 5))
R = ones(Int, 3)
@test sum!(R, A, init=false) == [36,41,46]
R = ones(Int, 1, 5)
@test sum!(R, A, init=false) == [7 16 25 34 43]
R = [2]
A = reshape(1:6, 3, 2)
A = collect(1:6, (3, 2))
@test prod!(R, A, init=false) == [1440]

# Small integers
Expand Down
Loading

0 comments on commit 0e2d6cc

Please sign in to comment.