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

feat(api): support more types in where arg to reductions #9246

Merged
merged 2 commits into from
May 24, 2024

Conversation

jcrist
Copy link
Member

@jcrist jcrist commented May 24, 2024

All reductions in ibis take an optional columnar where argument - if present, this is used to filter the input before performing the reduction.

Previously we accepted only actual Value inputs here, or deferred expressions. This means that the following would work:

In [1]: import ibis

In [2]: t = ibis.table({"a": "int", "b": "int", "c": "bool"})

In [3]: r1 = t.a.min(where=t.b > 10)  # literal expression

In [4]: r2 = t.a.min(where=_.b > 10)  # deferred expression, resolved against `t`

But not string inputs (would reference columns in the parent table by name), or callables (the lambda form of a deferred expression). Instead, all other inputs were coerced to a boolean (meaning t.a.min(where="bool_col") was surprisingly the same as t.a.min(where=True), but not t.a.min(where=_.bool_col)).

This PR:

  • Expands the scope of values that are allowed in where args to column reductions to include direct values (t.bool_col), deferred expressions (_.bool_col), callables (lambda t: t.bool_col), or string column names ("bool_col"). All other inputs are coerced to a literal.
  • Applies the same to table reductions (t.count() or t.nunique()). In this case we just make use of the existing bind functionality.
  • Adds expression-level tests to check the where arg is being properly handled in all reductions. In several cases it wasn't.

This is a precursor leading to #9170, where we want to add additional args to some columnar reductions that bind to the parent table.

@jcrist jcrist requested review from gforsyth May 24, 2024 17:29
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.

Sweet!

I'll let you merge this one, just in case you'd like to rebase and keep the two commits separate.

@gforsyth gforsyth added the expressions Issues or PRs related to the expression API label May 24, 2024
@jcrist jcrist merged commit 582165f into ibis-project:main May 24, 2024
76 checks passed
@jcrist jcrist deleted the support-more-types-in-where branch May 24, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expressions Issues or PRs related to the expression API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants