Deprecate vectorized imag methods in favor of compact broadcast syntax #18566
Conversation
| elseif which == "SI" | ||
| dmap = x->-imag(x) | ||
| dmap = x -> (-).(imag.(x)) |
There was a problem hiding this comment.
have or are we planning on deprecating vectorized unary minus for arrays?
There was a problem hiding this comment.
Not that I am aware of. Exploiting fusion was the objective above. Are you suggesting that we should consider deprecating vectorized unary minus? Best!
There was a problem hiding this comment.
No, no, I just forgot that fusion would make this worth doing. It looks funny is all.
There was a problem hiding this comment.
Quite funny it looks indeed :). Would imag.((-).(x)) look better to you?
There was a problem hiding this comment.
I think that map(t -> -imag(t), x) is simpler to read in this case.
There was a problem hiding this comment.
That looks like a winking owl. The way you have it here gets rid of the real component before negating. Not sure if the compiler would elide negating the real component if you did it the other way around.
There was a problem hiding this comment.
Agreed, the explicit map is simple and clear. A similar compact broadcast form would be (t -> -imag(t)).(x). Preferences? Best!
There was a problem hiding this comment.
Switched to map(t -> -imag(t), x) for now, but of course am happy to change to something else if need be. Thanks!
kshyatt
added
complex
|
Closing per discussion in #18495 (comment). Best! |
This PR deprecates all remaining vectorized
imagmethods (apart from those forSparseVectors, separate PR) in favor of compact broadcast syntax. Ref. #16285, #17302, #18495, #18512, #18513, #18558, and #18564. Best!(Unlike with
float,real, etc., the remaining vectorizedimagmethods never alias their input. This PR should be less controversial than #18495, #18512, and #18513 as a result.)