Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC: Updated documentation wording and examples for np.percentile. #7125

Merged
merged 2 commits into from
Jan 29, 2016

Conversation

madphysicist
Copy link
Contributor

Examples had some factual errors. Fixed up 80-char columns. Wording updated in a couple of places.

Examples had some factual errors. Wording updated in a couple of places.
instead). If the input contains integers, or floats of smaller
precision than 64, then the output data-type is float64. Otherwise,
the output data-type is the same as that of the input.
If `q` is a single percentile and `axis=None`, then the return is a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be the return value or the result rather than the return

@madphysicist
Copy link
Contributor Author

@shoyer I have accepted all of your comments. While we are on the subject, please let me know if the following appears to be a bug or if it is just me:

>>> x = np.arange(5)
>>> x
array([0, 1, 2, 3, 4])
>>> np.percentile(x, (25, 75), interpolation='linear')
array([ 1.,  3.])
>>> np.percentile(x, (25, 75), interpolation='higher')
array([1, 3])
>>> np.percentile(x, (25, 75), interpolation='lower')
array([1, 3])
>>> np.percentile(x, (25, 75), interpolation='nearest')
array([1, 3])
>>> np.percentile(x, (25, 75), interpolation='midpoint')
array([ 1.5,  3.5])

Specifically, how is 'linear' ending up on an integer boundary when there are five elements in the array? How is 'midpoint' giving fractional values when 'higher' and 'lower' coincide?

@shoyer
Copy link
Member

shoyer commented Jan 27, 2016

@madawilliams I think linear is right here -- these percentiles are exact (to within floating point precision).

I don't know what's going on with midpoint. I would look at the relevant code and check for unit tests and/or discussion when it was last changed (use git blame to figure that out).

@shoyer
Copy link
Member

shoyer commented Jan 27, 2016

This LGTM. I'll give other folks a day or two to comment and then merge.

@madphysicist
Copy link
Contributor Author

I did not think this was major enough to post on the mailing list. If you disagree, I will post it.

I did some research and all of the interpolation methods look correct except midpoint. Midpoint is definitely messed up in the example I gave and I have added a patch (#7129). Is there any point to opening an issue now that I have the PR?

@shoyer
Copy link
Member

shoyer commented Jan 27, 2016

Agreed, no need to ping the mailing list for this issue or open a separate issue for midpoint now that you have a PR.

shoyer added a commit that referenced this pull request Jan 29, 2016
DOC: Updated documentation wording and examples for np.percentile.
@shoyer shoyer merged commit 39164ac into numpy:master Jan 29, 2016
@shoyer
Copy link
Member

shoyer commented Jan 29, 2016

Thanks!

@charris we should consider backporting this doc fix to 1.11.

@charris charris added this to the 1.11.0 release milestone Jan 29, 2016
@madphysicist madphysicist deleted the percentile-doc branch January 29, 2016 22:07
@charris charris removed this from the 1.11.0 release milestone Feb 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants