BUG: Fixed 'midpoint' interpolation of np.percentile in odd cases. #7129
Conversation
| @@ -2055,7 +2055,7 @@ def compare_results(res, desired): | |||
| assert_array_equal(res[i], desired[i]) | |||
|
|
|||
|
|
|||
| class TestScoreatpercentile(TestCase): | |||
| class TestPercentile(TestCase): | |||
There was a problem hiding this comment.
There does not appear to be any good reason to name the test otherwise than with the name of the function being tested in this case. Perhaps the function name changed at some point and the test was updated?
There was a problem hiding this comment.
Yes,I think this was ported from a scipy function with that name.
16c47a7 to
d0f16e4
Compare
|
@seberg I think it can go in, but perhaps note in |
|
Agreed, @madphysicist can you add a short note in |
|
Given that this is an incorrect-results bug, I'm tempted to relax my usual
|
|
@madphysicist Could you squash the commits into one? EDIT: into two, leave the notes bit separate. |
'midpoint' must return the same as 'higher' and 'lower' when the two are the same, not 'lower' + 0.5 as it was doing.
db0e816 to
9ec694b
Compare
|
I really don't think many people use other interpolators than the default one of 'linear', and if they do, it is most likely to do 'higher' or 'lower', not 'midpoint'. Otherwise I am sure someone would have noticed the incorrect behavior by now. I only noticed it because I was trying to figure out what exactly a percentile is based on some simple cases :) |
|
Also, I just checked out |
|
Ah, so you don't think this needs a backport? |
|
I don't think a backport would hurt, but I think the odds of it helping anything much are extremely slim. It's really not my decision. |
|
Well, lets leave it for 1.12 then. It is easy to get carried away with backports. |
…olation BUG: Fixed 'midpoint' interpolation of np.percentile in odd cases.
|
@charris this is a bugfix and 0.11 is not released yet, I don't see a reason to delay this, there is no evidence yet software is relying on the incorrect result. The pandas issue was just adapting the testsuite to the correct result. |
interpolation='midpoint'must return the same asinterpolation='higher'andinterpolation='lower'when the two are the same, not 'lower' + 0.5 as it was doing.Another way of putting it: the 50th percentile of an array with an odd number of elements is the middle of the (sorted) array, no matter what the
interpolationargument is.I updated the test to deal with odd instead of even number of elements in the input array.
If this fix gets backported, it should go back as far as 1.0.9, when the
interpolationkeyword was introduced.