From 830d8c99bbd16a01bcc0fc6052ff65ec687692e5 Mon Sep 17 00:00:00 2001 From: Tony Kelman Date: Sun, 1 Jan 2017 07:32:58 -0800 Subject: [PATCH] Revert "Limit broadcast mechanism over Nullables (#19787)" This reverts commit f147aaac00a8cb3a4b15e2afd3407e260226e9ea. --- NEWS.md | 10 ++-- base/broadcast.jl | 102 ++++++++++++++++++++++++---------- base/nullable.jl | 4 ++ base/sparse/higherorderfns.jl | 4 +- doc/src/manual/types.md | 12 ++++ test/broadcast.jl | 9 +-- test/nullable.jl | 47 ++++++++++++++++ 7 files changed, 145 insertions(+), 43 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7f6e08d07dd1df..3db8df2d210a15 100644 --- a/NEWS.md +++ b/NEWS.md @@ -55,10 +55,13 @@ This section lists changes that do not have deprecation warnings. * `broadcast` now treats `Ref` (except for `Ptr`) arguments as 0-dimensional arrays ([#18965]). - * `broadcast` now handles missing data (`Nullable`s) allowing operations to - be lifted over mixtures of `Nullable`s and scalars, as if the `Nullable` - were like an array with zero or one element. ([#16961], [#19787]). + be lifted over `Nullable`s, as if the `Nullable` were like an array with + zero or one element. ([#16961]). Note that many situations where `Nullable` + types had been treated like scalars before will no longer work. For + example, `get.(xs)` on `xs::Array{T <: Nullable}` will now treat the + nullables as a container, and attempt to operate on the data contained. + This use case will need to be migrated to `map(get, xs)`. * The runtime now enforces when new method definitions can take effect ([#17057]). The flip-side of this is that new method definitions should now reliably actually @@ -801,4 +804,3 @@ Language tooling improvements [#19543]: https://github.com/JuliaLang/julia/issues/19543 [#19598]: https://github.com/JuliaLang/julia/issues/19598 [#19635]: https://github.com/JuliaLang/julia/issues/19635 -[#19787]: https://github.com/JuliaLang/julia/issues/19787 diff --git a/base/broadcast.jl b/base/broadcast.jl index cb470a70f6e4fa..ce9d1f6e66a9ae 100644 --- a/base/broadcast.jl +++ b/base/broadcast.jl @@ -5,12 +5,10 @@ module Broadcast using Base.Cartesian using Base: promote_eltype_op, linearindices, tail, OneTo, to_shape, _msk_end, unsafe_bitgetindex, bitcache_chunks, bitcache_size, dumpbitcache, - nullable_returntype, null_safe_eltype_op, hasvalue + nullable_returntype, null_safe_eltype_op, hasvalue, is_nullable_array import Base: broadcast, broadcast! export broadcast_getindex, broadcast_setindex!, dotview -typealias ScalarType Union{Type{Any}, Type{Nullable}} - ## Broadcasting utilities ## # fallbacks for some special cases @inline broadcast(f, x::Number...) = f(x...) @@ -30,7 +28,8 @@ containertype(::Type) = Any containertype{T<:Ptr}(::Type{T}) = Any containertype{T<:Tuple}(::Type{T}) = Tuple containertype{T<:Ref}(::Type{T}) = Array -containertype{T<:AbstractArray}(::Type{T}) = Array +containertype{T<:AbstractArray}(::Type{T}) = + is_nullable_array(T) ? Array{Nullable} : Array containertype{T<:Nullable}(::Type{T}) = Nullable containertype(ct1, ct2) = promote_containertype(containertype(ct1), containertype(ct2)) @inline containertype(ct1, ct2, cts...) = promote_containertype(containertype(ct1), containertype(ct2, cts...)) @@ -38,20 +37,28 @@ containertype(ct1, ct2) = promote_containertype(containertype(ct1), containertyp promote_containertype(::Type{Array}, ::Type{Array}) = Array promote_containertype(::Type{Array}, ct) = Array promote_containertype(ct, ::Type{Array}) = Array -promote_containertype(::Type{Tuple}, ::ScalarType) = Tuple -promote_containertype(::ScalarType, ::Type{Tuple}) = Tuple +promote_containertype(::Type{Tuple}, ::Type{Any}) = Tuple +promote_containertype(::Type{Any}, ::Type{Tuple}) = Tuple promote_containertype(::Type{Any}, ::Type{Nullable}) = Nullable promote_containertype(::Type{Nullable}, ::Type{Any}) = Nullable +promote_containertype(::Type{Nullable}, ::Type{Array}) = Array{Nullable} +promote_containertype(::Type{Array}, ::Type{Nullable}) = Array{Nullable} +promote_containertype(::Type{Array{Nullable}}, ::Type{Array{Nullable}}) = + Array{Nullable} +promote_containertype(::Type{Array{Nullable}}, ::Type{Array}) = Array{Nullable} +promote_containertype(::Type{Array}, ::Type{Array{Nullable}}) = Array{Nullable} +promote_containertype(::Type{Array{Nullable}}, ct) = Array{Nullable} +promote_containertype(ct, ::Type{Array{Nullable}}) = Array{Nullable} promote_containertype{T}(::Type{T}, ::Type{T}) = T ## Calculate the broadcast indices of the arguments, or error if incompatible # array inputs broadcast_indices() = () broadcast_indices(A) = broadcast_indices(containertype(A), A) -broadcast_indices(::ScalarType, A) = () +broadcast_indices(::Union{Type{Any}, Type{Nullable}}, A) = () broadcast_indices(::Type{Tuple}, A) = (OneTo(length(A)),) broadcast_indices(::Type{Array}, A::Ref) = () -broadcast_indices(::Type{Array}, A) = indices(A) +broadcast_indices{T<:Array}(::Type{T}, A) = indices(A) @inline broadcast_indices(A, B...) = broadcast_shape((), broadcast_indices(A), map(broadcast_indices, B)...) # shape (i.e., tuple-of-indices) inputs broadcast_shape(shape::Tuple) = shape @@ -125,7 +132,9 @@ end Base.@propagate_inbounds _broadcast_getindex(A, I) = _broadcast_getindex(containertype(A), A, I) Base.@propagate_inbounds _broadcast_getindex(::Type{Array}, A::Ref, I) = A[] -Base.@propagate_inbounds _broadcast_getindex(::ScalarType, A, I) = A +Base.@propagate_inbounds _broadcast_getindex(::Type{Any}, A, I) = A +Base.@propagate_inbounds _broadcast_getindex(::Union{Type{Any}, + Type{Nullable}}, A, I) = A Base.@propagate_inbounds _broadcast_getindex(::Any, A, I) = A[I] ## Broadcasting core @@ -276,20 +285,28 @@ ftype(f, A) = typeof(f) ftype(f, A...) = typeof(a -> f(a...)) ftype(T::Type, A...) = Type{T} -typestuple(a) = (Base.@_pure_meta; Tuple{eltype(a)}) -typestuple(T::Type) = (Base.@_pure_meta; Tuple{Type{T}}) -typestuple(a, b...) = (Base.@_pure_meta; Tuple{typestuple(a).types..., typestuple(b...).types...}) +# nullables need to be treated like scalars sometimes and like containers +# other times, so there are two variants of typestuple. + +# if the first argument is Any, then Nullable should be treated like a +# scalar; if the first argument is Array, then Nullable should be treated +# like a container. +typestuple(::Type, a) = (Base.@_pure_meta; Tuple{eltype(a)}) +typestuple(::Type{Any}, a::Nullable) = (Base.@_pure_meta; Tuple{typeof(a)}) +typestuple(::Type, T::Type) = (Base.@_pure_meta; Tuple{Type{T}}) +typestuple{T}(::Type{T}, a, b...) = (Base.@_pure_meta; Tuple{typestuple(T, a).types..., typestuple(T, b...).types...}) -ziptype(A) = typestuple(A) -ziptype(A, B) = (Base.@_pure_meta; Iterators.Zip2{typestuple(A), typestuple(B)}) -@inline ziptype(A, B, C, D...) = Iterators.Zip{typestuple(A), ziptype(B, C, D...)} +# these functions take the variant of typestuple to be used as first argument +ziptype{T}(::Type{T}, A) = typestuple(T, A) +ziptype{T}(::Type{T}, A, B) = (Base.@_pure_meta; Iterators.Zip2{typestuple(T, A), typestuple(T, B)}) +@inline ziptype{T}(::Type{T}, A, B, C, D...) = Iterators.Zip{typestuple(T, A), ziptype(T, B, C, D...)} -_broadcast_type(f, T::Type, As...) = Base._return_type(f, typestuple(T, As...)) -_broadcast_type(f, A, Bs...) = Base._default_eltype(Base.Generator{ziptype(A, Bs...), ftype(f, A, Bs...)}) +_broadcast_type{S}(::Type{S}, f, T::Type, As...) = Base._return_type(f, typestuple(S, T, As...)) +_broadcast_type{T}(::Type{T}, f, A, Bs...) = Base._default_eltype(Base.Generator{ziptype(T, A, Bs...), ftype(f, A, Bs...)}) # broadcast methods that dispatch on the type of the final container @inline function broadcast_c(f, ::Type{Array}, A, Bs...) - T = _broadcast_type(f, A, Bs...) + T = _broadcast_type(Any, f, A, Bs...) shape = broadcast_indices(A, Bs...) iter = CartesianRange(shape) if isleaftype(T) @@ -300,6 +317,13 @@ _broadcast_type(f, A, Bs...) = Base._default_eltype(Base.Generator{ziptype(A, Bs end return broadcast_t(f, Any, shape, iter, A, Bs...) end +@inline function broadcast_c(f, ::Type{Array{Nullable}}, A, Bs...) + @inline rec(x) = broadcast(f, x) + @inline rec(x, y) = broadcast(f, x, y) + @inline rec(x, y, z) = broadcast(f, x, y, z) + @inline rec(xs...) = broadcast(f, xs...) + broadcast_c(rec, Array, A, Bs...) +end function broadcast_c(f, ::Type{Tuple}, As...) shape = broadcast_indices(As...) n = length(shape[1]) @@ -307,7 +331,7 @@ function broadcast_c(f, ::Type{Tuple}, As...) end @inline function broadcast_c(f, ::Type{Nullable}, a...) nonnull = all(hasvalue, a) - S = _broadcast_type(f, a...) + S = _broadcast_type(Array, f, a...) if isleaftype(S) && null_safe_eltype_op(f, a...) Nullable{S}(f(map(unsafe_get, a)...), nonnull) else @@ -323,21 +347,28 @@ end """ broadcast(f, As...) -Broadcasts the arrays, tuples, `Ref`s, nullables, and/or scalars `As` to a +Broadcasts the arrays, tuples, `Ref`, nullables, and/or scalars `As` to a container of the appropriate type and dimensions. In this context, anything -that is not a subtype of `AbstractArray`, `Ref` (except for `Ptr`s), `Tuple`, +that is not a subtype of `AbstractArray`, `Ref` (except for `Ptr`s) or `Tuple`, or `Nullable` is considered a scalar. The resulting container is established by the following rules: - If all the arguments are scalars, it returns a scalar. - If the arguments are tuples and zero or more scalars, it returns a tuple. - - If the arguments contain at least one array or `Ref`, it returns an array - (expanding singleton dimensions), and treats `Ref`s as 0-dimensional arrays, - and tuples as a 1-dimensional arrays. + - If there is at least an array or a `Ref` in the arguments, it returns an array + (and treats any `Ref` as a 0-dimensional array of its contents and any tuple + as a 1-dimensional array) expanding singleton dimensions. -The following additional rule applies to `Nullable` arguments: If there is at -least one `Nullable`, and all the arguments are scalars or `Nullable`, it -returns a `Nullable` treating `Nullable`s as "containers". +The following additional rules apply to `Nullable` arguments: + + - If there is at least a `Nullable`, and all the arguments are scalars or + `Nullable`, it returns a `Nullable`. + - If there is at least an array or a `Ref` with `Nullable` entries, or there + is at least an array or a `Ref` (perhaps with scalar entries instead of + `Nullable` entries) and a nullable, then the result is an array of + `Nullable` entries. + - If there is a tuple and a nullable, the result is an error, as this case is + not currently supported. A special syntax exists for broadcasting: `f.(args...)` is equivalent to `broadcast(f, args...)`, and nested `f.(g.(args...))` calls are fused into a @@ -402,8 +433,21 @@ Nullable{String}("XY") julia> broadcast(/, 1.0, Nullable(2.0)) Nullable{Float64}(0.5) -julia> (1 + im) ./ Nullable{Int}() -Nullable{Complex{Float64}}() +julia> [Nullable(1), Nullable(2), Nullable()] .* 3 +3-element Array{Nullable{Int64},1}: + 3 + 6 + #NULL + +julia> [1+im, 2+2im, 3+3im] ./ Nullable{Int}() +3-element Array{Nullable{Complex{Float64}},1}: + #NULL + #NULL + #NULL + +julia> Ref(7) .+ Nullable(3) +0-dimensional Array{Nullable{Int64},0}: +10 ``` """ @inline broadcast(f, A, Bs...) = broadcast_c(f, containertype(A, Bs...), A, Bs...) diff --git a/base/nullable.jl b/base/nullable.jl index de44f2eba97b51..b785f184e82f61 100644 --- a/base/nullable.jl +++ b/base/nullable.jl @@ -302,6 +302,10 @@ hasvalue(x) = true all(f::typeof(hasvalue), t::Tuple) = f(t[1]) & all(f, tail(t)) all(f::typeof(hasvalue), t::Tuple{}) = true +is_nullable_array(::Any) = false +is_nullable_array{T}(::Type{T}) = eltype(T) <: Nullable +is_nullable_array(A::AbstractArray) = eltype(A) <: Nullable + # Overloads of null_safe_op # Unary operators diff --git a/base/sparse/higherorderfns.jl b/base/sparse/higherorderfns.jl index df4290345a0f83..182f82381c4e59 100644 --- a/base/sparse/higherorderfns.jl +++ b/base/sparse/higherorderfns.jl @@ -73,7 +73,7 @@ function _noshapecheck_map{Tf,N}(f::Tf, A::SparseVecOrMat, Bs::Vararg{SparseVecO fofzeros = f(_zeros_eltypes(A, Bs...)...) fpreszeros = _iszero(fofzeros) maxnnzC = fpreszeros ? min(length(A), _sumnnzs(A, Bs...)) : length(A) - entrytypeC = Base.Broadcast._broadcast_type(f, A, Bs...) + entrytypeC = Base.Broadcast._broadcast_type(Any, f, A, Bs...) indextypeC = _promote_indtype(A, Bs...) C = _allocres(size(A), indextypeC, entrytypeC, maxnnzC) return fpreszeros ? _map_zeropres!(f, C, A, Bs...) : @@ -101,7 +101,7 @@ function _diffshape_broadcast{Tf,N}(f::Tf, A::SparseVecOrMat, Bs::Vararg{SparseV fofzeros = f(_zeros_eltypes(A, Bs...)...) fpreszeros = _iszero(fofzeros) indextypeC = _promote_indtype(A, Bs...) - entrytypeC = Base.Broadcast._broadcast_type(f, A, Bs...) + entrytypeC = Base.Broadcast._broadcast_type(Any, f, A, Bs...) shapeC = to_shape(Base.Broadcast.broadcast_indices(A, Bs...)) maxnnzC = fpreszeros ? _checked_maxnnzbcres(shapeC, A, Bs...) : _densennz(shapeC) C = _allocres(shapeC, indextypeC, entrytypeC, maxnnzC) diff --git a/doc/src/manual/types.md b/doc/src/manual/types.md index cc49741fdb3bec..3f2364adfb8891 100644 --- a/doc/src/manual/types.md +++ b/doc/src/manual/types.md @@ -1426,3 +1426,15 @@ conveniently using `.`-prefixed operators: julia> Nullable(2) ./ Nullable(3) .+ Nullable(1.0) Nullable{Float64}(1.66667) ``` + +[`broadcast()`](@ref) also allows one to work with multiple data at the same +time, without manually writing for loops. This enables performing the same +operation to arrays where the data is possibly missing; for example + +```julia +julia> [Nullable(2), Nullable(), Nullable(3)] .+ 3 +3-element Array{Nullable{Int64},1}: + 5 + #NULL + 6 +``` diff --git a/test/broadcast.jl b/test/broadcast.jl index ea3152a68c685e..43ab80805c6480 100644 --- a/test/broadcast.jl +++ b/test/broadcast.jl @@ -363,7 +363,7 @@ StrangeType18623(x,y) = (x,y) let f(A, n) = broadcast(x -> +(x, n), A) @test @inferred(f([1.0], 1)) == [2.0] - g() = (a = 1; Base.Broadcast._broadcast_type(x -> x + a, 1.0)) + g() = (a = 1; Base.Broadcast._broadcast_type(Any, x -> x + a, 1.0)) @test @inferred(g()) === Float64 end @@ -411,10 +411,3 @@ Base.Broadcast.broadcast_c(f, ::Type{Array19745}, A, Bs...) = @test isa(aa .+ 1, Array19745) @test isa(aa .* aa', Array19745) end - -# broadcast should only "peel off" one container layer -@test get.([Nullable(1), Nullable(2)]) == [1, 2] -let io = IOBuffer() - broadcast(x -> print(io, x), [Nullable(1.0)]) - @test String(take!(io)) == "Nullable{Float64}(1.0)" -end diff --git a/test/nullable.jl b/test/nullable.jl index 7f7ac55c122d27..7e82eec5a8d366 100644 --- a/test/nullable.jl +++ b/test/nullable.jl @@ -507,6 +507,48 @@ end @test Nullable(10.5) === @inferred(broadcast(+, 1, 2, Nullable(3), Nullable(4.0), Nullable(1//2))) +# broadcasting for arrays +@test istypeequal(@inferred(broadcast(+, [1, 2, 3], Nullable{Int}(1))), + Nullable{Int}[2, 3, 4]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[1, 2, 3], 1)), + Nullable{Int}[2, 3, 4]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[1, 2, 3], Nullable(1))), + Nullable{Int}[2, 3, 4]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[1, Nullable()], Nullable(1))), + Nullable{Int}[2, Nullable()]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[Nullable(), 1], + Nullable{Int}())), + Nullable{Int}[Nullable(), Nullable()]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[Nullable(), 1], + Nullable{Int}[1, Nullable()])), + Nullable{Int}[Nullable(), Nullable()]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[Nullable(), 1], + Nullable{Int}[Nullable(), 1])), + Nullable{Int}[Nullable(), 2]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[Nullable(), Nullable()], + Nullable{Int}[1, 2])), + Nullable{Int}[Nullable(), Nullable()]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[Nullable(), 1], + Nullable{Int}[1])), + Nullable{Int}[Nullable(), 2]) +@test istypeequal(@inferred(broadcast(+, Nullable{Float64}[1.0, 2.0], + Nullable{Float64}[1.0 2.0; 3.0 4.0])), + Nullable{Float64}[2.0 3.0; 5.0 6.0]) +@test istypeequal(@inferred(broadcast(+, Nullable{Int}[1, 2], [1, 2], 1)), + Nullable{Int}[3, 5]) + +@test istypeequal(@inferred(broadcast(/, 1, Nullable{Int}[1, 2, 4])), + Nullable{Float64}[1.0, 0.5, 0.25]) +@test istypeequal(@inferred(broadcast(muladd, Nullable(2), 42, + [Nullable(1337), Nullable{Int}()])), + Nullable{Int}[1421, Nullable()]) + +# heterogenous types (not inferrable) +@test istypeequal(broadcast(+, Any[1, 1.0], Nullable(1//2)), + Any[Nullable(3//2), Nullable(1.5)]) +@test istypeequal(broadcast(+, Any[Nullable(1) Nullable(1.0)], Nullable(big"1")), + Any[Nullable(big"2") Nullable(big"2.0")]) + # test fast path taken for op in (+, *, -) for b1 in (false, true) @@ -515,6 +557,11 @@ for op in (+, *, -) @inferred(broadcast(op, Nullable{Int}(1, b1), Nullable{Int}(2, b2))) end + A = [1, 2, 3] + res = @inferred(broadcast(op, A, Nullable{Int}(1, b1))) + @test res[1] === Nullable{Int}(op(1, 1), b1) + @test res[2] === Nullable{Int}(op(2, 1), b1) + @test res[3] === Nullable{Int}(op(3, 1), b1) end end