8000 numpy.clip documentation could be misleading for a_max < a_min · Issue #18782 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

numpy.clip documentation could be misleading for a_max < a_min #18782

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.

Sign up for GitHub

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

Jump to bottom
Closed
xamm opened this issue Apr 15, 2021 · 5 comments
Closed

numpy.clip documentation could be misleading for a_max < a_min #18782

xamm opened this issue Apr 15, 2021 · 5 comments
Labels
04 - Documentation

Comments

@xamm
Copy link
Contributor
xamm commented Apr 15, 2021

Documentation

In the documentation for clip() the return type is defined as:

An array with the elements of a, but where values < a_min are replaced with a_min, and those > a_max with a_max.

When a_min is greater than a_max the return values are only defined by a_max:

import numpy as np
arr = np.array([1, -0.25, -1])
arr.clip(0, -0.5)
# returns array([-0.5, -0.5, -0.5])

If a_min is greater than a_max, the description of the return values in the documentation could be misleading, since all values are replaced by a_max.

It might be good to explain this special behavior in the documentation for the case a_max < a_min.

If a change in documentation is desired, I am happy to contribute.
@cooperrc
Copy link
Member
cooperrc commented Apr 15, 2021 •
edited
Loading

It looks like numpy.clip checks for a_min and then amax. Since there is no warning or error, I think it makes sense to add an example where a_min > a_max.

I would suggest adding a Notes section in the docstring and flip the min/max in the first example e.g.

Notes
--------
When `a_min` is more than `a_max`, `clip` will return an 
array where all values are `a_max`, as seen in the second example. 

Examples
--------
>>> a = np.arange(10)
>>> np.clip(a, 1, 8)
array([1, 1, 2, 3, 4, 5, 6, 7, 8, 8])
>>> np.clip(a, a_max = 1, a_min = 8)
array([1, 1, 1, 1, 1, 1, 1, 1, 1, 1])

It would be great to have your contribution. Thanks for pointing out the issue!

@xamm xamm mentioned this issue Apr 16, 2021
Merged
charris added a commit that referenced this issue Apr 21, 2021
@charris
Merge pull request #18786 from xamm/clip-doc-add-note
f048051 
DOC: add note for clip() special case a_min > a_max See #18782
@cooperrc
Copy link
Member

@xamm, awesome work on the docs addition. Can we close this issue now?

@xamm
Copy link
Contributor Author
xamm commented Apr 26, 2021

From my side this issue can be closed. I just was not sure if I should do this by myself.

@cooperrc
Copy link
Member

From my side this issue can be closed. I just was not sure if I should do this by myself.

Thanks again @xamm! We really appreciate your detailed thoughts on this issue.

@melissawm, I think we are ready to close this issue.

@seberg seberg closed this as completed Apr 26, 2021
@seberg
Copy link
Member
seberg commented Apr 26, 2021

@cooperrc if you can't close issues, we have to change that! :)

@rgommers rgommers mentioned this issue Oct 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
04 - Documentation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@seberg @cooperrc @xamm
0