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.

Sign up for GitHub

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

Jump to bottom

feat(api): support median and quantile on more types #7810

Merged
merged 2 commits into from
Dec 20, 2023
Merged

feat(api): support median and quantile on more types #7810

merged 2 commits into from
Dec 20, 2023

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Dec 20, 2023 •
edited
Loading

Adds support for calling median and quantile on more types than numeric.

Tests are included for computing the median of dates + strings.

Closes #7250.

@cpcloud cpcloud added this to the 8.0 milestone Dec 20, 2023
@cpcloud cpcloud added feature Features or general enhancements postgres The PostgreSQL backend duckdb The DuckDB backend snowflake The Snowflake backend oracle The Oracle backend labels Dec 20, 2023
@cpcloud cpcloud force-pushed the generic-quantiles branch 3 times, most recently from 51119e3 to 7fa987c Compare December 20, 2023 11:31
@cpcloud cpcloud changed the title feat(api): support order statistics on more types feat(api): support median and quantile on more types Dec 20, 2023
@cpcloud cpcloud removed the snowflake The Snowflake backend label Dec 20, 2023
@cpcloud cpcloud force-pushed the generic-quantiles branch 8 times, most recently from 3537ec0 to 999f666 Compare December 20, 2023 12:16
@cpcloud cpcloud requested a review from gforsyth December 20, 2023 12:16
@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023 •
edited
Loading

Bleh, the github's download artifact action v4 seems incredibly unreliable right now.

xref: actions/download-artifact#249

@cpcloud cpcloud force-pushed the generic-quantiles branch 2 times, most recently from adb604f to 7b27017 Compare December 20, 2023 13:36
@ibis-project ibis-project deleted a comment from github-actions bot Dec 20, 2023
@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Ok, the commit linter is successfully not commenting on PRs with valid titles and descriptions.

I'll edit the PR title to be invalid to try the case where we want a comment to show up

@cpcloud cpcloud changed the title feat(api): support median and quantile on more types port median and quantile on more types Dec 20, 2023
@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Sweet, that's working.

@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Last case to check is that the linter isn't spamming comments when additional edits don't fix the issue.

@cpcloud cpcloud changed the title port median and quantile on more types sport median and quantile on more types Dec 20, 2023
@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Ok, looks like it's a bit spammy! Fixing it...

@cpcloud cpcloud changed the title sport median and quantile on more types spot median and quantile on more types Dec 20, 2023
@cpcloud cpcloud changed the title spot median and quantile on more types try to avoid spam Dec 20, 2023
@ibis-project ibis-project deleted a comment from github-actions bot Dec 20, 2023
@ibis-project ibis-project deleted a comment from github-actions bot Dec 20, 2023
@ibis-project ibis-project deleted a comment from github-actions bot Dec 20, 2023
@cpcloud cpcloud changed the title try to avoid spam try to avoid spams Dec 20, 2023
@github-actions GitHub Actions
Copy link
Contributor

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Ok, I think the spam factory has been shutdown.

One more try

@cpcloud cpcloud changed the title try to avoid spams really, gimme the spam Dec 20, 2023
gforsyth
gforsyth approved these changes Dec 20, 2023
View reviewed changes
Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

Looks good!

@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

I had one more edit, which was to actually fail the check if the message is bogus!

@cpcloud cpcloud changed the title really, gimme the spam failures everywhere Dec 20, 2023
@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Ok, looks like that's working. Lemme fix the title, and then we can try squash merging 😬

@cpcloud cpcloud changed the title failures everywhere feat(api): support median and quantile on more types Dec 20, 2023
@cpcloud
Copy link
Member Author

cpcloud commented Dec 20, 2023

Ok, squashing 😬

@cpcloud cpcloud merged commit 49c75a8 into ibis-project:master Dec 20, 2023
89 of 94 checks passed
@cpcloud cpcloud deleted the generic-quantiles branch December 20, 2023 17:30
mfatihaktas pushed a commit to mfatihaktas/ibis that referenced this pull request Dec 20, 2023
@cpcloud @mfatihaktas
feat(api): support median and quantile on more types (ibis-project#7810)
fa504ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gforsyth gforsyth gforsyth approved these changes

Assignees
No one assigned
Labels
duckdb The DuckDB backend feature Features or general enhancements oracle The Oracle backend postgres The PostgreSQL backend
Projects
None yet
Milestone
8.0
Development

Successfully merging this pull request may close these issues.

feat: DateValue.median() .quantile()
2 participants
@cpcloud @gforsyth