BUG: ma.median alternate fix for #7592 #7635
Conversation
| @@ -653,6 +662,9 @@ def _median(a, axis=None, out=None, overwrite_input=False): | |||
| elif axis < 0: | |||
| axis += a.ndim | |||
|
|
|||
| if (asorted.ndim==1): | |||
|
@ahaldane Ping. |
|
Either solution (this or #7592) seems fine to me, at least with the suggested changes in either PR. On the plus side this PR avoids the convoluted isinstance checks, on the downside it introduces a separate branch to the code. Let's not be like "Burdian's Ass" who can't choose a pile of hay! I think to tidy it up you should follow @charris' sugestions. I also think it might be better not to have a separate I'd also suggest you change |
|
(see also my comment above in @charris's thread) |
|
Looks good to me! I would like to merge. @AmitAronovitch one or two final things: I would get rid of the parenthesis in Further, I plan to squash everything together if you don't do it for me. If you do end up squashing, could you format the commit message according to the guidelines? In the commit message you should also include the sentence "Fixes #5969." so that git auto-closes it for us. |
Fixes numpy#5969. Performance fix numpy#4760 had caused wrong shaped results in the 1D case. This fix restores the original 1D behavior.
Recommended type for nd-indexing is a tuple. See numpy#4434
ca36412 to
a4cc361
Compare
|
I kept the casting to tuple out of the squashed commit, since it is independent of the bugfix. |
|
Looks good, thanks a lot for all the revisions @AmitAronovitch! Merging now. |
Please see discussion in #7592
For the 1d case: use the original solution (copied from original code, before merge of #4760)
For the n-dim case: leave implementation as before.