Repository navigation
Bump polars upper bound to <1.40 #22048
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
Changes from 55 commits
59d344e
4b40d81
22524d6
714fefe
fa3c810
a84384d
fbb4e57
f9181e8
3582005
8892885
a6c4404
1143dbc
97be357
93470c7
7abf5b1
7defd7c
2b7babc
3ca8964
e59bc97
f6c0dde
e823983
0b4b063
72009df
1f72afd
0ef9dcb
94330c8
d3801f7
b13b15a
9c4762f
9ede121
5b10326
d54780f
4011aa4
3615f52
a9161c5
3b36212
948b3a8
387dd84
5b6471b
7b50a4c
2250f60
a20a021
27c31a3
463d172
13831f7
0d83811
f7e909e
63550e8
18d3ea9
767735e
05d1946
74b731e
d2a018d
97d052b
445b5ac
0fb5680
66fdb71
c0e23eb
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 |
|---|---|---|
|
|
@@ -240,15 +240,14 @@ def do_evaluate( | |
| if maintain_order: | ||
| column = column.sorted_like(values) | ||
| return column | ||
| elif self.name == "set_sorted": # pragma: no cover | ||
| # TODO: LazyFrame.set_sorted is proper IR concept (ie. FunctionIR::Hint) | ||
| # and is is currently not implemented. We should reimplement it as a MapFunction. | ||
| elif self.name == "set_sorted": | ||
|
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. FYI @rjzamora Polars optimizer selectivity hint "set_sorted" is implemented in this PR |
||
| (column,) = (child.evaluate(df, context=context) for child in self.children) | ||
| (asc,) = self.options | ||
| if isinstance(self.options[0], str): | ||
| descending = self.options[0] == "descending" # pragma: no cover | ||
| else: | ||
| descending, _ = self.options | ||
| order = ( | ||
| plc.types.Order.ASCENDING | ||
| if asc == "ascending" | ||
| else plc.types.Order.DESCENDING | ||
| plc.types.Order.DESCENDING if descending else plc.types.Order.ASCENDING | ||
| ) | ||
| null_order = plc.types.NullOrder.BEFORE | ||
| if column.null_count > 0 and (n := column.size) > 1: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| from cudf_polars.utils.versions import ( | ||
| POLARS_VERSION_LT_136, | ||
| POLARS_VERSION_LT_138, | ||
| POLARS_VERSION_LT_139, | ||
| ) | ||
|
|
||
| if TYPE_CHECKING: | ||
|
|
@@ -45,6 +46,29 @@ | |
| __all__ = ["Translator", "translate_named_expr"] | ||
|
|
||
|
|
||
| def _align_decimal_float_for_comparison( | ||
| *operands: expr.Expr, | ||
| ) -> tuple[expr.Expr, ...]: | ||
| """ | ||
| Cast Decimal operands to Float64 when mixed with Float operands. | ||
|
|
||
| libcudf does not produce correct results for Decimal vs Float | ||
| comparisons. This matches Polars' supertype resolution where | ||
| Decimal + Float yields Float64. | ||
| """ | ||
| has_decimal = any(plc.traits.is_fixed_point(op.dtype.plc_type) for op in operands) | ||
| has_float = any(plc.traits.is_floating_point(op.dtype.plc_type) for op in operands) | ||
| if has_decimal and has_float: | ||
| f64 = DataType(pl.Float64()) | ||
| return tuple( | ||
| expr.Cast(f64, False, op) # noqa: FBT003 | ||
| if plc.traits.is_fixed_point(op.dtype.plc_type) | ||
| else op | ||
| for op in operands | ||
| ) | ||
| return operands | ||
|
|
||
|
|
||
| def _check_compression(data: bytes) -> str | None: | ||
| # Vendored from Polars' SupportedCompression::check in | ||
| # polars-io/src/utils/compression.rs | ||
|
|
@@ -125,7 +149,7 @@ def translate_ir(self, *, n: int | None = None) -> ir.IR: | |
| # IR is versioned with major.minor, minor is bumped for backwards | ||
| # compatible changes (e.g. adding new nodes), major is bumped for | ||
| # incompatible changes (e.g. renaming nodes). | ||
| if (version := self.visitor.version()) >= (12, 1): | ||
| if (version := self.visitor.version()) >= (12, 2): | ||
| e = NotImplementedError( | ||
| f"No support for polars IR {version=}" | ||
| ) # pragma: no cover; no such version for now. | ||
|
|
@@ -247,6 +271,17 @@ def __exit__(self, *args: Any) -> None: | |
| self.translator._expr_context = self._prev | ||
|
|
||
|
|
||
| def _is_dynamic_pred(visitor: Any, expr_ir: Any) -> bool: | ||
|
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. FYI: |
||
| try: | ||
| visitor.view_expression(expr_ir.node) | ||
| except Exception as e: | ||
| if str(e) == "dynamic_pred": | ||
| return True | ||
| raise # pragma: no cover | ||
| else: | ||
| return False | ||
|
|
||
|
|
||
| @singledispatch | ||
| def _translate_ir(node: Any, translator: Translator, schema: Schema) -> ir.IR: | ||
| raise NotImplementedError( | ||
|
|
@@ -307,7 +342,7 @@ def _(node: plrs._ir_nodes.Scan, translator: Translator, schema: Schema) -> ir.I | |
| if typ in ("csv", "ndjson"): | ||
| for p in paths: | ||
| if plc.io.SourceInfo._is_remote_uri(p): | ||
| continue | ||
| continue # pragma: no cover | ||
| data = _read_file_bytes(Path(p)) | ||
| compression = _check_compression(data) | ||
| if compression is not None: | ||
|
|
@@ -326,9 +361,12 @@ def _(node: plrs._ir_nodes.Scan, translator: Translator, schema: Schema) -> ir.I | |
| n_rows, | ||
| row_index, | ||
| include_file_paths, | ||
| translate_named_expr(translator, n=node.predicate, schema=schema) | ||
| if node.predicate is not None | ||
| else None, | ||
| ( | ||
| None | ||
| if node.predicate is None | ||
| or _is_dynamic_pred(translator.visitor, node.predicate) | ||
| else translate_named_expr(translator, n=node.predicate, schema=schema) | ||
| ), | ||
| parquet_options, | ||
| ) | ||
|
|
||
|
|
@@ -537,6 +575,8 @@ def _(node: plrs._ir_nodes.Slice, translator: Translator, schema: Schema) -> ir. | |
| def _(node: plrs._ir_nodes.Filter, translator: Translator, schema: Schema) -> ir.IR: | ||
| with set_node(translator.visitor, node.input): | ||
| inp = translator.translate_ir(n=None) | ||
| if _is_dynamic_pred(translator.visitor, node.predicate): | ||
| return inp | ||
| mask = translate_named_expr(translator, n=node.predicate, schema=inp.schema) | ||
| return ir.Filter(schema, mask, inp) | ||
|
|
||
|
|
@@ -585,9 +625,14 @@ def _(node: plrs._ir_nodes.Union, translator: Translator, schema: Schema) -> ir. | |
|
|
||
| @_translate_ir.register | ||
| def _(node: plrs._ir_nodes.HConcat, translator: Translator, schema: Schema) -> ir.IR: | ||
| if POLARS_VERSION_LT_139: | ||
| strict = False # pragma: no cover | ||
| else: | ||
| _, strict, _ = node.options | ||
| return ir.HConcat( | ||
| schema, | ||
| False, # noqa: FBT003 | ||
| strict, | ||
| *(translator.translate_ir(n=n) for n in node.inputs), | ||
| ) | ||
|
|
||
|
|
@@ -625,7 +670,7 @@ def _(node: plrs._ir_nodes.Sink, translator: Translator, schema: Schema) -> ir.I | |
| if k in {"mkdir", "maintain_order", "sync_on_close"}: | ||
| options[k] = v | ||
|
|
||
| if sink_kind in ("Csv", "NDJson"): | ||
| if sink_kind in ("Csv", "NDJson", "Json"): | ||
| compression = format_options.get("compression") | ||
| if compression and compression != "Uncompressed": | ||
| raise NotImplementedError( | ||
|
|
@@ -748,6 +793,10 @@ def _( | |
| ) | ||
| (closed,) = options | ||
| lop, rop = expr.BooleanFunction._BETWEEN_OPS[closed] | ||
| if not POLARS_VERSION_LT_139: | ||
|
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. Can you write a test in cudf_polars for this?
Contributor
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. Sure added in 463d172 |
||
| # IsBetween type coercion does not | ||
| # insert casts for Decimal/Float comparisons. | ||
| column, lo, hi = _align_decimal_float_for_comparison(column, lo, hi) | ||
| return expr.BinOp( | ||
| dtype, | ||
| plc.binaryop.BinaryOperator.LOGICAL_AND, | ||
|
|
@@ -820,14 +869,9 @@ def _( | |
| dtype: DataType, | ||
| schema: Schema, | ||
| ) -> expr.Expr: | ||
| if isinstance( | ||
| if hasattr(plrs._expr_nodes, "RollingGroupOptions") and isinstance( | ||
| node.options, plrs._expr_nodes.RollingGroupOptions | ||
| ): # pragma: no cover; polars gives Aexpr::Rolling node now | ||
| # TODO: As of polars 1.36.0, rolling is represented as a Rolling expression node | ||
| # but is currently not implemented in the python node visitor. Once we support it, | ||
| # we should move this branch to separate translator dispatch function for the new node. | ||
|
|
||
| # pl.col("a").rolling(...) | ||
| ): # pragma: no cover; polars >=1.36 uses AExpr::Rolling now | ||
| with set_expr_context(translator, ExecutionContext.ROLLING): | ||
| agg = translator.translate_expr(n=node.function, schema=schema) | ||
| name_generator = unique_names(schema) | ||
|
|
@@ -928,6 +972,61 @@ def _( | |
| assert_never(node.options) | ||
|
|
||
|
|
||
| def _translate_rolling( | ||
| node: plrs._expr_nodes.Rolling, | ||
| translator: Translator, | ||
| dtype: DataType, | ||
| schema: Schema, | ||
| ) -> expr.Expr: | ||
| # pl.col("a").rolling(...) | ||
| with set_expr_context(translator, ExecutionContext.ROLLING): | ||
|
mroeschke marked this conversation as resolved.
Resolved
|
||
| agg_expr = translator.translate_expr(n=node.function, schema=schema) | ||
| orderby = translator.visitor.view_expression(node.index_column).name | ||
| orderby_dtype = schema[orderby].plc_type | ||
| if plc.traits.is_integral(orderby_dtype): | ||
| orderby_dtype = plc.DataType(plc.TypeId.INT64) | ||
| name_generator = unique_names(schema) | ||
| aggs, named_post_agg = decompose_single_agg( | ||
| expr.NamedExpr(next(name_generator), agg_expr), | ||
| name_generator, | ||
| is_top=True, | ||
| context=ExecutionContext.ROLLING, | ||
| ) | ||
| named_aggs = [a for a, _ in aggs] | ||
| closed_window = node.closed_window | ||
| if isinstance(named_post_agg.value, expr.Col): | ||
| (named_agg,) = named_aggs | ||
| return expr.RollingWindow( | ||
| named_agg.value.dtype, | ||
| orderby_dtype, | ||
| node.offset, | ||
| node.period, | ||
| closed_window, | ||
| orderby, | ||
| named_agg.value, | ||
| ) | ||
| replacements: dict[expr.Expr, expr.Expr] = { | ||
| expr.Col(a.value.dtype, a.name): expr.RollingWindow( | ||
| a.value.dtype, | ||
| orderby_dtype, | ||
| node.offset, | ||
| node.period, | ||
| closed_window, | ||
| orderby, | ||
| a.value, | ||
| ) | ||
| for a in named_aggs | ||
| } | ||
| return replace([named_post_agg.value], replacements)[0] | ||
|
|
||
|
|
||
| # Rolling was added in polars 1.39. Use explicit registration (not annotation-based) | ||
| # so that Python 3.14's stricter get_type_hints() resolution does not fail when | ||
| # running on older polars versions that lack this node type. | ||
| if hasattr(plrs._expr_nodes, "Rolling"): | ||
| _translate_expr.register(plrs._expr_nodes.Rolling)(_translate_rolling) | ||
|
|
||
|
|
||
| @_translate_expr.register | ||
| def _( | ||
| node: plrs._expr_nodes.Literal, | ||
|
|
||
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.
Yeah we should cap this at 1.40 (even though 1.40.1 is out). I'll do the next bump in the next RAPIDS release.