RFC: Nullables as collections #16961
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,28 @@ 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, is_nullable_array | ||
| import Base: broadcast, broadcast! | ||
| export bitbroadcast, dotview | ||
| export broadcast_getindex, broadcast_setindex! | ||
|
|
||
| ## Broadcasting utilities ## | ||
|
|
||
| broadcast_array_type() = Array | ||
|
There was a problem hiding this comment. Isn't there a more general mechanism for this? Cc: @pabloferz @martinholters There was a problem hiding this comment. It's used as a marker to select whether to recurse or otherwise. Perhaps that's best done with a different trait. There was a problem hiding this comment. If the new subtyping algorithm gets merged before, I believe you could remove this and have @inline broadcast(f, As::AbstractArray...) = broadcast_c(f, Array, As...)
@inline broadcast(f, As::A...) where A<:AbstractArray{E} where E<: Nullable = broadcast_c(f, Array{Nullable}, As...)
containertype{T<:AbstractArray}(::Type{T}) = Array
containertype{E<:Nullable,T<:AbstractArray{E}}(::Type{T}) = Array{Nullable}There was a problem hiding this comment. Sounds like a more generally useful design indeed. It's likely that these type system improvements will be merged before or at the same time as this PR, since both need to be in before the feature freeze. Though for now let's keep the custom version. There was a problem hiding this comment. I'm going to avoid changing this for now because the subtyping algorithm will likely provide a better way to do it. |
||
| broadcast_array_type(A, As...) = | ||
| if is_nullable_array(A) || broadcast_array_type(As...) === Array{Nullable} | ||
| Array{Nullable} | ||
| else | ||
| Array | ||
| end | ||
|
|
||
| # fallbacks for some special cases | ||
| broadcast(f) = f() | ||
|
There was a problem hiding this comment. this is defined in sysimg.jl now and isn't needed here There was a problem hiding this comment. Thanks, must have missed that in the rebase. |
||
| @inline broadcast(f, x::Number...) = f(x...) | ||
| @inline broadcast{N}(f, t::NTuple{N}, ts::Vararg{NTuple{N}}) = map(f, t, ts...) | ||
| @inline broadcast(f, As::AbstractArray...) = | ||
| broadcast_c(f, broadcast_array_type(As...), As...) | ||
|
|
||
| # special cases for "X .= ..." (broadcast!) assignments | ||
| broadcast!(::typeof(identity), X::AbstractArray, x::Number) = fill!(X, x) | ||
|
|
@@ -30,7 +41,9 @@ 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}) = | ||
| 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...)) | ||
|
|
||
|
|
@@ -39,16 +52,26 @@ promote_containertype(::Type{Array}, ct) = Array | |
| promote_containertype(ct, ::Type{Array}) = Array | ||
| 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}}) = | ||
|
There was a problem hiding this comment. Isn't this one covered by the There was a problem hiding this comment. This is needed to avoid an ambiguity. |
||
| 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(::Union{Type{Any}, Type{Nullable}}, A) = () | ||
|
There was a problem hiding this comment. IIUC, There was a problem hiding this comment. The type information is computed assuming There was a problem hiding this comment. Not sure what this means, I'm not really familiar with that code. @martinholters @pabloferz? There was a problem hiding this comment. I mean that nullable is only treated as a scalar as a special case. |
||
| broadcast_indices(::Type{Tuple}, A) = (OneTo(length(A)),) | ||
| broadcast_indices(::Type{Array}, A::Ref) = () | ||
| 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 | ||
|
|
@@ -123,6 +146,8 @@ 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(::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 | ||
|
|
@@ -272,19 +297,28 @@ end | |
| ftype(f, A) = typeof(f) | ||
| ftype(f, A...) = typeof(a -> f(a...)) | ||
| ftype(T::Type, A...) = Type{T} | ||
|
|
||
| # nullables need to be treated like scalars sometimes and like containers | ||
|
There was a problem hiding this comment. The occurrence of "sometimes" here definitely raises a red flag. |
||
| # other times, so there are two variants of typestuple. | ||
| immutable NullableIsScalar end | ||
| immutable NullableIsContainer end | ||
|
|
||
| typestuple(::Any, a) = (Base.@_pure_meta; Tuple{eltype(a)}) | ||
| typestuple(::NullableIsScalar, a::Nullable) = (Base.@_pure_meta; Tuple{typeof(a)}) | ||
|
There was a problem hiding this comment. I like this solution much better 👍, but I don't think you need to introduce a new type here, you could just use |
||
| typestuple(::Any, T::Type) = (Base.@_pure_meta; Tuple{Type{T}}) | ||
| typestuple(g, a, b...) = (Base.@_pure_meta; Tuple{typestuple(g, a).types..., typestuple(g, b...).types...}) | ||
|
|
||
| # these functions take the variant of typestuple to be used as first argument | ||
| ziptype(g, A) = typestuple(g, A) | ||
| ziptype(g, A, B) = (Base.@_pure_meta; Iterators.Zip2{typestuple(g, A), typestuple(g, B)}) | ||
| @inline ziptype(g, A, B, C, D...) = Iterators.Zip{typestuple(g, A), ziptype(g, B, C, D...)} | ||
|
|
||
| _broadcast_type(g, f, T::Type, As...) = Base._return_type(f, typestuple(g, T, As...)) | ||
| _broadcast_type(g, f, A, Bs...) = Base._default_eltype(Base.Generator{ziptype(g, 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(NullableIsScalar(), f, A, Bs...) | ||
| shape = broadcast_indices(A, Bs...) | ||
| iter = CartesianRange(shape) | ||
| if isleaftype(T) | ||
|
|
@@ -295,27 +329,59 @@ _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]) | ||
| return ntuple(k->f((_broadcast_getindex(A, k) for A in As)...), n) | ||
| end | ||
| @inline function broadcast_c(f, ::Type{Nullable}, a...) | ||
| nonnull = all(hasvalue, a) | ||
| S = _broadcast_type(NullableIsContainer(), f, a...) | ||
| if isleaftype(S) && null_safe_eltype_op(f, a...) | ||
| Nullable{S}(f(map(unsafe_get, a)...), nonnull) | ||
| else | ||
| if nonnull | ||
| Nullable(f(map(unsafe_get, a)...)) | ||
| else | ||
| Nullable{nullable_returntype(S)}() | ||
| end | ||
| end | ||
| end | ||
| @inline broadcast_c(f, ::Type{Any}, a...) = f(a...) | ||
|
|
||
| """ | ||
| broadcast(f, As...) | ||
|
|
||
| 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) 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 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 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 | ||
| single broadcast loop. | ||
|
|
@@ -372,6 +438,28 @@ julia> string.(("one","two","three","four"), ": ", 1:4) | |
| "two: 2" | ||
| "three: 3" | ||
| "four: 4" | ||
|
|
||
| julia> Nullable("X") .* "Y" | ||
| Nullable{String}("XY") | ||
|
|
||
| julia> broadcast(/, 1.0, Nullable(2.0)) | ||
| Nullable{Float64}(0.5) | ||
|
|
||
| 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...) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a breaking change or just a new feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a breaking change, because
Nullables that were treated like scalars in some cases before are now treated like collections. To keepNullables like scalars you need to use theRef(x)or(x,)trick now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe say that here to point out what about it is breaking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out that v0.5 did not support treating nullables as scalars (treating arbitrary things as scalars itself was a 0.6 addition). But there is another thing that would break;
get.(xs)will now try to descend into theNullableinstead of operating on theNullableitself. I added that as an example.