Support shift over grouped windows in cudf-polars - #23451
Conversation
|
cc @Matt711 (since you originally implemented |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds grouped window ChangesGrouped window shift support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Matt711
left a comment
There was a problem hiding this comment.
Looks good, only comment is can you make sure either we fallback or support a nested expression like pl.col("x").shift(1).fill_null(strategy="forward").over("g")?
|
/merge |
|
Update: I just pushed #23466 to enable |
Follow up to #23451 Fully addresses item (7) of #22032 Adds the necessary per-group sort operation to preserve input row order before evaluating input-order-sensitive `over(...)` expressions without an explicit `order_by` argument. This allows streaming execution of grouped `shift` and `cum_sum` windows that rely on the incoming row order. Authors: - Richard (Rick) Zamora (https://github.com/rjzamora) Approvers: - Matthew Roeschke (https://github.com/mroeschke) URL: #23466
Adds cudf-polars support for grouped-window
shift, includingshift(...).over(...)andshift(..., fill_value=...).over(...).Partially addresses example/feature (7) in #22032. Like existing
cum_sumsupport, implicit-ordershift().over(...)withoutorder_bystill falls back for multi-rank execution. I plan to remove that limitation for bothcum_sumandshiftin a follow-up PR.