Bump polars upper bound to <1.40 - #22048
Merged
rapids-bot[bot] merged 58 commits intoMay 18, 2026
Merged
Conversation
Matt711
added
improvement
Matt711
marked this pull request as draft
April 7, 2026 22:32
github-actions
Bot
added
Python
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Member
Author
|
Please Do Not Review. The PR isn't ready yet |
Member
Author
|
/ok to test 714fefe |
Matt711
changed the title
Member
Author
|
/ok to test fa3c810 |
Contributor
|
Wow this is a pretty significant bump of our minimum version from 1.30 to 1.39 (appears just released last month). Is the idea just to guarantee that some Polars features/fixes are available? |
Member
Author
Yeah it's a big bump but I think it makes sense to do:
But if folks are strong -1 to this big of a jump, I can relax the minimum to 1.37 or so. |
Member
Author
|
/ok to test fbb4e57 |
Contributor
|
Agreed that our supported window has grown unwieldy. Looking at Polars versions in default cloud environment data (~March 2026), 1.39 may be a little aggressive AzureML: 1.38.1 |
…/bump-polars-1.40
Contributor
|
/ok to test d2a018d |
…/bump-polars-1.40
…son_remote is xfailed now)
Contributor
|
/ok to test 445b5ac |
mroeschke
marked this pull request as ready for review
May 15, 2026 23:49
📝 WalkthroughWalkthroughThis PR broadens Polars upper-bound pins to <1.40, adds POLARS_VERSION_LT_139, introduces HConcat strict checking, gates translation/rolling logic for newer Polars, updates experimental HConcat wiring, simplifies test harness expected-fail entries, and refactors/extends tests to cover version-dependent behavior. ChangesPolars 1.39 version support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
python/cudf_polars/cudf_polars/dsl/translate.py (1)
274-282: ⚡ Quick winAvoid keying
dynamic_preddetection off a private exception string.Matching
str(e) == "dynamic_pred"makes this path depend on a non-public Polars message. A patch-level Polars change would silently turn sorted-head plans back into translation failures. Please back this with a regression test or switch to a typed/symbolic check if Polars exposes one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cudf_polars/cudf_polars/dsl/translate.py` around lines 274 - 282, The code in _is_dynamic_pred currently relies on string-matching the exception message from visitor.view_expression(expr_ir.node); replace this fragile check with a typed/symbolic check: catch the specific Polars exception class (use the appropriate Polars exception type exposed by your Polars version instead of Exception) or, if Polars exposes a sentinel attribute on the exception (e.g., e.code or e.kind), test that property rather than str(e); update the except clause to "except <PolarsExceptionType> as e" and check the typed property, and add a regression test: call _is_dynamic_pred with a known dynamic predicate plan and assert it returns True so future Polars message changes will fail the test rather than silently break translation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cudf_polars/cudf_polars/testing/inject_gpu_engine.py`:
- Line 450: The streaming expected-failures mapping contains a typo: the key
"tests/unit/sql/test_window_functions.py::test_window_multiple_named_window"
(singular) should match the non-streaming key
"tests/unit/sql/test_window_functions.py::test_window_multiple_named_windows"
(plural); update the key in the mapping to use the plural
"test_window_multiple_named_windows" so xfail matching succeeds (look for the
dict of expected failures / streaming expected-failures entries where that
string is defined).
In `@python/cudf_polars/tests/expressions/test_rolling.py`:
- Around line 108-111: The test named test_rolling_agg_first currently calls
pl.col("a").sum().rolling("b", period="2i"), so it doesn't exercise the "first"
aggregation; update the expression in test_rolling_agg_first to use the first
aggregation (e.g., pl.col("a").first().rolling("b", period="2i")) so the test
matches its name and intent, leaving the rest of the setup (df, q, and
assert_gpu_result_equal) unchanged.
- Around line 159-160: The xfail currently gates all polars >=1.36 but should
only target the unsupported 1.36–1.38 window; update the pytest mark on
test_rolling_inside_groupby_raises so the condition becomes True only for
versions >=1.36 and <1.39 (e.g. condition=POLARS_VERSION_GE_136 and
POLARS_VERSION_LT_139) instead of the existing condition=not
POLARS_VERSION_LT_136.
---
Nitpick comments:
In `@python/cudf_polars/cudf_polars/dsl/translate.py`:
- Around line 274-282: The code in _is_dynamic_pred currently relies on
string-matching the exception message from
visitor.view_expression(expr_ir.node); replace this fragile check with a
typed/symbolic check: catch the specific Polars exception class (use the
appropriate Polars exception type exposed by your Polars version instead of
Exception) or, if Polars exposes a sentinel attribute on the exception (e.g.,
e.code or e.kind), test that property rather than str(e); update the except
clause to "except <PolarsExceptionType> as e" and check the typed property, and
add a regression test: call _is_dynamic_pred with a known dynamic predicate plan
and assert it returns True so future Polars message changes will fail the test
rather than silently break translation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a790b5c1-5fc0-4d22-9be5-916025438c67
📒 Files selected for processing (24)
conda/environments/all_cuda-129_arch-aarch64.yamlconda/environments/all_cuda-129_arch-x86_64.yamlconda/environments/all_cuda-132_arch-aarch64.yamlconda/environments/all_cuda-132_arch-x86_64.yamlconda/recipes/cudf-polars/recipe.yamldependencies.yamlpython/cudf_polars/cudf_polars/dsl/expressions/rolling.pypython/cudf_polars/cudf_polars/dsl/expressions/unary.pypython/cudf_polars/cudf_polars/dsl/ir.pypython/cudf_polars/cudf_polars/dsl/translate.pypython/cudf_polars/cudf_polars/experimental/expressions.pypython/cudf_polars/cudf_polars/experimental/io.pypython/cudf_polars/cudf_polars/experimental/select.pypython/cudf_polars/cudf_polars/testing/inject_gpu_engine.pypython/cudf_polars/cudf_polars/utils/versions.pypython/cudf_polars/pyproject.tomlpython/cudf_polars/tests/experimental/test_rolling.pypython/cudf_polars/tests/expressions/test_booleanfunction.pypython/cudf_polars/tests/expressions/test_rolling.pypython/cudf_polars/tests/expressions/test_sort.pypython/cudf_polars/tests/test_hconcat.pypython/cudf_polars/tests/test_parquet_filters.pypython/cudf_polars/tests/test_scan.pypython/cudf_polars/tests/test_window_functions.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/cudf_polars/tests/expressions/test_rolling.py (1)
159-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow the xfail condition to the unsupported 1.36–1.38 range.
Line 159 currently xfails all Polars
>=1.36, which can mask regressions in>=1.39where rolling-expression handling is re-enabled elsewhere in this file.Proposed fix
-@pytest.mark.xfail(condition=not POLARS_VERSION_LT_136, reason="not supported") +@skip_rolling_expr_136_to_138 def test_rolling_inside_groupby_raises():🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cudf_polars/tests/expressions/test_rolling.py` around lines 159 - 160, The xfail on test_rolling_inside_groupby_raises is too broad; narrow it to only Polars 1.36–1.38. Change the condition from "not POLARS_VERSION_LT_136" to "not POLARS_VERSION_LT_136 and POLARS_VERSION_LT_139" (or introduce a helper like POLARS_VERSION_GTE_136_AND_LT_139) so the decorator only xfails for >=1.36 and <1.39 while allowing >=1.39 to run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@python/cudf_polars/tests/expressions/test_rolling.py`:
- Around line 159-160: The xfail on test_rolling_inside_groupby_raises is too
broad; narrow it to only Polars 1.36–1.38. Change the condition from "not
POLARS_VERSION_LT_136" to "not POLARS_VERSION_LT_136 and POLARS_VERSION_LT_139"
(or introduce a helper like POLARS_VERSION_GTE_136_AND_LT_139) so the decorator
only xfails for >=1.36 and <1.39 while allowing >=1.39 to run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4a340595-7e62-407d-8760-83d51cb09ecf
📒 Files selected for processing (10)
python/cudf_polars/cudf_polars/dsl/expressions/rolling.pypython/cudf_polars/cudf_polars/dsl/ir.pypython/cudf_polars/cudf_polars/dsl/translate.pypython/cudf_polars/cudf_polars/experimental/expressions.pypython/cudf_polars/cudf_polars/experimental/select.pypython/cudf_polars/cudf_polars/testing/inject_gpu_engine.pypython/cudf_polars/pyproject.tomlpython/cudf_polars/tests/experimental/test_rolling.pypython/cudf_polars/tests/expressions/test_rolling.pypython/cudf_polars/tests/test_hconcat.py
🚧 Files skipped from review as they are similar to previous changes (9)
- python/cudf_polars/pyproject.toml
- python/cudf_polars/cudf_polars/experimental/select.py
- python/cudf_polars/tests/experimental/test_rolling.py
- python/cudf_polars/tests/test_hconcat.py
- python/cudf_polars/cudf_polars/dsl/ir.py
- python/cudf_polars/cudf_polars/experimental/expressions.py
- python/cudf_polars/cudf_polars/dsl/translate.py
- python/cudf_polars/cudf_polars/dsl/expressions/rolling.py
- python/cudf_polars/cudf_polars/testing/inject_gpu_engine.py
jameslamb
approved these changes
May 18, 2026
jameslamb
left a comment
jameslamb
left a comment
Member
There was a problem hiding this comment.
approving for packaging-codeowners
Member
Author
Nope! Thanks for driving it @mroeschke ! |
Contributor
|
/merge |
rapids-bot
Bot
merged commit May 18, 2026
3249ba8
into
NVIDIA:release/26.06
225 of 227 checks passed
madsbk
added a commit
to madsbk/cudf
that referenced
this pull request
May 18, 2026
PR NVIDIA#22048 ("Bump polars upper bound to <1.40", merged today) added the new `test_hconcat_strict_different_heights` test, which imports `assert_collect_raises`. However, PR NVIDIA#22535 ("Clean up legacy test scaffolding", also merged into `release/26.06`) removed that helper. The two PRs landed on `release/26.06` without the conflict being noticed. On `main`, `test_hconcat.py` does not contain the strict-mode test, so the issue is limited to `release/26.06`.
rapids-bot Bot
pushed a commit
that referenced
this pull request
May 19, 2026
#22558) PR #22048 (merged today) added the new `test_hconcat_strict_different_heights` test, which imports `assert_collect_raises`. However, PR #22535 (also merged today) removed that helper. The two PRs landed on `release/26.06` without the conflict being noticed. On `main`, `test_hconcat.py` does not contain the strict-mode test, so the issue is limited to `release/26.06`. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Matthew Murray (https://github.com/Matt711) URL: #22558
madsbk
pushed a commit
to madsbk/cudf
that referenced
this pull request
May 19, 2026
Updated cudf-polars to support Polars 1.39. Summary: * **Dependency pin** updated across conda envs, the recipe, `dependencies.yaml`, and `pyproject.toml`. New `POLARS_VERSION_LT_139` flag gates version specific code. * **Rolling expressions:** polars 1.39 makes `pl.col(...).rolling(...)` accessible again via `AExpr::Rolling`. A new `_translate_rolling` handles it, registered only when the node type exists. Rolling tests use a single `skip_rolling_expr_136_to_138` marker. * **HConcat strict mode:** added a `strict` slot on the `HConcat` IR that raises `pl.exceptions.ShapeError` on height mismatch, threaded through every construction site. * **IsBetween Decimal vs Float:** new `_align_decimal_float_for_comparison` casts Decimal to Float64 on 1.39+, since polars no longer inserts that cast and libcudf would otherwise give wrong results. * **set_sorted:** options shape changed from `(asc_str,)` to `(descending_bool, ...)`; translator branches on type. * **Dynamic predicates:** new `_is_dynamic_pred` helper makes Scan and Filter skip predicates that raise `"dynamic_pred"`. * **IR version ceiling** raised from `(12, 1)` to `(12, 2)`. Sink format check now includes `"Json"`, and a precedence bug in `_sink_to_file` is fixed. Authors: - Matthew Murray (https://github.com/Matt711) - Matthew Roeschke (https://github.com/mroeschke) Approvers: - James Lamb (https://github.com/jameslamb) - Matthew Roeschke (https://github.com/mroeschke) URL: NVIDIA#22048
madsbk
added a commit
to madsbk/cudf
that referenced
this pull request
May 19, 2026
NVIDIA#22558) PR NVIDIA#22048 (merged today) added the new `test_hconcat_strict_different_heights` test, which imports `assert_collect_raises`. However, PR NVIDIA#22535 (also merged today) removed that helper. The two PRs landed on `release/26.06` without the conflict being noticed. On `main`, `test_hconcat.py` does not contain the strict-mode test, so the issue is limited to `release/26.06`. Authors: - Mads R. B. Kristensen (https://github.com/madsbk) Approvers: - Matthew Murray (https://github.com/Matt711) URL: NVIDIA#22558
This was referenced May 19, 2026
3 tasks
rapids-bot Bot
pushed a commit
that referenced
this pull request
May 26, 2026
Follow ups to #22048 closes #20551 * Removes some Polars dependency pinnings which should no longer be necessary * Un-skips some unit tests that should be fixed now Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - James Lamb (https://github.com/jameslamb) - Tom Augspurger (https://github.com/TomAugspurger) URL: #22605
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Updated cudf-polars to support Polars 1.39.
Summary:
dependencies.yaml, andpyproject.toml. NewPOLARS_VERSION_LT_139flag gates version specific code.pl.col(...).rolling(...)accessible again viaAExpr::Rolling. A new_translate_rollinghandles it, registered only when the node type exists. Rolling tests use a singleskip_rolling_expr_136_to_138marker.strictslot on theHConcatIR that raisespl.exceptions.ShapeErroron height mismatch, threaded through every construction site._align_decimal_float_for_comparisoncasts Decimal to Float64 on 1.39+, since polars no longer inserts that cast and libcudf would otherwise give wrong results.(asc_str,)to(descending_bool, ...); translator branches on type._is_dynamic_predhelper makes Scan and Filter skip predicates that raise"dynamic_pred".(12, 1)to(12, 2). Sink format check now includes"Json", and a precedence bug in_sink_to_fileis fixed.Checklist