BUG: fixed failure of np.ma.median for 1-D even arrays. #8414
Conversation
For such arrays, the sum of the two entries closest to the middle was not divided by 2. Now fixed, with test cases adapted to ensure this stays OK.
|
hm |
|
added a commit which fixes the small inconsistency and added more tests |
| s = np.true_divide(s, 2., casting='safe', out=out) | ||
| # masked ufuncs do not fullfill returned is out | ||
| # fix this to return the same in the nd path | ||
| if out is not None: |
There was a problem hiding this comment.
If out is not None, I believe it is returned by true_divide so that s has already been set.
There was a problem hiding this comment.
it is not which is why I have added that code
maybe a bug of masked arrays or a feature of ndarray subtypes
There was a problem hiding this comment.
Hmm, so the comment says this is not the case for masked?
There was a problem hiding this comment.
The problem only seems to occur when out is a MaskedArray.
| np.true_divide(s, 2., casting='unsafe') | ||
| if not odd: | ||
| s = np.true_divide(s, 2., casting='safe', out=out) | ||
| # masked ufuncs do not fullfill returned is out |
There was a problem hiding this comment.
This need clarification. np.true_divide is not a masked function, but you are saying that s is not out in some circumstances? When precisely does that happen?
There was a problem hiding this comment.
masked array __array_wrap__ creates a view of the object so it still references the same data but it is not the same object
There was a problem hiding this comment.
That does mean __array_wrap__ is not implemented correctly (which at least for Quantity I found not all that trivial to do...). But anyway, that deserves a separate issue: see #8416.
| else: | ||
| assert_equal(out, 15.) | ||
| assert_(r is out) | ||
| assert_(type(r) == MaskedArray) |
There was a problem hiding this comment.
Should be is ? Of course, singletons can be a bit hazardous...
There was a problem hiding this comment.
due to the is check earlier this is actually unnecessary but it does serve as extra documentation on the output type
There was a problem hiding this comment.
I think @charris just meant that it should be type(r) is MaskedArray (which I think is correct).
| np.true_divide(s, 2., casting='unsafe') | ||
| if not odd: | ||
| s = np.true_divide(s, 2., casting='safe', out=out) | ||
| # masked ufuncs do not fullfill returned is out |
There was a problem hiding this comment.
That does mean __array_wrap__ is not implemented correctly (which at least for Quantity I found not all that trivial to do...). But anyway, that deserves a separate issue: see #8416.
| s = np.true_divide(s, 2., casting='safe', out=out) | ||
| # masked ufuncs do not fullfill returned is out | ||
| # fix this to return the same in the nd path | ||
| if out is not None: |
There was a problem hiding this comment.
The problem only seems to occur when out is a MaskedArray.
| else: | ||
| assert_equal(out, 15.) | ||
| assert_(r is out) | ||
| assert_(type(r) == MaskedArray) |
There was a problem hiding this comment.
I think @charris just meant that it should be type(r) is MaskedArray (which I think is correct).
| mask=[True]*3 + [False]*4 + [True]*3) | ||
| assert_equal(r, e) | ||
| assert_(r is out) | ||
| assert_(type(r) == MaskedArray) |
juliantaylor
force-pushed
the
ma/median-even-number-of-elements-fix
branch
from
9179a86
to
6d52633
Compare
@charris: I went ahead and made changes that should solve the problem with even-numbered 1-d arrays. It turned out to be slightly trickier than we thought (although the missing
outis still the culprit, the division by 2 should obviously not be done for odd number of elements; also, we cannot uses.datasince it is a scalar).@juliantaylor - if you could have a look, it would be much appreciated.
fixes #8409