API: Partially revert unique with return_inverse#26914
Merged
charris merged 1 commit intonumpy:mainfrom Jul 13, 2024
Merged
Conversation
There was a good argument that it is not possible to reconstruct the original array with `axis=None` without first reshaping and changing the result shape helped with it. However, it was always possible to do it for other axis values by using `np.take` rather than `np.take_along_axis`. Changing it for all axis values is unnecessary to achieve reconstruction because `np.take(arr, inverse, axis=axis)` already performed the job except for `axis=None`. Thus, this keeps the change for axis=None, but reverts numpygh-25553 for numerical axis.
Member
|
Thanks Sebastian. |
dfm
added a commit
to dfm/jax
that referenced
this pull request
Jul 15, 2024
In numpy/numpy#26914, the behavior of the `return_inverse` argument to `np.unique` was partially reverted to the pre-v2.0 behavior. The PR brings JAX's implementation compatible with the `numpy>2.0.0` behavior.
Member
|
@seberg Can we close data-apis/array-api#820? |
Member
Author
|
We/I wanted to align both functions IMO, the behavior here is strictly clearer, so I think it would make sense to align with NumPy there. For |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a good argument that it is not possible to reconstruct the original array with
axis=Nonewithout first reshaping and changing the result shape helped with it.However, it was always possible to do it for other axis values by using
np.takerather thannp.take_along_axis. Changing it for all axis values is unnecessary to achieve reconstruction becausenp.take(arr, inverse, axis=axis)already performed the job except foraxis=None.Thus, this keeps the change for axis=None, but reverts gh-25553 for numerical axis.
I tried to go through some of the linked PRs, I extremely rough estimate is that at least 2/3 of the time, this would have save a code change. There are a few others that just need flattened (scipy because of a later call, sklearn/cuml because they use a view trick assuming the result is 1-D when it is not). So in many (I am sure not all), of the cases where this doesn't avoid a regression, it is very clean to reshape explicitly.
@charris I am not sure about release notes for 2.0.1, I don't think we incorporate the fragments, but this one might be worth stressing slightly more. I thought I'd add a
noteto the old release notes (even if we may not rebuild them for a while).