From 73933f53124c126783940cf018ac27ff456bfddc Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 29 Mar 2018 19:01:17 -0400 Subject: [PATCH 1/2] Deprecate squeeze(A, dims) to use keyword arguments This is a follow-up to issue #25501 and seems to make sense in that same vein. This also adds NEWS for squeeze and all the other related functions (#25501 was missing NEWS). --- NEWS.md | 6 ++++++ base/abstractarraymath.jl | 11 +++++------ base/deprecated.jl | 2 ++ stdlib/SparseArrays/test/sparse.jl | 6 +++--- test/arrayops.jl | 21 +++++++++++---------- test/offsetarray.jl | 22 +++++++++++----------- test/subarray.jl | 2 +- 7 files changed, 39 insertions(+), 31 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1b8a039330f2a..4a52f28173a83 100644 --- a/NEWS.md +++ b/NEWS.md @@ -667,6 +667,12 @@ Deprecated or removed Instead, reshape the array or add trailing indices so the dimensionality and number of indices match ([#14770], [#23628]). + * The use of a positional dimension argument has largely been deprecated in favor of a + `dims` keyword argument. This includes the functions `sum`, `prod`, `maximum`, + `minimum`, `all`, `any`, `findmax`, `findmin`, `mean`, `varm`, `std`, `var`, `cov`, + `cor`, `median`, `mapreducedim`, `reducedim`, `sort`, `accumulate`, `accumulate!`, + `cumsum`, `cumsum!`, `cumprod`, `cumprod!`, `flipdim`, and `squeeze` ([#25501]). + * `indices(a)` and `indices(a,d)` have been deprecated in favor of `axes(a)` and `axes(a, d)` ([#25057]). diff --git a/base/abstractarraymath.jl b/base/abstractarraymath.jl index 9860888a63379..130dcd74e3b7e 100644 --- a/base/abstractarraymath.jl +++ b/base/abstractarraymath.jl @@ -43,7 +43,7 @@ _sub(t::Tuple, ::Tuple{}) = t _sub(t::Tuple, s::Tuple) = _sub(tail(t), tail(s)) """ - squeeze(A, dims) + squeeze(A; dims) Remove the dimensions specified by `dims` from array `A`. Elements of `dims` must be unique and within the range `1:ndims(A)`. @@ -57,14 +57,15 @@ julia> a = reshape(Vector(1:4),(2,2,1,1)) 1 3 2 4 -julia> squeeze(a,3) +julia> squeeze(a; dims=3) 2×2×1 Array{Int64,3}: [:, :, 1] = 1 3 2 4 ``` """ -function squeeze(A::AbstractArray, dims::Dims) +squeeze(A; dims=throw(ArgumentError("the dimensions to squeeze must be specified with a `dims` keyword argument"))) = _squeeze(A, dims) +function _squeeze(A::AbstractArray, dims::Dims) for i in 1:length(dims) 1 <= dims[i] <= ndims(A) || throw(ArgumentError("squeezed dims must be in range 1:ndims(A)")) length(axes(A, dims[i])) == 1 || throw(ArgumentError("squeezed dims must all be size 1")) @@ -80,9 +81,7 @@ function squeeze(A::AbstractArray, dims::Dims) end reshape(A, d::typeof(_sub(axes(A), dims))) end - -squeeze(A::AbstractArray, dim::Integer) = squeeze(A, (Int(dim),)) - +_squeeze(A::AbstractArray, dim::Integer) = _squeeze(A, (Int(dim),)) ## Unary operators ## diff --git a/base/deprecated.jl b/base/deprecated.jl index 4ad071716d703..4b98f11196cfc 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -1364,6 +1364,8 @@ export readandwrite @deprecate flipdim(A, d) reverse(A, dims=d) +@deprecate squeeze(A, dims) squeeze(A, dims=dims) + # PR #25196 @deprecate_binding ObjectIdDict IdDict{Any,Any} diff --git a/stdlib/SparseArrays/test/sparse.jl b/stdlib/SparseArrays/test/sparse.jl index 4abd7c31d127d..c21fa103a1691 100644 --- a/stdlib/SparseArrays/test/sparse.jl +++ b/stdlib/SparseArrays/test/sparse.jl @@ -162,11 +162,11 @@ end @testset "squeeze" begin for i = 1:5 am = sprand(20, 1, 0.2) - av = squeeze(am, 2) + av = squeeze(am, dims=2) @test ndims(av) == 1 @test all(av.==am) am = sprand(1, 20, 0.2) - av = squeeze(am, 1) + av = squeeze(am, dims=1) @test ndims(av) == 1 @test all(av' .== am) end @@ -1363,7 +1363,7 @@ end @testset "error conditions for reshape, and squeeze" begin local A = sprand(Bool, 5, 5, 0.2) @test_throws DimensionMismatch reshape(A,(20, 2)) - @test_throws ArgumentError squeeze(A,(1, 1)) + @test_throws ArgumentError squeeze(A,dims=(1, 1)) end @testset "float" begin diff --git a/test/arrayops.jl b/test/arrayops.jl index 26045e4371aa6..555777bebb80a 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -237,16 +237,17 @@ end @test vec(b) == vec(a) a = rand(1, 1, 8, 8, 1) - @test @inferred(squeeze(a, 1)) == @inferred(squeeze(a, (1,))) == reshape(a, (1, 8, 8, 1)) - @test @inferred(squeeze(a, (1, 5))) == squeeze(a, (5, 1)) == reshape(a, (1, 8, 8)) - @test @inferred(squeeze(a, (1, 2, 5))) == squeeze(a, (5, 2, 1)) == reshape(a, (8, 8)) - @test_throws ArgumentError squeeze(a, 0) - @test_throws ArgumentError squeeze(a, (1, 1)) - @test_throws ArgumentError squeeze(a, (1, 2, 1)) - @test_throws ArgumentError squeeze(a, (1, 1, 2)) - @test_throws ArgumentError squeeze(a, 3) - @test_throws ArgumentError squeeze(a, 4) - @test_throws ArgumentError squeeze(a, 6) + @test @inferred(squeeze(a, dims=1)) == @inferred(squeeze(a, dims=(1,))) == reshape(a, (1, 8, 8, 1)) + @test @inferred(squeeze(a, dims=(1, 5))) == squeeze(a, dims=(5, 1)) == reshape(a, (1, 8, 8)) + @test @inferred(squeeze(a, dims=(1, 2, 5))) == squeeze(a, dims=(5, 2, 1)) == reshape(a, (8, 8)) + @test_throws ArgumentError squeeze(a) + @test_throws ArgumentError squeeze(a, dims=0) + @test_throws ArgumentError squeeze(a, dims=(1, 1)) + @test_throws ArgumentError squeeze(a, dims=(1, 2, 1)) + @test_throws ArgumentError squeeze(a, dims=(1, 1, 2)) + @test_throws ArgumentError squeeze(a, dims=3) + @test_throws ArgumentError squeeze(a, dims=4) + @test_throws ArgumentError squeeze(a, dims=6) sz = (5,8,7) A = reshape(1:prod(sz),sz...) diff --git a/test/offsetarray.jl b/test/offsetarray.jl index afca91b3668dd..34943bff8499e 100644 --- a/test/offsetarray.jl +++ b/test/offsetarray.jl @@ -300,17 +300,17 @@ am = map(identity, a) # squeeze a0 = rand(1,1,8,8,1) a = OffsetArray(a0, (-1,2,3,4,5)) -@test @inferred(squeeze(a, 1)) == @inferred(squeeze(a, (1,))) == OffsetArray(reshape(a, (1,8,8,1)), (2,3,4,5)) -@test @inferred(squeeze(a, 5)) == @inferred(squeeze(a, (5,))) == OffsetArray(reshape(a, (1,1,8,8)), (-1,2,3,4)) -@test @inferred(squeeze(a, (1,5))) == squeeze(a, (5,1)) == OffsetArray(reshape(a, (1,8,8)), (2,3,4)) -@test @inferred(squeeze(a, (1,2,5))) == squeeze(a, (5,2,1)) == OffsetArray(reshape(a, (8,8)), (3,4)) -@test_throws ArgumentError squeeze(a, 0) -@test_throws ArgumentError squeeze(a, (1,1)) -@test_throws ArgumentError squeeze(a, (1,2,1)) -@test_throws ArgumentError squeeze(a, (1,1,2)) -@test_throws ArgumentError squeeze(a, 3) -@test_throws ArgumentError squeeze(a, 4) -@test_throws ArgumentError squeeze(a, 6) +@test @inferred(squeeze(a, dims=1)) == @inferred(squeeze(a, dims=(1,))) == OffsetArray(reshape(a, (1,8,8,1)), (2,3,4,5)) +@test @inferred(squeeze(a, dims=5)) == @inferred(squeeze(a, dims=(5,))) == OffsetArray(reshape(a, (1,1,8,8)), (-1,2,3,4)) +@test @inferred(squeeze(a, dims=(1,5))) == squeeze(a, dims=(5,1)) == OffsetArray(reshape(a, (1,8,8)), (2,3,4)) +@test @inferred(squeeze(a, dims=(1,2,5))) == squeeze(a, dims=(5,2,1)) == OffsetArray(reshape(a, (8,8)), (3,4)) +@test_throws ArgumentError squeeze(a, dims=0) +@test_throws ArgumentError squeeze(a, dims=(1,1)) +@test_throws ArgumentError squeeze(a, dims=(1,2,1)) +@test_throws ArgumentError squeeze(a, dims=(1,1,2)) +@test_throws ArgumentError squeeze(a, dims=3) +@test_throws ArgumentError squeeze(a, dims=4) +@test_throws ArgumentError squeeze(a, dims=6) # other functions v = OffsetArray(v0, (-3,)) diff --git a/test/subarray.jl b/test/subarray.jl index 947b63965a58f..be0712a3287fe 100644 --- a/test/subarray.jl +++ b/test/subarray.jl @@ -24,7 +24,7 @@ function Agen_slice(A::AbstractArray, I...) push!(sd, i) end end - squeeze(B, sd) + squeeze(B, dims=sd) end _Agen(A, i1) = [A[j1] for j1 in i1] From 30399e56b00409783cedbf0cbe50f33722484d50 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 29 Mar 2018 19:13:54 -0400 Subject: [PATCH 2/2] Use the official required kwarg form I forgot this existed --- base/abstractarraymath.jl | 2 +- test/arrayops.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/base/abstractarraymath.jl b/base/abstractarraymath.jl index 130dcd74e3b7e..c57338139bde4 100644 --- a/base/abstractarraymath.jl +++ b/base/abstractarraymath.jl @@ -64,7 +64,7 @@ julia> squeeze(a; dims=3) 2 4 ``` """ -squeeze(A; dims=throw(ArgumentError("the dimensions to squeeze must be specified with a `dims` keyword argument"))) = _squeeze(A, dims) +squeeze(A; dims) = _squeeze(A, dims) function _squeeze(A::AbstractArray, dims::Dims) for i in 1:length(dims) 1 <= dims[i] <= ndims(A) || throw(ArgumentError("squeezed dims must be in range 1:ndims(A)")) diff --git a/test/arrayops.jl b/test/arrayops.jl index 555777bebb80a..015095c33e4dc 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -240,7 +240,7 @@ end @test @inferred(squeeze(a, dims=1)) == @inferred(squeeze(a, dims=(1,))) == reshape(a, (1, 8, 8, 1)) @test @inferred(squeeze(a, dims=(1, 5))) == squeeze(a, dims=(5, 1)) == reshape(a, (1, 8, 8)) @test @inferred(squeeze(a, dims=(1, 2, 5))) == squeeze(a, dims=(5, 2, 1)) == reshape(a, (8, 8)) - @test_throws ArgumentError squeeze(a) + @test_throws UndefKeywordError squeeze(a) @test_throws ArgumentError squeeze(a, dims=0) @test_throws ArgumentError squeeze(a, dims=(1, 1)) @test_throws ArgumentError squeeze(a, dims=(1, 2, 1))