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

Split unique into separate functions #275

Merged
merged 7 commits into from
Oct 21, 2021
Merged

Split unique into separate functions #275

merged 7 commits into from
Oct 21, 2021

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Sep 27, 2021

This PR

  • resolves gh-212 by splitting unique into separate functions with a fixed number of arguments, thus addressing the concerns regarding unique's polymorphic behavior.

  • splits unique in the following functions:

    • unique_all(x, /): returns (1) unique values, (2) indices of first occurrence, (3) reverse indices, and (4) counts. This is the equivalent of the previous unique API with return_index, return_inverse, and return_counts kwargs as True.
    • unique_inverse(x, /): returns (1) unique values and (2) reverse indices. As discussed in the OP for gh-212, this addresses the fairly common use case of setting return_inverse to True in the previous API.
    • unique_values(x, /): only returns (1) unique values. This is the most common use of the previous API.
  • chooses to retain return_index as a return value in unique_all as this returned array does have value in being able to construct the array of unique values and because deriving this returned array is somewhat tedious to construct via the objects in this specification otherwise.

  • eliminates polymorphic return behavior by removing kwargs from the respective APIs which affect which and how many arrays are returned.

  • adds guidance on expected output array shapes.

  • requires that named tuples be returned.

  • does not use the function name unique in order to avoid breaking backward compatibility in array-specification implementations.

@kgryte kgryte added the API change Changes to existing functions or objects in the API. label Sep 27, 2021
@kgryte kgryte added this to the v2021 milestone Oct 4, 2021
@honno

This comment has been minimized.

@rgommers
Copy link
Member

I suggest not mixing the "are NaN's unique" question with the discussion on unique being polymorphic and hence this PR to split it into three separate functions. They are orthogonal problems. Could you move your question to the NaN-unique issue @honno?

@kgryte
Copy link
Contributor Author

kgryte commented Oct 21, 2021

This PR was discussed during the 7 October 2021 consortium meeting. No objections were raised and the proposed changes were seen as positive. Will merge. Any needed changes can be addressed in subsequent PRs.

@kgryte kgryte merged commit f9f9bc4 into main Oct 21, 2021
@kgryte kgryte deleted the unique branch October 21, 2021 00:44
@leofang leofang mentioned this pull request Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Changes to existing functions or objects in the API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Address polymorphic return value for unique?
3 participants