Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit broadcast mechanism over Nullables #19787

Merged
merged 5 commits into from
Jan 1, 2017
Merged

Limit broadcast mechanism over Nullables #19787

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e0f1cb1
Limit broadcast mechanism over Nullables
pabloferz Dec 30, 2016
e5c270a
Test that broadcast doesn't propagate too much
pabloferz Dec 31, 2016
f3aec3b
[ci skip] Update manual section on Nullables
pabloferz Dec 31, 2016
dcba165
Address @TotalVerb comments
pabloferz Dec 31, 2016
afdd247
Address @Sacha0 comments
pabloferz Dec 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address @TotalVerb comments
  • Loading branch information
@pabloferz
pabloferz committed Dec 31, 2016
commit dcba165869f6de3db4448a8778c567bb9c997d10
13 changes: 3 additions & 10 deletions base/broadcast.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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
nullable_returntype, null_safe_eltype_op, hasvalue
import Base: broadcast, broadcast!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the is_nullable_array import can be removed, and probably the function itself too (in base/nullable.jl).

export broadcast_getindex, broadcast_setindex!, dotview

Expand Down Expand Up @@ -276,17 +276,10 @@ 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
# 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(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...})

# these functions take the variant of typestuple to be used as first argument
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...)}
Expand Down Expand Up @@ -332,8 +325,8 @@ end

Broadcasts the arrays, tuples, `Ref`, nullables, and/or scalars `As` to a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not from this pull request, but should `Ref` be plural `Ref`s for consistency?

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`
is considered a scalar. The resulting container is established by
that is not a subtype of `AbstractArray`, `Ref` (except for `Ptr`s), `Tuple`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Serial/Oxford) comma after 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.
Expand Down
4 changes: 0 additions & 4 deletions base/nullable.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,10 +302,6 @@ 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

Expand Down