Skip to content

API: Always allow sorted=False and make a note about it #28503

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

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

seberg
Copy link
Member

@seberg seberg commented Mar 14, 2025

User intent for sorted=False is very clear, so I think we can really just allow it.
However, note in the docs that it could change. Also add a bigger note in the unique_* functions that still always return a sorted result but would change in the future (whenever sorted=False does work).


This is a small follow up to gh-26018 and gh-28493 (and the original addition of the unique_* family of functions, that always should have warned about the potential of API evolution).

User intent for `sorted=False` is very clear, so I think we can
really just allow it.
However, note in the docs that it could change.  Also add a bigger
note in the `unique_*` functions that still always return a sorted
result but would change in the future (whenever `sorted=False` does
work).
@seberg
Copy link
Member Author

seberg commented Mar 14, 2025

CC @adrinjalali for awareness.

Copy link
Contributor

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Fair enough! LGTM.

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Yes, this makes sense. Might as well warn that it may change.

@mhvk mhvk merged commit 2126e3c into numpy:main Mar 14, 2025
73 checks passed
@seberg seberg deleted the unsorted-unique branch March 14, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants