refactor: Get Column into polars-expr
#19660
Conversation
coastalwhite
requested review from
ritchie46,
orlp and
c-peters
as code owners
github-actions
bot
added
internal
| let length = output_length(l, r)?; | ||
| match (l, r) { | ||
| (Column::Series(l), Column::Scalar(r)) => { | ||
| let r = r.as_single_value_series(); |
There was a problem hiding this comment.
This was not functioning properly for series with length=0, so I replaced it with Column::try_apply_broadcasting_binary_elementwise.
| self.as_materialized_series() | ||
| .bitand(rhs.as_materialized_series()) | ||
| .map(Column::from) | ||
| } | ||
| pub fn bitor(&self, rhs: &Self) -> PolarsResult<Self> { |
There was a problem hiding this comment.
I don't really understand why we have these and std::ops::BitAnd, etc. on Series.
There was a problem hiding this comment.
Me neither. Maybe we can try to remove it in a follow op.
| @@ -15,7 +15,7 @@ impl Series { | |||
| } | |||
|
|
|||
| #[doc(hidden)] | |||
| pub fn agg_valid_count(&self, groups: &GroupsProxy) -> Series { | |||
| pub unsafe fn agg_valid_count(&self, groups: &GroupsProxy) -> Series { | |||
There was a problem hiding this comment.
This was before incorrectly marked as safe
| @@ -139,8 +139,8 @@ impl Executor for JoinExec { | |||
|
|
|||
| let df = df_left._join_impl( | |||
| &df_right, | |||
| left_on_series, | |||
| right_on_series, | |||
| left_on_series.into_iter().map(|c| c.take_materialized_series()).collect(), | |||
There was a problem hiding this comment.
I don't really think this is a very large optimization problem, but hopefully this can be resolved when Column is fully in polars-expr.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19660 +/- ##
==========================================
- Coverage 79.86% 79.74% -0.12%
==========================================
Files 1537 1541 +4
Lines 211923 212214 +291
Branches 2446 2446
==========================================
- Hits 169249 169229 -20
- Misses 42120 42431 +311
Partials 554 554 ☔ View full report in Codecov by Sentry. |
|
Nice! |
This starts the moving of
Columnintopolars-expr. I tried to keep this PR to a minimal, and will do the follow-up in other PRs.For example, what this PR allows:
This can now finally produces a
Column::Scalarinstead of having to materialize to aColumn::Series.