Keep MultiIndex columns in GroupBy.agg for MultiIndex-column sources - #23366
Merged
rapids-bot[bot] merged 3 commits intoJul 22, 2026
Merged
Conversation
GroupBy.agg flattened a MultiIndex-column source's aggregation result to flat tuple labels instead of keeping hierarchical columns like pandas. Preserve the MultiIndex (and its per-level metadata) when the aggregation keeps the source's tuple labels; relabeling aggregations (agg(new=(col, func))) emit new flat labels, so the source's multi-level metadata is not attached to those. Fixes 3 pandas-tests (test_groupby_with_hier_columns, test_wrap_aggregated_output_multindex, test_multiindex_custom_func); their xfail entries are removed. Attribution verified against an isolated build containing only this change.
github-actions
Bot
added
Python
3 tasks
|
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 (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughGroupBy aggregation now conditionally preserves MultiIndex column metadata based on output labels, including empty results. Regression tests cover hierarchical and flat named-aggregation outputs, and related expected-failure entries are removed. ChangesGroupBy MultiIndex aggregation
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)
✨ 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: 1
🧹 Nitpick comments (2)
python/cudf/cudf/tests/groupby/test_agg.py (2)
824-835: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse named source levels in the flat-relabeling regression.
Because this source MultiIndex has no names, the test cannot detect
level_namesleaking into the flat result. Setnames=["l0", "l1"]and assert that the relabeled output has a flatIndexwithname is None. Pandas uses the named-aggregation output keys as the result columns. (raw.githubusercontent.com)As per coding guidelines, test changes should cover the metadata edge cases exercised by the implementation.
🤖 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/cudf/tests/groupby/test_agg.py` around lines 824 - 835, Update test_agg_relabel_flat_columns_from_multiindex to create the source MultiIndex with names ["l0", "l1"], then explicitly assert that the named-aggregation result columns are a flat Index whose name is None for both pandas and cuDF outputs. Preserve the existing aggregation comparison while ensuring the regression detects leaked source level names.Source: Coding guidelines
809-821: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the empty MultiIndex result path.
GroupBy.aggnow preserveslevel_dtypeswhen no value columns are aggregated, but this test only exercises a non-empty result. Add an empty-frame/no-value-columns case with named, non-object MultiIndex levels so the new metadata path is validated.As per coding guidelines, Python tests must cover relevant empty and edge cases.
🤖 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/cudf/tests/groupby/test_agg.py` around lines 809 - 821, Extend test_agg_multiindex_columns_preserved with an empty-frame case containing named, non-object MultiIndex levels and no value columns, then compare cudf and pandas GroupBy.agg results to validate preservation of level_dtypes in the empty result path.Source: Coding guidelines
🤖 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 1328-1351: The flat relabeled-aggregation branch in ColumnAccessor
construction must not inherit MultiIndex column metadata: in
python/cudf/cudf/core/groupby/groupby.py lines 1328-1351, remove the source
level_names from the multiindex=False branch while preserving flat-label
behavior. Add a named-MultiIndex regression test in
python/cudf/cudf/tests/groupby/test_agg.py lines 824-835 verifying the relabeled
result has no stale columns name.
---
Nitpick comments:
In `@python/cudf/cudf/tests/groupby/test_agg.py`:
- Around line 824-835: Update test_agg_relabel_flat_columns_from_multiindex to
create the source MultiIndex with names ["l0", "l1"], then explicitly assert
that the named-aggregation result columns are a flat Index whose name is None
for both pandas and cuDF outputs. Preserve the existing aggregation comparison
while ensuring the regression detects leaked source level names.
- Around line 809-821: Extend test_agg_multiindex_columns_preserved with an
empty-frame case containing named, non-object MultiIndex levels and no value
columns, then compare cudf and pandas GroupBy.agg results to validate
preservation of level_dtypes in the empty result path.
🪄 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: 4c603a1b-d90f-489e-8298-b28c6b4e7ff5
📒 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_agg.py
💤 Files with no reviewable changes (1)
- python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
galipremsagar
added
bug
Contributor
Author
|
/okay to test 413f7b9 |
3 tasks
vyasr
approved these changes
Jul 21, 2026
Comment on lines
+1337
to
+1338
| multiindex=True, | ||
| level_names=self.obj._data.level_names, |
Collaborator
There was a problem hiding this comment.
Minor nit: You could instead assign these arguments to a kwargs dict that defaults to empty so that you only need to write the ColumnAccessor call once along with the other arguments.
Contributor
Author
There was a problem hiding this comment.
Done in c37af8d — the two branches differed only in multiindex/level_dtypes, so those now live in a dict that defaults to empty and the ColumnAccessor construction is written once.
3 tasks
galipremsagar
added
5 - Ready to Merge
The MultiIndex-preserving and flat-relabel branches differed only in multiindex/level_dtypes; collect those in a dict that defaults to empty so the ColumnAccessor construction is written once.
Contributor
Author
|
/okay to test dd39376 |
Contributor
Author
|
/merge |
rapids-bot
Bot
merged commit Jul 22, 2026
09b08da
into
NVIDIA:release/26.08
232 of 239 checks passed
3 tasks
rapids-bot Bot
pushed a commit
that referenced
this pull request
Jul 23, 2026
Split out of #23255 (4/6). **Depends on #23365 (fidelity), the stack PR (#23370), and #23366 (groupby agg)** — 8 of the 29 un-xfailed pandas-tests need those fixes too, so this PR's pandas-tests job goes green once they merge. - Order result rows/columns by the removed level's codes (level order preserved, missing keys first) instead of sorted values with nulls last, by encoding the integer code columns instead of the level values. - Propagate the source frame's column-axis level names into the result instead of hardcoding `None`; also fixes the `ValueError: Length of names must match number of levels` crash when unstacking MultiIndex-column frames. - Promote integer source columns to float64 when the reshape introduces missing cells (pandas' block semantics), gated on `mode.pandas_compatible`; `pivot_table`/`crosstab` opt out via a module-private `_unstack` parameter when `fill_value` fills the cells afterwards. - Preserve unused categories of the removed level in the result's column levels (pandas GH 17845); also fixes a libcudf `Column sizes don't match` crash for indexes with unused categorical categories. - Validate the level on flat-index frames (`KeyError`) and duplicated index names (`ValueError`) like pandas; `pivot` with `values=` drops the original columns-axis names. Fixes 29 pandas-tests; their xfail entries are removed, three remaining `test_stack_unstack.py` entries get real failure reasons, and two classic categorical unstack params are un-xfailed. Attribution verified per node id against isolated builds: 21 pass with only this change, 4 need the stack PR, 2 need stack+fidelity, 2 need the groupby-agg PR. Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Matthew Roeschke (https://github.com/mroeschke) URL: #23368
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
4 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
Split out of #23255 (5/6).
GroupBy.aggflattened a MultiIndex-column source's aggregation result to flat tuple labels instead of keeping hierarchical columns like pandas. Preserve the MultiIndex (and its per-level metadata) when the aggregation keeps the source's tuple labels; relabeling aggregations (agg(new=(col, func))) emit new flat labels, so the source's multi-level metadata is not attached to those.Fixes 3 pandas-tests (
test_groupby_with_hier_columns,test_wrap_aggregated_output_multindex,test_multiindex_custom_func[<lambda>0]); their xfail entries are removed. Attribution verified by running the node ids against an isolated build containing only this change (pass) and a clean build (fail).Independent of the other #23255 split PRs; the unstack PR (4/6) depends on this one for two entangled tests.
Checklist