From f9598e10c874ddd794d81259b571ecae655e6983 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Mon, 6 Aug 2018 13:18:27 +0100 Subject: [PATCH] Remove tricky array/broadcasting/indexing deprecations (#28458) * Remove scalar .= deprecation * Remove to_index(::Bool) deprecation * broadcasting now falls back to iteration (fixes #23197, fixes #23746) --- base/broadcast.jl | 6 +-- base/deprecated.jl | 58 --------------------- base/indices.jl | 3 +- base/multidimensional.jl | 3 +- stdlib/LinearAlgebra/test/uniformscaling.jl | 6 +++ test/arrayops.jl | 12 ++--- test/broadcast.jl | 35 ++++++++----- test/compiler/compiler.jl | 2 +- 8 files changed, 41 insertions(+), 84 deletions(-) diff --git a/base/broadcast.jl b/base/broadcast.jl index 4d6629a02eb11..3e826c5fc7fb3 100644 --- a/base/broadcast.jl +++ b/base/broadcast.jl @@ -605,9 +605,9 @@ broadcastable(x::Union{Symbol,AbstractString,Function,UndefInitializer,Nothing,R broadcastable(x::Ptr) = Ref{Ptr}(x) # Cannot use Ref(::Ptr) until ambiguous deprecation goes through broadcastable(::Type{T}) where {T} = Ref{Type{T}}(T) broadcastable(x::Union{AbstractArray,Number,Ref,Tuple,Broadcasted}) = x -# In the future, default to collecting arguments. TODO: uncomment once deprecations are removed -# broadcastable(x) = collect(x) -# broadcastable(::Union{AbstractDict, NamedTuple}) = error("intentionally unimplemented to allow development in 1.x") +# Default to collecting iterables — which will error for non-iterables +broadcastable(x) = collect(x) +broadcastable(::Union{AbstractDict, NamedTuple}) = throw(ArgumentError("broadcasting over dictionaries and `NamedTuple`s is reserved")) ## Computation of inferred result type, for empty and concretely inferred cases only _broadcast_getindex_eltype(bc::Broadcasted) = Base._return_type(bc.f, eltypes(bc.args)) diff --git a/base/deprecated.jl b/base/deprecated.jl index e5228e5fa4ebd..cae24f7d9b413 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -575,33 +575,6 @@ function toc() return t end -# A[I...] .= with scalar indices should modify the element at A[I...] -function Broadcast.dotview(A::AbstractArray, args::Number...) - depwarn("the behavior of `A[I...] .= X` with scalar indices will change in the future. Use `A[I...] = X` instead.", :broadcast!) - view(A, args...) -end -Broadcast.dotview(A::AbstractArray{<:AbstractArray}, args::Integer...) = getindex(A, args...) -# Upon removing deprecations, also enable the @testset "scalar .=" in test/broadcast.jl - -# indexing with A[true] will throw an argument error in the future -function to_index(i::Bool) - depwarn("indexing with Bool values is deprecated. Convert the index to an integer first with `Int(i)`.", (:getindex, :setindex!, :view)) - convert(Int,i)::Int -end -# After deprecation is removed, enable the @testset "indexing by Bool values" in test/arrayops.jl -# Also un-comment the new definition in base/indices.jl - -# Broadcast no longer defaults to treating its arguments as scalar (#) -@noinline function Broadcast.broadcastable(x) - depwarn(""" - broadcast will default to iterating over its arguments in the future. Wrap arguments of - type `x::$(typeof(x))` with `Ref(x)` to ensure they broadcast as "scalar" elements. - """, (:broadcast, :broadcast!)) - return Ref{typeof(x)}(x) -end -@eval Base.Broadcast Base.@deprecate_binding Scalar DefaultArrayStyle{0} false -# After deprecation is removed, enable the fallback broadcastable definitions in base/broadcast.jl - # deprecate BitArray{...}(shape...) constructors to BitArray{...}(undef, shape...) equivalents @deprecate BitArray{N}(dims::Vararg{Int,N}) where {N} BitArray{N}(undef, dims) @deprecate BitArray(dims::NTuple{N,Int}) where {N} BitArray(undef, dims...) @@ -1376,28 +1349,6 @@ function slicedim(A::AbstractVector, d::Integer, i::Number) end end -# PR #26347: Deprecate implicit scalar broadcasting in setindex! -_axes(::Ref) = () -_axes(x) = axes(x) -setindex_shape_check(X::Base.Iterators.Repeated, I...) = nothing -function deprecate_scalar_setindex_broadcast_message(v, I...) - value = (_axes(Base.Broadcast.broadcastable(v)) == () ? "x" : "(x,)") - "using `A[I...] = x` to implicitly broadcast `x` across many locations is deprecated. Use `A[I...] .= $value` instead." -end -deprecate_scalar_setindex_broadcast_message(v, ::Colon, ::Vararg{Colon}) = - "using `A[:] = x` to implicitly broadcast `x` across many locations is deprecated. Use `fill!(A, x)` instead." - -function _iterable(v, I...) - depwarn(deprecate_scalar_setindex_broadcast_message(v, I...), :setindex!) - Iterators.repeated(v) -end -function setindex!(B::BitArray, x, I0::Union{Colon,UnitRange{Int}}, I::Union{Int,UnitRange{Int},Colon}...) - depwarn(deprecate_scalar_setindex_broadcast_message(x, I0, I...), :setindex!) - B[I0, I...] .= (x,) - B -end - - # PR #26283 @deprecate contains(haystack, needle) occursin(needle, haystack) @deprecate contains(s::AbstractString, r::Regex, offset::Integer) occursin(r, s, offset=offset) @@ -1493,15 +1444,6 @@ end depwarn("using similar(f, shape...) to call `f` with axes `shape` is deprecated; call `f` directly and/or add methods such that it supports axes", :similar) f(to_shape(dims)) end -# Deprecate non-integer/axis arguments to zeros/ones to match fill/trues/falses -@deprecate zeros(::Type{T}, dims...) where {T} zeros(T, convert(Dims, dims)...) -@deprecate zeros(dims...) zeros(convert(Dims, dims)...) -@deprecate zeros(::Type{T}, dims::NTuple{N, Any}) where {T, N} zeros(T, convert(Dims, dims)) -@deprecate zeros(dims::Tuple) zeros(convert(Dims, dims)) -@deprecate ones(::Type{T}, dims...) where {T} ones(T, convert(Dims, dims)...) -@deprecate ones(dims...) ones(convert(Dims, dims)...) -@deprecate ones(::Type{T}, dims::NTuple{N, Any}) where {T, N} ones(T, convert(Dims, dims)) -@deprecate ones(dims::Tuple) ones(convert(Dims, dims)) # Deprecate varargs size: PR #26862 @deprecate size(x, d1::Integer, d2::Integer) (size(x, d1), size(x, d2)) diff --git a/base/indices.jl b/base/indices.jl index 2292d2da94f97..adec164f080b5 100644 --- a/base/indices.jl +++ b/base/indices.jl @@ -231,8 +231,7 @@ indexing behaviors. This must return either an `Int` or an `AbstractArray` of `Int`s. """ to_index(i::Integer) = convert(Int,i)::Int -# TODO: Enable this new definition after the deprecations introduced in 0.7 are removed -# to_index(i::Bool) = throw(ArgumentError("invalid index: $i of type $(typeof(i))")) +to_index(i::Bool) = throw(ArgumentError("invalid index: $i of type $(typeof(i))")) to_index(I::AbstractArray{Bool}) = LogicalIndex(I) to_index(I::AbstractArray) = I to_index(I::AbstractArray{<:Union{AbstractArray, Colon}}) = diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 19d2b60b90d7f..4677e8649a680 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -632,11 +632,10 @@ function _setindex!(l::IndexStyle, A::AbstractArray, x, I::Union{Real, AbstractA A end -_iterable(X::AbstractArray, I...) = X @generated function _unsafe_setindex!(::IndexStyle, A::AbstractArray, x, I::Union{Real,AbstractArray}...) N = length(I) quote - x′ = unalias(A, _iterable(x, I...)) + x′ = unalias(A, x) @nexprs $N d->(I_d = unalias(A, I[d])) idxlens = @ncall $N index_lengths I @ncall $N setindex_shape_check x′ (d->idxlens[d]) diff --git a/stdlib/LinearAlgebra/test/uniformscaling.jl b/stdlib/LinearAlgebra/test/uniformscaling.jl index 8cadceae4a525..52271177a1f96 100644 --- a/stdlib/LinearAlgebra/test/uniformscaling.jl +++ b/stdlib/LinearAlgebra/test/uniformscaling.jl @@ -256,4 +256,10 @@ end @test (I - LL')\[[0], [0], [1]] == (I - LL)'\[[0], [0], [1]] == fill([1], 3) end +# Ensure broadcasting of I is an error (could be made to work in the future) +@testset "broadcasting of I (#23197)" begin + @test_throws MethodError I .+ 1 + @test_throws MethodError I .+ [1 1; 1 1] +end + end # module TestUniformscaling diff --git a/test/arrayops.jl b/test/arrayops.jl index 9199d0fce4e29..7c7ae0404510b 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -2388,7 +2388,8 @@ end test_zeros(zeros(Int, (2, 3)), Matrix{Int}, (2,3)) # #19265" - @test_throws Any zeros(Float64, [1.]) # TODO: Tighten back up to MethodError once 0.7 deprecations are removed + @test_throws MethodError zeros(Float64, [1.]) + @test_throws MethodError ones(Float64, [0, 0]) end # issue #11053 @@ -2431,11 +2432,10 @@ end @test_throws BoundsError checkbounds(zeros(2,3,0), 2, 3) end -# TODO: Enable this testset after the deprecations introduced in 0.7 are removed -# @testset "indexing by Bool values" begin -# @test_throws ArgumentError zeros(2)[false] -# @test_throws ArgumentError zeros(2)[true] -# end +@testset "indexing by Bool values" begin + @test_throws ArgumentError zeros(Float64, 2)[false] + @test_throws ArgumentError zeros(Float64, 2)[true] +end @testset "issue 24707" begin @test eltype(Vector{Tuple{V}} where V<:Integer) >: Tuple{Integer} diff --git a/test/broadcast.jl b/test/broadcast.jl index 0f3fe58a53edd..8225f873952f0 100644 --- a/test/broadcast.jl +++ b/test/broadcast.jl @@ -591,6 +591,18 @@ end @test broadcast(==, 1, AbstractArray) == false end +@testset "broadcasting falls back to iteration (issues #26421, #19577, #23746)" begin + @test_throws ArgumentError broadcast(identity, Dict(1=>2)) + @test_throws ArgumentError broadcast(identity, (a=1, b=2)) + @test_throws ArgumentError length.(Dict(1 => BitSet(1:2), 2 => BitSet(1:3))) + @test_throws MethodError broadcast(identity, Base) + + @test broadcast(identity, Iterators.filter(iseven, 1:10)) == 2:2:10 + d = Dict([1,2] => 1.1, [3,2] => 0.1) + @test length.(keys(d)) == [2,2] + @test Set(exp.(Set([1,2,3]))) == Set(exp.([1,2,3])) +end + # Test that broadcasting identity where the input and output Array shapes do not match # yields the correct result, not merely a partial copy. See pull request #19895 for discussion. let N = 5 @@ -651,18 +663,17 @@ end @test_throws DimensionMismatch (1, 2) .+ (1, 2, 3) end -# TODO: Enable after deprecations introduced in 0.7 are removed. -# @testset "scalar .=" begin -# A = [[1,2,3],4:5,6] -# A[1] .= 0 -# @test A[1] == [0,0,0] -# @test_throws ErrorException A[2] .= 0 -# @test_throws MethodError A[3] .= 0 -# A = [[1,2,3],4:5] -# A[1] .= 0 -# @test A[1] == [0,0,0] -# @test_throws ErrorException A[2] .= 0 -# end +@testset "scalar .=" begin + A = [[1,2,3],4:5,6] + A[1] .= 0 + @test A[1] == [0,0,0] + @test_throws ErrorException A[2] .= 0 + @test_throws MethodError A[3] .= 0 + A = [[1,2,3],4:5] + A[1] .= 0 + @test A[1] == [0,0,0] + @test_throws ErrorException A[2] .= 0 +end # Issue #22180 @test convert.(Any, [1, 2]) == [1, 2] diff --git a/test/compiler/compiler.jl b/test/compiler/compiler.jl index 0aa149411ae7a..570d5d683939a 100644 --- a/test/compiler/compiler.jl +++ b/test/compiler/compiler.jl @@ -571,7 +571,7 @@ f5575() = zeros5575(Type[Float64][1], 1) @test Base.return_types(f5575, ())[1] == Vector g5575() = zeros(Type[Float64][1], 1) -@test_broken Base.return_types(g5575, ())[1] == Vector # This should be fixed by removing deprecations +@test Base.return_types(g5575, ())[1] == Vector # make sure Tuple{unknown} handles the possibility that `unknown` is a Vararg