Remove broadcast_elwise_op and deprecate promote_eltype_op#19814
Remove broadcast_elwise_op and deprecate promote_eltype_op#19814
Conversation
base/deprecated.jl
Outdated
| @deprecate (|)(A::AbstractArray, b::Number) A .| b | ||
| @deprecate (|)(A::AbstractArray, B::AbstractArray) A .| B | ||
|
|
||
| # Calling promote_op is likely a bad idea, so deprecate to its convenience wrapper promote_eltype_op |
There was a problem hiding this comment.
Errr... should read "so deprecate its convenience wrapper". Does that make sense?
|
Oh, the travis i686 job failed due to Full log is here: https://gist.github.com/martinholters/362f5507ee7ded7b59f043c74cc52fbe |
|
Ah, that assertion failure is also what #19803 is about if I'm not mistaken, so unrelated to this PR? |
|
LGTM |
|
Good to go or are there any objections against this? |
|
Note that the existence of this function is not because of convenience, but because the suggested replacement is type-unstable. This will hammer performance of packages using this. |
| end | ||
| _broadcast_zpreserving!(args...) = broadcast!(args...) | ||
| _broadcast_zpreserving(args...) = Base.Broadcast.broadcast_elwise_op(args...) | ||
| _broadcast_zpreserving(f, As...) = broadcast!(f, similar(Array{promote_op(f, map(eltype, As)...)}, Base.Broadcast.broadcast_indices(As...)), As...) |
There was a problem hiding this comment.
It seems bad to have a type-unstable implementation, even if deprecated.
Along similar lines, |
AFAICT,
broadcast_elwise_opwas only used in the deprecation of_broadcast_zpreserving.I've found the following uses of
promote_eltype_op:Compat.promote_eltype_op.Probably the cleanest thing would be to update DataArrays not to rely on
promote_eltype_opand also deprecate it in Compat. Or Compat could just use the definition from the deprecation directly.