DEP: Deprecate nonzero(0d) in favor of calling atleast_1d explicitly #13708
Conversation
This is a weird enough corner case that either: * No one is running into it, and downstream code is broken on 0d arrays anyway * People already have hacks in place to work around it: * Calling `nonzero` but handling the result specially if the array was 0d. We want to dissuade this in case we change the result of nonzero to be better in future. * Not calling `nonzero` at all if the array is zerod (such as by using `atleast1d`). These users are unaffected.
eric-wieser
force-pushed
the
deprecate-nonzero
branch
from
829e196
to
f427b61
Compare
|
LGTM CI seems stuck |
|
@jhelie: Any idea why this PR is getting duplicate LGTM runs with different icons? |
|
There should be some mention of this in the docstring for Should this deprecation hit the mailing list? |
|
Probably a good idea to hit the list with this, although I don't expect anyone to care. Which section of the |
|
See commit 19f0c77 for a suggestion, feel free to delete/modify |
numpy/core/fromnumeric.py
Outdated
| @@ -1774,7 +1774,8 @@ def nonzero(a): | |||
| transpose(nonzero(a)) | |||
|
|
|||
| The result of this is always a 2-D array, with a row for | |||
| each non-zero element. | |||
| each non-zero element. If there is concern the input may be a 0-d array, | |||
| use `nonzero(atleast_1d(a))` instead. | |||
There was a problem hiding this comment.
I don't think we should be mentioning this next to the transpose(nonzero(a)) example, I think the change in #13610 does a better job of that section.
There was a problem hiding this comment.
I don't think 0-d arrays are important enough to justify their own section here, but we should have a passing mention for them somewhere.
There was a problem hiding this comment.
The positioning of this statement is particularly confusing because it seems to suggest that nonzero(atleast_1d(x)) is a substitute for transpose(nonzero(x))
@eric-wieser Our guess is that it might have been a transient issue due to when the update to the new App occurred. It shouldn't happen again (I see the latest analysis is not duplicated) but please do ping me again if it does. |
|
Obviously also affects the method, but that already refers to the function documentation. I'm happy with just going for this. |
eric-wieser
force-pushed
the
deprecate-nonzero
branch
from
19f0c77
to
7da5e64
Compare
Also indicate that the current behavior in this case is deprecated. This also removes the advice to use `a[nonzero(a)]` or `transpose(nonzero(a))`, for which there are better spellings.
eric-wieser
force-pushed
the
deprecate-nonzero
branch
from
7da5e64
to
dcf9d96
Compare
|
@mattip: Adjusted the docstring to be closer to what I had in mind. |
|
Milestoning as 1.17.0 because I'd like not to have to update the documentation to say 1.18 unless we consciously decide not to put this in. |
|
As noted above, I'm happy to put this in. Will wait another day just in case someone objects. |
This is a weird enough corner case that either:
nonzerobut handling the result specially if the array was 0d. We want to dissuade this in case we change the result of nonzero to be better in future.nonzeroat all if the array is zerod (such as by usingatleast1d). These users are unaffected.Follows up on #13632