Add agent skill for cudf_polars expressions - #23078
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new skill document for reviewing and implementing Polars Expression support in ChangesSkill Documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.agents/skills/review-cudf-polars-expressions/SKILL.md:
- Around line 75-89: Remove the hidden HTML comment blocks in SKILL.md and keep
any TODO guidance in visible markdown instead. Update the Step 4 section by
deleting the commented placeholders around the cudf-polars testing guidance so
downstream consumers only see the intended markdown content.
- Around line 44-46: The second GPUEngine example has an extra closing
parenthesis in the `collect` engine argument, causing a syntax error. Update the
`pl.GPUEngine(executor="streaming", raise_on_fail=True,
executor_options={"max_rows_per_partition": 2})` usage in the Step 1 examples so
the parentheses are balanced, and keep both streaming `collect` variations
aligned with the `GPUEngine` symbol in the SKILL.md instructions.
🪄 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: 67327162-0e88-4b22-8a57-e9779f7c8786
📒 Files selected for processing (1)
.agents/skills/review-cudf-polars-expressions/SKILL.md
| * The Python APIs for expressions are mostly defined in the `py-polars/src/polars/expr` directory in a Polars repository. | ||
| * Expressions are eventually exposed to cudf-polars in Rust in the `crates/polars-python/src/lazyframe/visitor/expr_nodes.rs` file in a Polars repository | ||
|
|
||
| Next, generate examples locally with the expression(s) to implement or review to further understand the behavior. |
There was a problem hiding this comment.
Idle thought: is generating examples better? Or should we tell it to find the relevant tests in the polars repo?
There was a problem hiding this comment.
I suppose it doesn't hurt to instruct an agent to look at Polars unit tests for understanding, but I'll keep the example generation since in a later step I added to use generated example to generate unit tests
|
|
||
| TODO: Add more guidance on a multiple partition implementation. | ||
|
|
||
| ## Step 4: Test the Polars expression implementation in cudf-polars |
There was a problem hiding this comment.
Should we move (part of) this to before the implementation, to encourage writing a failing test first?
There was a problem hiding this comment.
Instead in the testing section, I instructed the agent to git stash the implementation and run the new test to check that they fail as expected first.
Co-authored-by: Tom Augspurger <tom.augspurger88@gmail.com>
Co-authored-by: Tom Augspurger <tom.augspurger88@gmail.com>
…om/mroeschke/cudf into cudf_polars/skill/expressions_impl
|
/merge |
Description
I used a more barebones prompt that resembles this PR to implement #23015 and #23037, but I think it could be generally be useful to have when implementing reviewing expression support in cudf_polars.
Some of the guidelines is probably more suitable for generally cudf_polars development, but this skill is mainly a starting point that can be iterated on/split out
Checklist