refactor(python): Expose group_by_dynamic in pyir #19385
Conversation
github-actions
bot
added
internal
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19385 +/- ##
=======================================
Coverage 80.17% 80.17%
=======================================
Files 1528 1528
Lines 210325 210327 +2
Branches 2440 2440
=======================================
+ Hits 168619 168631 +12
+ Misses 41151 41141 -10
Partials 555 555 ☔ View full report in Codecov by Sentry. |
|
|
||
| #[getter] | ||
| fn label(&self, py: Python<'_>) -> PyResult<PyObject> { | ||
| let result = match self.inner.label { |
There was a problem hiding this comment.
We can tag Label with #[derive(IntoStaticStr)]
There was a problem hiding this comment.
question: I did not know about this derive macro previously, so there are a bunch of cases where I do this match dance. Is it worth tagging all those enums in polars with IntoStaticStr and then using this pattern here? This will be a breaking change (not a big deal) in the IR, since the names will change their case-pattern. e.g. StartBy, not start_by.
There was a problem hiding this comment.
We can even control the output with those macros. I believe #[strum(serialize_all = "snake_case")], see: https://docs.rs/strum/latest/strum/additional_attributes/index.html
There was a problem hiding this comment.
Ah, even better. I'll try with that and see if anything breaks locally.
wence-
force-pushed
the
wence/fea/dynamic-groupby-pyir
branch
from
f3ee681
to
324c7ff
Compare
Previously we were just ignoring this, which meant we would sometimes fail to fall back correctly in the GPU engine.
|
FWIW, would be good to get this into 1.11 |
wence-
force-pushed
the
wence/fea/dynamic-groupby-pyir
branch
from
324c7ff
to
7eea93c
Compare
wence-
requested review from
c-peters,
alexander-beedie,
MarcoGorelli,
reswqa and
orlp
as code owners
| @@ -57,7 +57,7 @@ impl NodeTraverser { | |||
| // Increment major on breaking changes to the IR (e.g. renaming | |||
| // fields, reordering tuples), minor on backwards compatible | |||
| // changes (e.g. exposing a new expression node). | |||
| const VERSION: Version = (2, 3); | |||
| const VERSION: Version = (3, 0); | |||
There was a problem hiding this comment.
join type of "leftsemi"/"leftanti" turned into "semi"/"anti"
|
It will make 1.11 :) |
Previously we were just ignoring this, which meant we would sometimes fail to fall back correctly in the GPU engine.