-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix reset_index: level bounds, duplicate columns, columns.name, tuple index names #23418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3b0e531
93ec11f
c6dc760
4d42790
646dcd4
2863d0f
d164f84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4836,37 +4836,55 @@ def _reset_index( | |
| elif names is not None: | ||
| raise NotImplementedError("names is not currently supported.") | ||
| if level is not None: | ||
| if ( | ||
| isinstance(level, int) | ||
| and level > 0 | ||
| and not isinstance(self.index, MultiIndex) | ||
| ): | ||
| raise IndexError( | ||
| f"Too many levels: Index has only 1 level, not {level + 1}" | ||
| ) | ||
| if not isinstance(level, (tuple, list)): | ||
| level = (level,) | ||
| _check_duplicate_level_names(level, self.index.names) | ||
| # Normalize to level numbers, which also validates the labels | ||
| # (out of bounds, unknown name, ambiguous duplicate name). | ||
| level = tuple( | ||
| self.index._level_index_from_level(lv) for lv in level | ||
| ) | ||
|
|
||
| index = self.index._new_index_for_reset_index(level, self.index.name) | ||
| if index is None: | ||
| index = RangeIndex(len(self)) | ||
| if drop: | ||
| return self._data, index | ||
|
|
||
| new_column_data = {} | ||
| new_column_items = [] | ||
| for name, col in self.index._columns_for_reset_index(level): | ||
| if name == "index" and "index" in self._data: | ||
| name = "level_0" | ||
| name = ( | ||
| tuple( | ||
| name if i == col_level else col_fill | ||
| for i in range(self._data.nlevels) | ||
| ) | ||
| if self._data.multiindex | ||
| else name | ||
| ) | ||
| new_column_data[name] = col | ||
| if self._data.multiindex: | ||
| nlevels = self._data.nlevels | ||
| if isinstance(name, tuple): | ||
| if len(name) > nlevels: | ||
| raise ValueError( | ||
| "Item must have length equal to number of levels." | ||
| ) | ||
| elif len(name) < nlevels: | ||
| if col_fill is None: | ||
| raise ValueError( | ||
| f"col_fill=None is incompatible with " | ||
| f"incomplete column name {name}" | ||
| ) | ||
| name = ( | ||
| (col_fill,) * col_level | ||
| + tuple(name) | ||
| + (col_fill,) * (nlevels - col_level - len(name)) | ||
| ) | ||
| # else len == nlevels: use as-is | ||
| else: | ||
| name = tuple( | ||
| name if i == col_level else col_fill | ||
| for i in range(nlevels) | ||
| ) | ||
| new_column_items.append((name, col)) | ||
|
Comment on lines
+4857
to
+4881
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Check whether col_level is validated against columns.nlevels
# anywhere before reaching IndexedFrame._reset_index (e.g. in the public
# DataFrame.reset_index/Series.reset_index wrappers).
rg -n -C5 'col_level' python/cudf/cudf/core/dataframe.py python/cudf/cudf/core/series.py python/cudf/cudf/core/indexed_frame.pyRepository: NVIDIA/cudf Length of output: 5556 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- _reset_index context ---'
sed -n '4815,4920p' python/cudf/cudf/core/indexed_frame.py
printf '%s\n' '--- reset_index tests mentioning col_level ---'
rg -n -C6 'reset_index|col_level' python/cudf/cudf/tests python/cudf/tests 2>/dev/null | head -n 300 || true
printf '%s\n' '--- all direct col_level validation references ---'
rg -n -C3 'col_level' python/cudf/cudf | head -n 300Repository: NVIDIA/cudf Length of output: 41079 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
import pandas as pd
columns = pd.MultiIndex.from_tuples([("a", "x"), ("b", "y")])
frame = pd.DataFrame({"value": [1, 2]}, index=pd.Index(["i", "j"]))
frame.columns = columns
for col_level in [-1, 0, 1, 2, 3]:
try:
result = frame.reset_index(col_level=col_level)
print(f"col_level={col_level}: columns={list(result.columns)!r}")
except Exception as exc:
print(f"col_level={col_level}: {type(exc).__name__}: {exc}")
PYRepository: NVIDIA/cudf Length of output: 268 🌐 Web query:
💡 Result: In pandas, the col_level parameter in DataFrame.reset_index is designed to specify the level into which index labels are inserted when a DataFrame has a MultiIndex for columns [1][2]. By default, it is set to 0, meaning labels are inserted into the first level of the column hierarchy [1][3]. Regarding the behavior with negative values or out-of-bounds inputs: 1. Out-of-Bounds/Negative Values: The implementation of col_level generally expects a valid level index (integer) or a level name (string) [1]. Providing a negative integer that exceeds the depth of the column index or an integer/string that does not correspond to an existing level will typically result in a ValueError or IndexError, depending on the underlying pandas version and internal validation checks for MultiIndex column structure. Pandas validates that the target level for the index labels exists within the current column structure. 2. Source Implementation Context: The col_level parameter logic is deeply tied to how pandas handles MultiIndex columns during index manipulation [4][5]. When you reset the index, pandas effectively promotes the index levels into the column space. The col_level parameter dictates the destination position within the existing column MultiIndex [4]. If the specified destination does not exist, the operation cannot complete because it cannot place the new labels into an invalid or non-existent structural level [5]. 3. Typical Error Patterns: Users encountering issues with col_level often find that the error is not inherently a bug in the parameter itself, but rather a structural mismatch between the intended insertion point and the actual dimensions of the column MultiIndex [5]. If you are seeing an out-of-bounds error, it is likely because the integer provided for col_level is higher than the number of levels currently in the column index (e.g., trying to place at level 5 in a DataFrame with only 2 levels). For robust code, ensure that your col_level matches an existing index level (0 to n-1, where n is the number of column levels) or provides a valid name, and consider using col_fill if you need to create or populate additional levels [1][6]. Citations:
🏁 Script executed: #!/bin/bash
set -eu
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/pandas-dev/pandas/main/pandas/core/frame.py \
-o "$tmpdir/frame.py"
printf '%s\n' '--- pandas reset_index implementation ---'
rg -n -C30 'def reset_index' "$tmpdir/frame.py" | head -n 220Repository: NVIDIA/cudf Length of output: 3917 🏁 Script executed: #!/bin/bash
set -eu
tmpfile="$(mktemp)"
trap 'rm -f "$tmpfile"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/pandas-dev/pandas/main/pandas/core/frame.py \
-o "$tmpfile"
sed -n '7480,7615p' "$tmpfile"Repository: NVIDIA/cudf Length of output: 5209 🏁 Script executed: #!/bin/bash
set -eu
tmpfile="$(mktemp)"
trap 'rm -f "$tmpfile"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/pandas-dev/pandas/main/pandas/core/frame.py \
-o "$tmpfile"
sed -n '7615,7735p' "$tmpfile"Repository: NVIDIA/cudf Length of output: 4850 🏁 Script executed: #!/bin/bash
set -eu
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
curl -fsSL \
https://raw.githubusercontent.com/pandas-dev/pandas/main/pandas/core/indexes/multi.py \
-o "$tmpdir/multi.py"
curl -fsSL \
https://raw.githubusercontent.com/pandas-dev/pandas/main/pandas/core/indexes/base.py \
-o "$tmpdir/base.py"
printf '%s\n' '--- pandas level resolution ---'
rg -n -C25 'def _get_level_number' "$tmpdir/multi.py" "$tmpdir/base.py" | head -n 180
printf '%s\n' '--- cuDF column-level helpers ---'
rg -n -C5 '_get_level_number|level_names|nlevels|level.*index' \
python/cudf/cudf/core/column_accessor.py \
python/cudf/cudf/core/indexed_frame.py | head -n 260Repository: NVIDIA/cudf Length of output: 27341 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- ColumnAccessor level helper ---'
sed -n '1,90p' python/cudf/cudf/core/column_accessor.py
rg -n -C20 'def _get_level' python/cudf/cudf/core/column_accessor.py python/cudf/cudf/core | head -n 180Repository: NVIDIA/cudf Length of output: 12156 Normalize When 🤖 Prompt for AI Agents |
||
| seen = set(self._data.keys()) | ||
| for name, _ in new_column_items: | ||
| if name in seen: | ||
| raise ValueError(f"cannot insert {name}, already exists") | ||
| seen.add(name) | ||
| new_column_data = dict(new_column_items) | ||
| # This is to match pandas where the new data columns are always | ||
| # inserted to the left of existing data columns. | ||
| label_dtype = None | ||
|
|
@@ -4885,7 +4903,7 @@ def _reset_index( | |
| ColumnAccessor( | ||
| {**new_column_data, **self._data}, | ||
| self._data.multiindex, | ||
| self._data._level_names, | ||
| self._data.level_names, | ||
| label_dtype=label_dtype, | ||
| ), | ||
| index, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Accept NumPy integer level specifiers.
_validate_index_levelusesisinstance(level, int), so valid values such asnp.int64(0)on a flat index are treated as names and raiseKeyError. Use the existingis_integerpredicate, matchingget_level_values.Proposed fix
🤖 Prompt for AI Agents