Skip to content

Add unique_counts() and fix the description of unique_all() #317

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 5 commits into from
Nov 8, 2021

Conversation

leofang
Copy link
Contributor

@leofang leofang commented Nov 5, 2021

Close #291.

This PR:

@leofang leofang requested a review from kgryte November 5, 2021 15:30
@leofang leofang added this to the v2021 milestone Nov 5, 2021
@leofang leofang added API extension Adds new functions or objects to the API. Maintenance Bug fix, typo fix, or general maintenance. labels Nov 5, 2021
@leofang
Copy link
Contributor Author

leofang commented Nov 5, 2021

hmmm why does this PR only have 1 check instead of 3?

@kgryte
Copy link
Contributor

kgryte commented Nov 5, 2021

@leofang Not sure why it has only one check. 🧐

@leofang leofang closed this Nov 5, 2021
@leofang leofang reopened this Nov 5, 2021
@leofang
Copy link
Contributor Author

leofang commented Nov 5, 2021

Not sure why it has only one check. 🧐

My guess is the other two CIs only run for this repo's branches, not for branches from forks. There is a piece of permission bit that needs to be set (at least it's the case for GH actions).

@kgryte
Copy link
Contributor

kgryte commented Nov 5, 2021

Ah, right. This was probably intentional in order to avoid drive-by PRs consuming resources.

@leofang leofang changed the title Add unique_counts and apply a few fixes to unique_*() functions Add unique_counts() and fix the description of unique_all() Nov 8, 2021
@leofang
Copy link
Contributor Author

leofang commented Nov 8, 2021

I have reverted all changes on the returned integer type (discussion continued in #319) to make this PR easier to review 🙂

Copy link
Contributor

@kgryte kgryte left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @leofang!

@kgryte kgryte merged commit b6f5ad7 into data-apis:main Nov 8, 2021
@leofang leofang deleted the unique_count branch November 8, 2021 07:05
@leofang
Copy link
Contributor Author

leofang commented Nov 8, 2021

Thanks, @kgryte, good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API extension Adds new functions or objects to the API. Maintenance Bug fix, typo fix, or general maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unique_counts()?
3 participants