Fix DataFrame.rename for an unnamed MultiIndex level - #23062
Merged
Conversation
github-actions
Bot
added
Python
3 tasks
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates MultiIndex level renaming so targeted values are overwritten in place, preserves index level names after rebuilding, adds coverage for unnamed MultiIndex cases, and removes a related test-plugin exclusion entry. ChangesMultiIndex Level Rename Fix
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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
🤖 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/dataframe.py`:
- Around line 4242-4256: The MultiIndex `level` rename path in
`DataFrame.rename` is missing the same `OverflowError` suppression used in the
non-level branch, so `errors='ignore'` can still raise from `find_and_replace`.
Update the `level is not None and isinstance(self.index, MultiIndex)` branch to
catch `OverflowError` around the `level_values._column.find_and_replace(...)`
call and return the original object when `errors='ignore'`, matching the
existing rename behavior.
🪄 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: 20b4d10b-2a78-4861-8682-cd576f5729ff
📒 Files selected for processing (3)
python/cudf/cudf/core/dataframe.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.pypython/cudf/cudf/tests/dataframe/methods/test_rename.py
💤 Files with no reviewable changes (1)
- python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Comment on lines
4242
to
4256
| if level is not None and isinstance(self.index, MultiIndex): | ||
| level = self.index._get_level_label(level) | ||
| # Resolve to the ColumnAccessor label (e.g. the positional key | ||
| # ``0`` for an unnamed level), NOT the level *name* (which is | ||
| # ``None`` for unnamed levels and would insert a spurious extra | ||
| # column rather than overwrite the level being renamed). | ||
| ca_label, _ = self.index._level_to_ca_label(level) | ||
| level_values = self.index.get_level_values(level) | ||
| ca = self.index._data.copy(deep=copy) | ||
| ca[level] = level_values._column.find_and_replace( | ||
| vals = list(index.values()) | ||
| is_all_na = all(val is None for val in vals) | ||
| ca[ca_label] = level_values._column.find_and_replace( | ||
| to_replace=list(index.keys()), | ||
| replacement=list(index.values()), | ||
| replacement=vals, | ||
| all_nan=is_all_na, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="python/cudf/cudf/core/dataframe.py"
# Show the relevant region with line numbers
sed -n '4225,4295p' "$file" | cat -n
# Also inspect the sibling branch around the documented try/except
sed -n '4258,4288p' "$file" | cat -nRepository: rapidsai/cudf
Length of output: 4914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="python/cudf/cudf/core/dataframe.py"
# Inspect method signature/docstring and surrounding control flow
sed -n '4170,4255p' "$file" | cat -nRepository: rapidsai/cudf
Length of output: 3607
Add the OverflowError guard to the MultiIndex level rename path.
The errors='ignore' path already suppresses OverflowError in the non-level branch, but this MultiIndex-level branch doesn’t. If find_and_replace overflows here, rename(..., errors='ignore') will raise instead of returning the original object.
Fix
- ca[ca_label] = level_values._column.find_and_replace(
- to_replace=list(index.keys()),
- replacement=vals,
- all_nan=is_all_na,
- )
- out_index = type(self.index)._from_data(
- ca, name=self.index.name
- )
- out_index.names = self.index.names
+ try:
+ ca[ca_label] = level_values._column.find_and_replace(
+ to_replace=list(index.keys()),
+ replacement=vals,
+ all_nan=is_all_na,
+ )
+ out_index = type(self.index)._from_data(
+ ca, name=self.index.name
+ )
+ out_index.names = self.index.names
+ except OverflowError:
+ pass🤖 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/core/dataframe.py` around lines 4242 - 4256, The MultiIndex
`level` rename path in `DataFrame.rename` is missing the same `OverflowError`
suppression used in the non-level branch, so `errors='ignore'` can still raise
from `find_and_replace`. Update the `level is not None and
isinstance(self.index, MultiIndex)` branch to catch `OverflowError` around the
`level_values._column.find_and_replace(...)` call and return the original object
when `errors='ignore'`, matching the existing rename behavior.
vyasr
approved these changes
Jul 1, 2026
Contributor
Author
|
/okay to test 8fe27b7 |
galipremsagar
added
bug
Contributor
Author
|
/okay to test b722630 |
Contributor
Author
|
/okay to test 129ca6a |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/cudf/cudf/core/dataframe.py (1)
4234-4241: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale pandas-compat note: "Not Supporting: level" contradicts the implemented level-based MultiIndex rename.
This unchanged docstring note predates (and is now more clearly contradicted by) the level-handling logic below and the top-level
levelparameter documentation (lines 4192-4193). Consider removing/updating it in a follow-up so users aren't misled into thinkinglevelis unsupported.🤖 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/core/dataframe.py` around lines 4234 - 4241, The pandas-compat note in DataFrame.rename is stale because it still says “Not Supporting: level” even though the rename implementation and the level parameter docs already support level-based MultiIndex renaming. Update or remove that note in DataFrame.rename so it matches the behavior exposed by the level handling logic and the documented level parameter.
🤖 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.
Nitpick comments:
In `@python/cudf/cudf/core/dataframe.py`:
- Around line 4234-4241: The pandas-compat note in DataFrame.rename is stale
because it still says “Not Supporting: level” even though the rename
implementation and the level parameter docs already support level-based
MultiIndex renaming. Update or remove that note in DataFrame.rename so it
matches the behavior exposed by the level handling logic and the documented
level parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 77931d7a-1c97-4b5a-9892-5c64aed5f6f6
📒 Files selected for processing (2)
python/cudf/cudf/core/dataframe.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
✅ Files skipped from review due to trivial changes (1)
- python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Contributor
Author
|
/okay to test 5b8620c |
Contributor
Author
|
/merge |
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
3 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 #22966 per review feedback.
DataFrame.rename(index=..., level=...)on a MultiIndex with an unnamed level previously inserted a spurious extra level instead of overwriting the target level: the level name (Nonefor an unnamed level) was used as theColumnAccessorkey. It now resolves to the positional ColumnAccessor label viaMultiIndex._level_to_ca_label, restores the original level names afterward, and forwardsall_nantofind_and_replaceso all-null replacements behave like the non-level path.Adds a cuDF unit test and removes the now-passing
cudf.pandasxfail entry.Checklist