MAINT: make np.ma.apply_along_axis consistent with np.apply_along_axis #8511
Conversation
5ccdc5b to
43f3c79
Compare
|
☔ The latest upstream changes (presumably #8614) made this pull request unmergeable. Please resolve the merge conflicts. |
43f3c79 to
c8a4815
Compare
|
☔ The latest upstream changes (presumably #8584) made this pull request unmergeable. Please resolve the merge conflicts. |
This changes the behavior somewhat, but not in ways related to masked arrays It would previously try to find a suitable dtype for all values, now it just uses the first dtype.
c8a4815 to
72f88cd
Compare
numpy/ma/extras.py
Outdated
| return result | ||
| def wrapped_func(a, *args, **kwargs): | ||
| res = func1d(a, *args, **kwargs) | ||
| return np.asanyarray(res).view(masked_array) |
There was a problem hiding this comment.
Hmm, I note that the original only returned a masked array when arr had the _mask attribute.
There was a problem hiding this comment.
To me, it seems to make sense to always return a MaskedArray, since that is more or less the point of having this different function. However, we should not presuppose the output type, i.e., if the output is already a MaskedArray subclass, there is no reason to change it. So, would this be covered by just returning [np.ma.]asanyarray(res)?
There was a problem hiding this comment.
I think you're right, this should be np.ma.asanyarray
|
This is certainly a vast simplification of the original, which seems unduly complicated. However, the results may differ from the original in more than just #8352, which worries me. |
|
If this goes in there should also be a mention in the |
numpy/ma/extras.py
Outdated
| return result | ||
| def wrapped_func(a, *args, **kwargs): | ||
| res = func1d(a, *args, **kwargs) | ||
| return np.asanyarray(res).view(masked_array) |
There was a problem hiding this comment.
To me, it seems to make sense to always return a MaskedArray, since that is more or less the point of having this different function. However, we should not presuppose the output type, i.e., if the output is already a MaskedArray subclass, there is no reason to change it. So, would this be covered by just returning [np.ma.]asanyarray(res)?
|
I think here there again was only a trivial fix to be made. Want to do it? It certainly remains a nice cleanup. p.s. By the way, I was just trying to find your comment about the signature you'd like for gufuncs where the output shape depends on the inputs... |
|
@mhvk: Done |
|
The circle-ci failure seems strange; has the setup changed? If so, a rebase would solve it, which also allows to squash the two commits. Anyway, all looks OK otherwise. |
|
Let's give this a shot. Eric, could you add a bit to the released notes and squash the commits. Maybe rebase in the process. |
|
My worry is that we've steered people towards np.ma.apply_along_axis to work around #8352, meaning we really ought to provide them with a different workaround before merging this. |
|
@eric-wieser What do you want to do with this? |
|
I'll try to revisit this one soon to at least solve the conflicts. I think to actually fix the cutting strings problem we need a way to request a specific dtype, which is something that is impossible to do without creating a new function as One option would be Another quick fix might just be to provide |
This transfers the fixes made in #8441 to
np.ma.apply_along_axes.This comes with a behaviour change - previously, the
maversion would not fall afoul of #8352.