Potential fix for #6462 #6527
Potential fix for #6462 #6527
Conversation
|
Can you add a test case to make sure it stays fixed? To figure out where, my usual strategy is to grep the other test files to find some existing tests for median. |
|
@njsmith Added some tests, I think I did it right, and it's looks like they're working as expected. |
|
@ethankruse Take a look at |
|
The commit message should also be more descriptive, something along the lines |
|
@charris Thanks for the style guideline tip. Should be updated and just about ready to go now. |
| with warnings.catch_warnings(record=True) as w: | ||
| warnings.filterwarnings('always', '', RuntimeWarning) | ||
| assert_equal(np.median(a, axis=0), b) | ||
| assert_equal(np.median(a, axis=1), b) |
There was a problem hiding this comment.
Do you need to check w[0] here?
There was a problem hiding this comment.
Ah, no, those actually don't produce the warnings, just empty arrays. I had put everything into the catch blocks by default, but for those I guess it's meaningless. I can drop it and recommit.
np.median([]) returns NaN. Fixes bug/regression that raised an IndexError. Added tests to ensure continued support of empty arrays.
|
OK, merged. Thanks @ethankruse . |
This fixes the IndexError when taking the median of an empty array in the new median implementation (#6462). Taking the median over an axis of size 0 now returns a NaN as it had in previous implementations instead of the IndexError.