Fix groupby and single-column to_numpy pandas-compatibility bugs - #22783
Conversation
|
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. |
to_numpy pandas-compatibility bugs
|
/okay to test 1b463e4 |
|
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:
📝 WalkthroughWalkthroughPromotes single-column nullable integer arrays to float64 when dtype is unset and nulls exist; updates many GroupBy behaviors (iteration order, indices construction, cumcount, agg finalization, idxmin/idxmax label mapping, ngroup determinism, pct_change, key normalization) and adjusts tests and the pandas-testing-plugin registry. ChangesGroupBy pandas-compatibility fixes and frame array conversion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
|
This change broke one of cudf's own tests. |
Fixed the failures. |
There was a problem hiding this comment.
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/cudf/core/groupby/groupby.py`:
- Around line 3898-3915: The new unconditional key normalization in the
_Grouping block alters GroupBy behavior; only perform the
object-key-to-float-NaN and nans_to_nulls normalization when pandas-compatible
mode is enabled (the same condition used in GroupBy.__init__ that gates
obj.nans_to_nulls()). Wrap the loop that mutates self._key_columns (the
normalized list creation and replacement) behind the pandas compatibility check
(e.g., the existing mode.pandas_compatible/GroupBy __init__ condition) so that
outside pandas_compatible mode keys are left untouched.
🪄 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: c1355959-6fc0-4d27-b820-996dfb1fa457
📒 Files selected for processing (3)
python/cudf/cudf/core/groupby/groupby.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.pypython/cudf/cudf/tests/groupby/test_reductions.py
|
/okay to test a63bdc4 |
|
/okay to test d21dffc |
|
/okay to test 39c22d0 |
|
/merge |
Description
Fixes several cudf behaviors to match pandas, enabling the full
tests/groupby/test_groupby_dropna.pypandas suite (and a few related tests) undercudf.pandas:DataFrame.to_numpy()/.to_cupy()/.valueson a single nullable-integer column now returnNaN(float64) instead of leaking the integer null sentinel, matchingSeriesand the multi-column path.groupby.idxmin/idxmaxreturn index labels (via gather) instead of positions.groupby.ngrouprespectssortanddropna.groupby.applyuses the UDF-returned index for the innerMultiIndexlevel instead of the original row positions.GroupBy.indicesreturns numpy arrays in pandas-compatible mode.NaN/null collapse into a single group, and an all-null object key yields a float64NaNlabel.Removes the corresponding
xfail/skip entries from the pandas testing plugin.Checklist