Remove eltype_plus #17398
Remove eltype_plus #17398
Conversation
|
You could try |
Yes, that would require a |
|
Has anyone added |
No, but that sounds like a good idea indeed. |
| body = gen_broadcast_body_sparse(op, true) | ||
| OP = Symbol(string(op)) | ||
| @eval begin | ||
| function ($OP){Tv1,Ti1,Tv2,Ti2}(A_1::SparseMatrixCSC{Tv1,Ti1}, A_2::SparseMatrixCSC{Tv2,Ti2}) | ||
| if size(A_1,1) != size(A_2,1) || size(A_1,2) != size(A_2,2) | ||
| throw(DimensionMismatch("")) | ||
| end | ||
| Tv = ($pro)(A_1, A_2) | ||
| Tv = promote_eltype_op($op, A_1, A_2) |
There was a problem hiding this comment.
Since you have the types as parameters in the function is better to use promote_op directly. E.g. Tv = promote_op($op, Tv1, Tv2). Also if #17389 gets merged promote_eltype_op should return a better type in general but it won't be inferrable (the same applies below).
There was a problem hiding this comment.
Since you have the types as parameters in the function is better to use
promote_opdirectly.
Not sure why, but I don't see any harm either, update is coming.
martinholters
force-pushed
the
rem_eltype_plus
branch
from
4c16c57
to
04aac31
Compare
|
Preconditions for merging this:
|
|
Ready from my side, but no need to rush it into 0.5, I guess. |
|
Do tag 0.5.x if you think it should get into the 0.5 release at some point. |
|
It's probably safe to do now since we were careful about it, but even safer to wait until we branch and not backport the deletion since there may be unregistered code using it. |
Left over from #17313.
Note that DataFrames uses
eltype_plus, so either this needs a deprecation (but am I right that I cannot put that indeprecated.jlbecause it has to go in theBroadcastsubmodule?) or DataFrames should get some lead time to usepromote_eltype_op(+, ...)instead.