BUG: Fix numpy.ma.median. #8016
Conversation
|
Needs tests, just putting up now for reference. |
| if asorted.mask is nomask: | ||
| counts = asorted.shape[axis] | ||
| else: | ||
| counts = asorted.shape[axis] - (asorted.mask).sum(axis=axis) |
There was a problem hiding this comment.
purely stylistic, but why the parentheses?
charris
force-pushed
the
fix-ma-median
branch
from
affc441
to
5e07cdc
Compare
|
|
||
| counts = asorted.shape[axis] - (asorted.mask).sum(axis=axis) | ||
| if asorted.mask is nomask: | ||
| counts = asorted.shape[axis] |
There was a problem hiding this comment.
This gets the wrong dimension... The following does work:
counts = np.full(asorted.shape[:axis] + asorted.shape[axis+1:],
asorted.shape[axis])
There was a problem hiding this comment.
The above also solves the astropy test problems.
There was a problem hiding this comment.
How about the count function? I note that it also handles a tuple of axes, which median currently does not.
def count(self, axis=None, keepdims=np._NoValue):
"""
Count the non-masked elements of the array along the given axis.
The count function is already imported and used for the 1-d case in _median.
There was a problem hiding this comment.
well, almost all problems... I think the remaining ones are ours, though...
|
OK, I've made use of the |
|
|
||
| if asorted.ndim == 1: | ||
| idx, odd = divmod(count(asorted), 2) | ||
| return asorted[idx - (not odd) : idx + 1].mean() | ||
| return asorted[idx + odd - 1 : idx + 1].mean() |
There was a problem hiding this comment.
It is a bit strange that below (https://github.com/numpy/numpy/pull/8016/files#diff-6fd4523c4f6741d8d9e9a20d14561729R745) for inexact dtype the mean is done by summing and then dividing, and here this is ignored. It is out of scope of this PR, though.
|
@sharris - it is indeed not the nicest bit of code, but I think within its context what you've done makes sense (I like the use of |
|
Just wanted to say that with this PR the astropy-tests are fixed (at least locally). 👍 |
charris
force-pushed
the
fix-ma-median
branch
4 times, most recently
from
a9dc9f4
to
07538cf
Compare
|
OK, I think this is ready to go. If anyone wants to run extras tests that would be nice. |
This fixes four bugs:
- Use of booleans for indexing in ma.extras._median
- Error when mask is nomask and ndim != 1 in ma.extras._median
- Did not work for 0-d arrays.
- No out argument for {0,1}-d arrays.
Closes numpy#8015.
charris
force-pushed
the
fix-ma-median
branch
from
07538cf
to
89f02f0
Compare
|
@charris - all looks OK to me. As @MSeifert04 noted, with this PR, the astropy tests pass again. (I assume your last push of 11 minutes ago was just for edits to the commits, not actual changes.) |
|
@mhvk Edited a commit message and fixed the error message for one of the tests. |
charris
force-pushed
the
fix-ma-median
branch
from
89f02f0
to
ad5b13a
Compare
This fixes four bugs:
Closes #8015.