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

refactor: pass ExprMetadata down to compliant Exprs from narwhals.Expr #1848

Open
MarcoGorelli opened this issue Jan 21, 2025 · 0 comments
Open
Labels

Comments

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Jan 21, 2025

Currently, we track expression metadata at the compliant level:

  • whether it aggregates, changes length, transforms
  • the function name

This is quite error-prone, as we repeat logic in quite a few places.

I'd prefer it if:

  • we didn't track the function name at all for most backends. It's only really needed for pandas/pyarrow/dask because of limitations in their group-by APIs, so we should only do it there
  • one of the metadatas we could track is what kind of column selection we start with (nth, col, selector, all). Because in group_by-agg, selector and all require some special treatment. But this could be done much more simply at the Narwhals level, without tracking function names everywhere in DuckDB / PySpark / Ibis / whatever else we add, which I hope would also have a modern enough syntax

To do this, I haven't tried, but perhaps:

  • _to_compliant_expr could take a metadata argument, which takes a TypeDict of metadata which we keep track of at the narwhals/expr.py and narwhals/functions.py level
  • compliant expressions just read from the metadata passed by above to determine how and whether to broadcast
@MarcoGorelli MarcoGorelli changed the title refactor: pass _aggregates down to compliant Exprs from narwhals.Expr refactor: pass ExprMetadata down to compliant Exprs from narwhals.Expr Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant