-
Notifications
You must be signed in to change notification settings - Fork 182
feat: Allow scalars in horizontal expressions #3435
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
f777def
5332f33
af06b23
a2c4325
0cad6c1
7365262
ca99472
c91ece5
fd6f5c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -235,7 +235,9 @@ def func(df: PandasLikeDataFrame) -> list[PandasLikeSeries]: | |
|
|
||
| def min_horizontal(self, *exprs: PandasLikeExpr) -> PandasLikeExpr: | ||
| def func(df: PandasLikeDataFrame) -> list[PandasLikeSeries]: | ||
| series = list(chain.from_iterable(expr(df) for expr in exprs)) | ||
| series = self._series._align_full_broadcast( | ||
| *chain.from_iterable(expr(df) for expr in exprs) | ||
| ) | ||
|
Comment on lines
-238
to
+240
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'd like to check if we need to do this align-full-broadcast, eye-balling
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's because we don't allow scalars in coalesce? (Disclaimer: from mobile, I only checked signature) |
||
| return [ | ||
| PandasLikeSeries( | ||
| self.concat( | ||
|
|
@@ -255,7 +257,9 @@ def func(df: PandasLikeDataFrame) -> list[PandasLikeSeries]: | |
|
|
||
| def max_horizontal(self, *exprs: PandasLikeExpr) -> PandasLikeExpr: | ||
| def func(df: PandasLikeDataFrame) -> list[PandasLikeSeries]: | ||
| series = list(chain.from_iterable(expr(df) for expr in exprs)) | ||
| series = self._series._align_full_broadcast( | ||
| *chain.from_iterable(expr(df) for expr in exprs) | ||
| ) | ||
| return [ | ||
| PandasLikeSeries( | ||
| self.concat( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1262,7 +1262,9 @@ def _expr_with_horizontal_op(name: str, *exprs: IntoExpr, **kwargs: Any) -> Expr | |
| ) | ||
|
|
||
|
|
||
| def sum_horizontal(*exprs: IntoExpr | Iterable[IntoExpr]) -> Expr: | ||
| def sum_horizontal( | ||
| *exprs: PythonLiteral | IntoExpr | Iterable[PythonLiteral | IntoExpr], | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will comment here to exemplify the behavior. Polars definition of # Inputs that can convert into a `col` expression
IntoExprColumn: TypeAlias = Union["Expr", "Series", str]
# Inputs that can convert into an expression
IntoExpr: TypeAlias = PythonLiteral | IntoExprColumn | NoneI am not sure if it's reasonable for us to eventually align with those definition and distinction
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this in polars too. I think an issue in |
||
| ) -> Expr: | ||
| """Sum all values horizontally across columns. | ||
|
|
||
| Warning: | ||
|
|
@@ -1296,7 +1298,9 @@ def sum_horizontal(*exprs: IntoExpr | Iterable[IntoExpr]) -> Expr: | |
| return _expr_with_horizontal_op("sum_horizontal", *flatten(exprs)) | ||
|
|
||
|
|
||
| def min_horizontal(*exprs: IntoExpr | Iterable[IntoExpr]) -> Expr: | ||
| def min_horizontal( | ||
| *exprs: PythonLiteral | IntoExpr | Iterable[PythonLiteral | IntoExpr], | ||
| ) -> Expr: | ||
| """Get the minimum value horizontally across columns. | ||
|
|
||
| Notes: | ||
|
|
@@ -1328,7 +1332,9 @@ def min_horizontal(*exprs: IntoExpr | Iterable[IntoExpr]) -> Expr: | |
| return _expr_with_horizontal_op("min_horizontal", *flatten(exprs)) | ||
|
|
||
|
|
||
| def max_horizontal(*exprs: IntoExpr | Iterable[IntoExpr]) -> Expr: | ||
| def max_horizontal( | ||
| *exprs: PythonLiteral | IntoExpr | Iterable[PythonLiteral | IntoExpr], | ||
| ) -> Expr: | ||
| """Get the maximum value horizontally across columns. | ||
|
|
||
| Notes: | ||
|
|
@@ -1426,7 +1432,10 @@ def when(*predicates: IntoExpr | Iterable[IntoExpr]) -> When: | |
| return When(*predicates) | ||
|
|
||
|
|
||
| def all_horizontal(*exprs: IntoExpr | Iterable[IntoExpr], ignore_nulls: bool) -> Expr: | ||
| def all_horizontal( | ||
| *exprs: PythonLiteral | IntoExpr | Iterable[PythonLiteral | IntoExpr], | ||
| ignore_nulls: bool, | ||
| ) -> Expr: | ||
| r"""Compute the bitwise AND horizontally across columns. | ||
|
|
||
| Arguments: | ||
|
|
@@ -1555,7 +1564,10 @@ def lit(value: PythonLiteral, dtype: IntoDType | None = None) -> Expr: | |
| return Expr(ExprNode(ExprKind.LITERAL, "lit", value=value, dtype=dtype)) | ||
|
|
||
|
|
||
| def any_horizontal(*exprs: IntoExpr | Iterable[IntoExpr], ignore_nulls: bool) -> Expr: | ||
| def any_horizontal( | ||
| *exprs: PythonLiteral | IntoExpr | Iterable[PythonLiteral | IntoExpr], | ||
| ignore_nulls: bool, | ||
| ) -> Expr: | ||
| r"""Compute the bitwise OR horizontally across columns. | ||
|
|
||
| Arguments: | ||
|
|
@@ -1603,7 +1615,9 @@ def any_horizontal(*exprs: IntoExpr | Iterable[IntoExpr], ignore_nulls: bool) -> | |
| ) | ||
|
|
||
|
|
||
| def mean_horizontal(*exprs: IntoExpr | Iterable[IntoExpr]) -> Expr: | ||
| def mean_horizontal( | ||
| *exprs: PythonLiteral | IntoExpr | Iterable[PythonLiteral | IntoExpr], | ||
| ) -> Expr: | ||
| """Compute the mean of all values horizontally across columns. | ||
|
|
||
| Arguments: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to broadcast scalars first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangotbanned unlike the arrow case, here we need to align the series for how we implement the operation after