Skip to content

Consolidate evaluate_rapidsmpf into evaluate_streaming in cudf_polars - #22417

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
mroeschke:ref/cudf_polars/rmpf_follow_up
May 11, 2026
Merged

Consolidate evaluate_rapidsmpf into evaluate_streaming in cudf_polars#22417
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
mroeschke:ref/cudf_polars/rmpf_follow_up

Conversation

@mroeschke

@mroeschke mroeschke commented May 7, 2026

Copy link
Copy Markdown
Contributor

Description

In a followup we should merge evaluate_rapidsmpf and evaluate_streaming...

  • Removes a temporary iterable in select that I had from a leftover branch
  • Removes the devcontainers key in dependencies.yml that was leftover from troubleshooting the devcontainer CI job

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@mroeschke mroeschke self-assigned this May 7, 2026
@mroeschke
mroeschke requested a review from a team as a code owner May 7, 2026 22:39
@mroeschke
mroeschke requested a review from vyasr May 7, 2026 22:39
@mroeschke mroeschke added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 7, 2026
@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels May 7, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python May 7, 2026
@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR inlines RapidsMPF logical-plan evaluation into evaluate_streaming, removes the evaluate_rapidsmpf wrapper, simplifies temporary name generation in select by consuming the names iterator inline, and deletes the files.devcontainers section from dependencies.yaml.

Changes

Code Simplification

Layer / File(s) Summary
Name Generation Simplification
python/cudf_polars/cudf_polars/experimental/expressions.py
select builds named_exprs using NamedExpr(next(names), expr) directly in a list comprehension instead of precomputing output_names and zipping with strict=True.
Evaluation Function Inlining
python/cudf_polars/cudf_polars/experimental/parallel.py
Removed evaluate_rapidsmpf; evaluate_streaming now imports and calls evaluate_logical_plan(...) and returns its result directly.
Dependencies Group Removal
dependencies.yaml
Deleted the files.devcontainers section that specified output: none and a curated includes list for devcontainers.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: consolidating evaluate_rapidsmpf into evaluate_streaming in cudf_polars, which matches the primary objective of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly relates to the changeset, describing the consolidation of evaluate_rapidsmpf into evaluate_streaming, cleanup of a temporary iterable, and removal of leftover devcontainers configuration.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/experimental/parallel.py`:
- Around line 125-128: The import of evaluate_logical_plan must be moved into
the evaluate_streaming function and wrapped in a try/except that raises a clear
user-facing ImportError if rapidsmpf is missing; inside evaluate_streaming
(before calling evaluate_logical_plan(ir, config_options,
collect_metadata=False)) call _clear_source_info_cache(), then try to import
from cudf_polars.experimental.rapidsmpf.core; on ImportError raise a message
like "The streaming executor requires the 'rapidsmpf' package. Install it or
choose a different executor." and otherwise call evaluate_logical_plan and
return its result.
🪄 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: 3caedbaf-0d3a-4363-9233-88f2968a792d

📥 Commits

Reviewing files that changed from the base of the PR and between 64a3109 and a5fadc1.

📒 Files selected for processing (2)
  • python/cudf_polars/cudf_polars/experimental/expressions.py
  • python/cudf_polars/cudf_polars/experimental/parallel.py

Comment thread python/cudf_polars/cudf_polars/experimental/parallel.py
@mroeschke
mroeschke requested a review from a team as a code owner May 7, 2026 23:08
Comment thread dependencies.yaml
@mroeschke

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 5b88237 into NVIDIA:main May 11, 2026
371 of 376 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python May 11, 2026
@mroeschke
mroeschke deleted the ref/cudf_polars/rmpf_follow_up branch May 11, 2026 16:14
shrshi pushed a commit to shrshi/cudf that referenced this pull request May 12, 2026
…lars (NVIDIA#22417)

* Address the follow up review NVIDIA#22281 (comment)

> In a followup we should merge evaluate_rapidsmpf and evaluate_streaming...

* Removes a temporary iterable in `select` that I had from a leftover branch 
* Removes the `devcontainers` key in `dependencies.yml` that was leftover from troubleshooting the devcontainer CI job

Authors:
  - Matthew Roeschke (https://github.com/mroeschke)

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - James Lamb (https://github.com/jameslamb)
  - Peter Andreas Entschev (https://github.com/pentschev)

URL: NVIDIA#22417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cudf-polars Issues specific to cudf-polars improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants