Simplify and extend broadcast eltype promotion mechanism #19723
Conversation
|
|
||
| _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...)}) | ||
| eltypestuple(a) = (Base.@_pure_meta; Tuple{eltype(a)}) |
There was a problem hiding this comment.
What do you think of moving these either to abstractarray.jl or promotion.jl?
There was a problem hiding this comment.
Moving eltypestuple to abstractarray.jl or promotion.jl? _broadcast_eltype being the only consumer of eltypestuple, wherever _broadcast_eltype lives seems like the appropriate location? Or do you have other (broader) uses in mind for eltypesuple? Or did you mean something else altogether? Thanks for reviewing! :)
There was a problem hiding this comment.
Sorry for commenting above without providing a context. I was thinking that we could change the promote_op methods to take a function and the tuple of the eltypes and remove the promote_op(Any...) = Any, so it would make sense having eltypestuple somewhere in Base. But that might fit better in another PR.
There was a problem hiding this comment.
Sounds like a good plan! Agreed another PR might be better. Perhaps we can chat about cleaning up these mechanisms after feature freeze? Thanks again!
|
Rewritten with the changes to |
|
(Pinging @pabloferz for review. Thanks in advance!) |
| # other times, so there are two variants of typestuple. | ||
| # _broadcast_eltype is broadcast's primary result-eltype promotion mechanism. | ||
| # _broadcast_eltype uses eltypestuple to construct a tuple type of the eltypes | ||
| # of the input-array arguments passed to _broadcast_eltype (from an upstream broacast). |
There was a problem hiding this comment.
That word. Fixed. Thanks!
Sacha0
force-pushed
the
bceltype
branch
3 times, most recently
from
c113bea to
150c651
Compare
|
Rebased and fixed. Collides with #19787, not certain which should take precedence. Best! |
|
If no one objects I'd prefer to merge that one first. |
…re cases. Re-simplify broadcast's eltype promotion mechanism as in JuliaLang#19421. With benefit of JuliaLang#19667, this simplified mechanism should handle additional cases (e.g. closures accepting more than two arguments). Also rename the mechanism more precisely (_broadcast_type -> _broadcast_eltype, typestuple -> eltypestuple).
|
i686 travis failure looks related to #19792, most likely pre existing. |
(Requires #19667.) This pull request restores the simplification of
broadcast's eltype promotion mechanism in #19421. With benefit of #19667 (thanks@yuyichao!), that simplification handles more cases (e.g. closures accepting more than two arguments) than the present mechanism (_default_eltype-first-Generator-zip). This pull request also givesbroadcast's eltype promotion mechanism a more precise name (_broadcast_type->_broadcast_eltype). Best!