Skip to content

Fix DataFrame.stack/unstack pandas incompatibilities - #23255

Closed
galipremsagar wants to merge 10 commits into
NVIDIA:release/26.08from
galipremsagar:pandas-tests-stack-unstack
Closed

Fix DataFrame.stack/unstack pandas incompatibilities#23255
galipremsagar wants to merge 10 commits into
NVIDIA:release/26.08from
galipremsagar:pandas-tests-stack-unstack

Conversation

@galipremsagar

Copy link
Copy Markdown
Contributor

Description

Running pandas' own test suite under cudf.pandas, tests/frame/test_stack_unstack.py had 64 failing tests. This PR fixes 60 of them in cuDF classic (the remaining 4 assert pandas BlockManager block layouts or monkeypatch pandas' private _Unstacker, which cuDF cannot meaningfully satisfy) and removes the corresponding 60 xfail entries from the pandas-testing plugin.

DataFrame.stack

  • Resolve level positionally: integer column-level names no longer collide with level positions (pandas' Index.get_level_values resolves integers by name first, so frames with integer level names returned data from the wrong level).
  • Validate out-of-bounds integer levels (IndexError) and duplicated level names (ValueError) with pandas' messages; previously negative out-of-bounds levels silently wrapped around.
  • Build the stacked level keys from the column MultiIndex's own levels/codes so per-level dtypes survive: int64 levels with missing entries no longer upcast to float64, and categorical levels stay categorical through the pylibcudf tile step (which only sees codes).
  • Emit stacked keys in appearance order, matching pandas. This replaces the argsort-based reordering, which misaligned column data for non-involution column permutations (e.g. a 3-cycle) and NaN keys.
  • Attach pandas-faithful levels/codes to the result index eagerly: the original index contributes its own levels/codes (as in pandas), a flat index and the tiled level(s) get appearance-order factorization. This lets a later unstack restore the original row/column order. The legacy dropna path preserves them by masking the codes instead of gathering the index.

unstack / _pivot

  • Order result rows/columns by the removed level's codes (level order preserved, missing keys first) instead of by sorted values with nulls last, matching pandas.
  • Propagate the source frame's column-axis level names into the result instead of hardcoding None. This also fixes the ValueError: Length of names must match number of levels crash when unstacking frames with MultiIndex columns.
  • 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 since they fill missing cells afterwards and keep the integer dtype.
  • Preserve unused categories of the removed level in the result's column levels (pandas GH 17845); this 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.

Supporting fixes

  • ColumnAccessor: NaN-containing labels now match under pandas' all-NaNs-equal semantics; to_pandas_index restores recorded per-level dtypes when the cast round-trips losslessly; the cached pandas columns index is primed with the exact source pd.MultiIndex at construction and survives accessor copies. Rebuilding from tuples re-sorts the levels, which changes the behavior of pandas operations that work on codes (e.g. legacy stack(sort=True)) after a fast-to-slow conversion.
  • MultiIndex: lazy codes/levels materialization now factorizes with sort=True, matching the sorted levels pandas produces for per-row-value construction (set_index/from_arrays).
  • sort_index(axis=1) now honors level= and sort_remaining= (previously silently ignored).
  • GroupBy.agg keeps MultiIndex columns for MultiIndex-column sources instead of flattening to tuple labels.
  • NumericalColumn.as_numerical_column no longer mutates the column's dtype in place on equal-pylibcudf-type casts; the column object may be shared with the caller's frame.
  • Bool columns with nulls convert to pandas with np.nan (not None) in pandas-compatible mode, matching pandas' upcast-to-object representation.
  • cudf.pandas: an instance's transfer-blocking state is no longer baked into the class-level cached _MethodProxy, where it leaked into every later call on any instance of the class (order-dependent test failures).

Two previously-xfailed test_unstack_multiindex categorical params in cuDF's own test suite now pass and are un-xfailed.

Verified against the full cuDF test suites (reshape, dataframe, series, indexes, groupby, window, private-objects, cudf_pandas_tests) and neighboring pandas-tests files (pivot/melt/crosstab/sort_index/indexes-multi) with no regressions.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@galipremsagar
galipremsagar requested a review from a team as a code owner July 14, 2026 04:14
@copy-pr-bot

copy-pr-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas labels Jul 14, 2026
@galipremsagar galipremsagar added bug Something isn't working non-breaking Non-breaking change labels Jul 14, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jul 14, 2026
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test e02681b

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Updates cuDF column conversion, MultiIndex metadata preservation, level-aware sorting, stack/unstack ordering, pivot integer promotion, and pandas compatibility tests.

Changes

Reshape and metadata compatibility

Layer / File(s) Summary
Column conversion and NaN label handling
python/cudf/cudf/core/column/numerical.py, python/cudf/cudf/core/column_accessor.py
Avoids mutating source dtypes during numerical casts, canonicalizes NaN-containing labels, preserves cached pandas indexes, and restores MultiIndex level dtypes.
Axis metadata propagation
python/cudf/cudf/core/dataframe.py, python/cudf/cudf/core/groupby/groupby.py
Preserves MultiIndex level dtypes, exact pandas indexes, multiindex state, and column-axis metadata across construction, assignment, binary operations, and empty aggregations.
Index level sorting and normalization
python/cudf/cudf/core/indexed_frame.py, python/cudf/cudf/core/multiindex.py
Honors selected MultiIndex levels, supports negative and named levels, validates directions, and applies stable missing-aware sorting.
Stack level resolution and assembly
python/cudf/cudf/core/dataframe.py
Validates stack levels, preserves level dtypes and ordering through explicit levels/codes, retains scalar label types, and applies legacy dropna masking explicitly.
Pivot and unstack encoding
python/cudf/cudf/core/reshape.py
Unifies pivot and unstack encoding, preserves pandas code ordering and unused categories, restores column level names, and conditionally promotes integer results when missing cells are introduced.
Reshape compatibility validation
python/cudf/cudf/tests/reshape/*, python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Promotes categorical unstack cases from xfail, adds sparse integer pivot-table coverage, updates headers, and revises pandas-testing failure mappings.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested reviewers: brandon-b-miller, vyasr

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: fixing DataFrame stack/unstack pandas incompatibilities.
Description check ✅ Passed The description is directly about the stack/unstack compatibility fixes and related supporting changes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved pandas-compatible handling of missing boolean values and nullable floating-point data.
    • Preserved MultiIndex level names, dtypes, ordering, missing values, and categorical metadata across reshaping operations.
    • Enhanced stack, unstack, pivot, and pivot_table behavior, including integer handling when reshaping introduces missing values.
    • Added MultiIndex-aware column sorting by level.
    • Improved lookup of labels containing NaN.
    • Fixed transfer settings being incorrectly shared between proxy instances.
    • Improved empty aggregation metadata preservation.

Walkthrough

Changes

Reshape and metadata compatibility

Layer / File(s) Summary
Column conversion and label fidelity
python/cudf/cudf/core/column/*, python/cudf/cudf/core/column_accessor.py
Preserves pandas-compatible missing values, avoids dtype mutation during casts, handles NaN labels, and restores MultiIndex level dtypes.
DataFrame axis metadata propagation
python/cudf/cudf/core/dataframe.py, python/cudf/cudf/core/groupby/groupby.py
Propagates level dtypes, exact pandas MultiIndex caches, and multiindex state through construction, assignment, and empty aggregation.
Stack level resolution and assembly
python/cudf/cudf/core/dataframe.py
Validates stack levels and preserves ordering, dtypes, missing values, and legacy filtering behavior.
Pivot and unstack ordering
python/cudf/cudf/core/reshape.py
Adds code-based reshape ordering, category preservation, level-name propagation, and configurable integer promotion.
Index ordering and materialization
python/cudf/cudf/core/indexed_frame.py, python/cudf/cudf/core/multiindex.py
Adds level-aware sorting and sorted lazy MultiIndex factorization.

Proxy descriptor isolation

Layer / File(s) Summary
Callable proxy transfer state
python/cudf/cudf/pandas/fast_slow_proxy.py, python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py, python/cudf/cudf/tests/reshape/test_unstack.py
Stops cached callable descriptors from retaining instance-specific transfer state and updates related expectations and unstack cases.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested labels: improvement

Suggested reviewers: mroeschke, vyasr, matt711

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: pandas compatibility fixes for DataFrame stack/unstack behavior.
Description check ✅ Passed The description directly matches the changeset and explains the stack/unstack incompatibility fixes in detail.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
python/cudf/cudf/core/column/numerical.py (1)

861-909: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Move the equivalent-dtype short-circuit above the float→nullable-float branch
The current ordering makes the nans_to_nulls() fast path unreachable for plain NumPy float columns cast to pandas nullable floats; the earlier branch returns first. Reordering restores the intended zero-copy path.

🤖 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/column/numerical.py` around lines 861 - 909, Move the
dtype-equivalence short-circuit in the numerical column cast flow before the
float-to-pandas-nullable-float branch. Ensure plain NumPy float columns
targeting an equivalent nullable-float dtype reach the existing nans_to_nulls()
handling and ColumnBase.create path, while preserving the other conversion
checks and return behavior.
🤖 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/column_accessor.py`:
- Around line 368-394: Update the round-trip validation in the dtype restoration
loop around result.levels and self._level_dtypes to compare the cast-back index
using Index.equals. Preserve the existing cast exception handling and only
assign cast_lvl when the Index.equals comparison confirms the values, including
missing entries, are unchanged.

In `@python/cudf/cudf/core/indexed_frame.py`:
- Around line 2876-2899: Update the MultiIndex column sorting branch in the
level-based axis=1 path to preserve per-level ascending values instead of
reducing ascending to a single reverse boolean. Apply the same iterable-aware
ordering semantics used by the row-axis _get_sorted_inds path while retaining
scalar ascending behavior and the existing key_order/sort_remaining logic.

In `@python/cudf/cudf/core/reshape.py`:
- Around line 1213-1222: The pivot path in
python/cudf/cudf/core/reshape.py:1213-1222 must pass
promote_ints_on_missing=True when calling _pivot. In the pivot_table path at
python/cudf/cudf/core/reshape.py:1741-1758, pass
promote_ints_on_missing=(fill_value is None) to _unstack, preserving integer
types when missing combinations are filled by default while skipping promotion
when fill_value is provided, including 0.

---

Outside diff comments:
In `@python/cudf/cudf/core/column/numerical.py`:
- Around line 861-909: Move the dtype-equivalence short-circuit in the numerical
column cast flow before the float-to-pandas-nullable-float branch. Ensure plain
NumPy float columns targeting an equivalent nullable-float dtype reach the
existing nans_to_nulls() handling and ColumnBase.create path, while preserving
the other conversion checks and return 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: ac36fa3c-ec81-46c6-bd20-89780580e270

📥 Commits

Reviewing files that changed from the base of the PR and between e9cb870 and f356c93.

📒 Files selected for processing (11)
  • python/cudf/cudf/core/column/column.py
  • python/cudf/cudf/core/column/numerical.py
  • python/cudf/cudf/core/column_accessor.py
  • python/cudf/cudf/core/dataframe.py
  • python/cudf/cudf/core/groupby/groupby.py
  • python/cudf/cudf/core/indexed_frame.py
  • python/cudf/cudf/core/multiindex.py
  • python/cudf/cudf/core/reshape.py
  • python/cudf/cudf/pandas/fast_slow_proxy.py
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
  • python/cudf/cudf/tests/reshape/test_unstack.py

Comment thread python/cudf/cudf/core/column_accessor.py
Comment thread python/cudf/cudf/core/indexed_frame.py
Comment thread python/cudf/cudf/core/reshape.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
python/cudf/cudf/core/groupby/groupby.py (1)

1214-1240: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve multiindex in the empty-columns path. When len(data) == 0, this branch still forces multiindex=False, so a frame with MultiIndex columns and no value columns will rebuild as a flat empty Index instead of an empty MultiIndex. Mirror the adjacent branch and pass self.obj._data.multiindex here too.

♻️ Proposed fix
         if len(data) == 0 and not multilevel and self.obj.ndim == 2:
             data = ColumnAccessor(
                 data,
-                multiindex=False,
+                multiindex=self.obj._data.multiindex,
                 level_names=self.obj._data.level_names,
                 rangeindex=self.obj._data.rangeindex,
                 label_dtype=self.obj._data.label_dtype,
                 level_dtypes=self.obj._data.level_dtypes,
             )
🤖 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/groupby/groupby.py` around lines 1214 - 1240, Update
the empty-columns branch in the groupby result construction to pass
self.obj._data.multiindex to ColumnAccessor instead of forcing multiindex=False.
Preserve the existing source column metadata and behavior for non-empty data and
the adjacent branch.
🤖 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/column_accessor.py`:
- Around line 368-394: Update the round-trip guard in the level restoration
logic around result.set_levels to use Index.equals() for the cast_lvl comparison
instead of elementwise ==. Preserve the existing cast exception handling and
only restore the level dtype when the missing-aware equality check confirms a
lossless round trip.

In `@python/cudf/cudf/core/indexed_frame.py`:
- Around line 2893-2897: Update the label-sorting logic in the indexed-frame
sorting method around the self._column_names branch to consume iterable
ascending values per key level and honor na_position for null labels. Replace
the single reverse=not ascending sort with a null-aware stable multi-key sort,
preserving correct precedence and direction for each level.
- Around line 2882-2887: Update the local _level_number helper used to build
key_order so negative integer levels are normalized only when they are within
the valid range; reject values below -nlevels and nonnegative values at or above
nlevels before indexing or sorting. Preserve valid negative and nonnegative
level behavior while raising the established invalid-level exception for
out-of-range inputs.

In `@python/cudf/cudf/core/reshape.py`:
- Around line 1011-1014: Update the column-level name construction in the
unstack reshape flow to use the public axis names from columns_labels, via its
names/axis-name API, instead of the internal _column_names keys. Preserve
col_accessor.level_names and ensure duplicate level names remain unchanged and
correctly represented for positional multi-level unstack.
- Around line 1433-1443: Update the MultiIndex restoration logic around
full_level, pdi, and new_codes so missing labels represented by -1 do not
prevent restoring unused categories from full_level. Preserve the existing valid
codes, map missing entries to the appropriate missing-code representation, and
rebuild result._data.to_pandas_index with the complete level metadata even when
the unstacked level contains NA.
- Line 1757: Update the _unstack call in the pivot-table path so
promote_ints_on_missing is enabled only when fill_value is not None, preserving
pandas-style float promotion for sparse integer results without a fill value.
Add a regression test covering a sparse integer sum/min/max pivot with
fill_value=None and verifying the resulting nullable behavior.

---

Outside diff comments:
In `@python/cudf/cudf/core/groupby/groupby.py`:
- Around line 1214-1240: Update the empty-columns branch in the groupby result
construction to pass self.obj._data.multiindex to ColumnAccessor instead of
forcing multiindex=False. Preserve the existing source column metadata and
behavior for non-empty data and the adjacent branch.
🪄 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: 48c25aa3-4729-4173-8aaa-24e63c11f708

📥 Commits

Reviewing files that changed from the base of the PR and between e9cb870 and e02681b.

📒 Files selected for processing (12)
  • python/cudf/cudf/core/column/column.py
  • python/cudf/cudf/core/column/numerical.py
  • python/cudf/cudf/core/column_accessor.py
  • python/cudf/cudf/core/dataframe.py
  • python/cudf/cudf/core/groupby/groupby.py
  • python/cudf/cudf/core/indexed_frame.py
  • python/cudf/cudf/core/multiindex.py
  • python/cudf/cudf/core/reshape.py
  • python/cudf/cudf/pandas/fast_slow_proxy.py
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
  • python/cudf/cudf/tests/reshape/test_concat.py
  • python/cudf/cudf/tests/reshape/test_unstack.py

Comment thread python/cudf/cudf/core/column_accessor.py
Comment thread python/cudf/cudf/core/indexed_frame.py
Comment thread python/cudf/cudf/core/indexed_frame.py Outdated
Comment thread python/cudf/cudf/core/reshape.py
Comment thread python/cudf/cudf/core/reshape.py
Comment thread python/cudf/cudf/core/reshape.py
Comment thread python/cudf/cudf/core/reshape.py Outdated
else:
to_unstack.append(name)
table = agged.unstack(to_unstack)
table = _unstack(agged, to_unstack, promote_ints_on_missing=False)

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C3 \
  'pivot_table|fill_value|aggfunc.*(sum|min|max)' \
  python/cudf/cudf/tests -g 'test_*.py'

Repository: rapidsai/cudf

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant implementation around the cited lines.
sed -n '1710,1785p' python/cudf/cudf/core/reshape.py

echo
echo '--- pivot_table tests ---'
rg -n -C 3 'pivot_table|fill_value|sum|min|max' python/cudf/cudf/tests -g 'test_*.py'

Repository: rapidsai/cudf

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- reshape implementation ---'
sed -n '1730,1775p' python/cudf/cudf/core/reshape.py

echo
echo '--- pivot table tests focused ---'
sed -n '1,220p' python/cudf/cudf/tests/reshape/test_pivot_table.py

echo
echo '--- find fill_value=None pivot tests ---'
rg -n -C 2 'fill_value\s*=\s*None|fill_value=None|pivot_table.*None' python/cudf/cudf/tests/reshape/test_pivot_table.py

Repository: rapidsai/cudf

Length of output: 4203


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 'def _unstack|promote_ints_on_missing' python/cudf/cudf/core/reshape.py python/cudf/cudf -g '*.py'

Repository: rapidsai/cudf

Length of output: 10552


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 6 'def _unstack|promote_ints_on_missing' python/cudf/cudf/core/reshape.py

Repository: rapidsai/cudf

Length of output: 4169


Gate integer promotion on whether missing cells are filled
_unstack(..., promote_ints_on_missing=False) is still used when fill_value=None, so sparse integer sum/min/max pivot tables can keep nullable integer dtypes instead of pandas-style floats. Make the flag depend on fill_value is not None, and add a sparse integer fill_value=None case.

🤖 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/reshape.py` at line 1757, Update the _unstack call in
the pivot-table path so promote_ints_on_missing is enabled only when fill_value
is not None, preserving pandas-style float promotion for sparse integer results
without a fill value. Add a regression test covering a sparse integer
sum/min/max pivot with fill_value=None and verifying the resulting nullable
behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b769725promote_ints_on_missing=fill_value is None, matching pandas (verified: pandas 3.0.3 gives float64 for a sparse int sum with fill_value=None and int64 with fill_value=0). Added test_pivot_table_sparse_int_fill_value covering sum/min/max × fill_value None/0.

@galipremsagar

Copy link
Copy Markdown
Contributor Author

Merged main (resolving conflicts with #22966, which landed a parametrized _maybe_materialize_codes_and_levels(sort=...) — this branch's eager levels/codes attachment in stack supersedes its sort=False call there) and addressed the CodeRabbit review in b769725, including the outside-diff finding: the groupby empty-columns branch now preserves multiindex from the source frame. Validated: test_stack_unstack.py still at 1020 passed / 4 inherent failures; classic reshape/groupby/indexes/sort_index suites green (20k+ tests); pivot/melt/crosstab pandas-tests match the known-failures list.

@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test b769725

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Note

We couldn't fetch the incremental changes for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/reshape.py`:
- Around line 1076-1078: Update the MultiIndex unstack flow around remaining,
encode_side, and _pivot to special-case a one-level MultiIndex. Validate the
requested level, flatten that sole index level, and route through the existing
single-index path so the result is the expected stacked Series rather than an
empty DataFrame; preserve current behavior for multi-level indexes.
🪄 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: 1073532d-d61f-49ef-91c9-53a9aac17b99

📥 Commits

Reviewing files that changed from the base of the PR and between 015e090 and b769725.

📒 Files selected for processing (15)
  • python/cudf/cudf/core/column/column.py
  • python/cudf/cudf/core/column/numerical.py
  • python/cudf/cudf/core/column_accessor.py
  • python/cudf/cudf/core/dataframe.py
  • python/cudf/cudf/core/groupby/groupby.py
  • python/cudf/cudf/core/indexed_frame.py
  • python/cudf/cudf/core/multiindex.py
  • python/cudf/cudf/core/reshape.py
  • python/cudf/cudf/pandas/fast_slow_proxy.py
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
  • python/cudf/cudf/tests/dataframe/test_np_ufuncs.py
  • python/cudf/cudf/tests/reshape/test_concat.py
  • python/cudf/cudf/tests/reshape/test_pivot_table.py
  • python/cudf/cudf/tests/reshape/test_unstack.py
  • python/cudf/cudf/tests/series/test_np_ufuncs.py
🚧 Files skipped from review as they are similar to previous changes (8)
  • python/cudf/cudf/tests/dataframe/test_np_ufuncs.py
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
  • python/cudf/cudf/tests/series/test_np_ufuncs.py
  • python/cudf/cudf/core/column_accessor.py
  • python/cudf/cudf/pandas/fast_slow_proxy.py
  • python/cudf/cudf/core/indexed_frame.py
  • python/cudf/cudf/core/column/column.py
  • python/cudf/cudf/tests/reshape/test_concat.py

Comment thread python/cudf/cudf/core/reshape.py
@galipremsagar

Copy link
Copy Markdown
Contributor Author

CI failures investigated and fixed in 6dab03e. Three root causes:

  1. The ungated bool-with-nulls to_pandas conversion had to be dropped entirely. CI showed pandas is not self-consistent here: read_orc/read_parquet ingest produces object columns holding None for null bools, while unstack's upcast produces np.nan — so the conversion change broke 4 ORC/Parquet reader tests, 2 to_pandas doctests, and an eval/numexpr pandas-test that all compare against pandas' literal None. The test_unstack_bool plugin entry is restored (with a real reason) and the dependent test-expectation updates are reverted.

  2. The _MethodProxy transfer-block fix is reverted. Removing the baked block exposed ~200 pandas-test failures (strings/extract/find_replace/split, etc.) that pass on main only because an early mixed-object test poisons the class-level method cache into forcing the slow path for every later test in the process — verified: with main's proxy, test_findall[string=str[python]] fails alone but passes when run after test_findall[string=object]. That contamination (and the genuine divergences it hides) deserves a dedicated PR rather than riding on this one.

  3. Two genuine metadata regressions fixed: relabeling groupby aggregations (agg(new=(col, func))) crashed rebuilding the columns index when the source had MultiIndex columns (the source's multi-level metadata was applied to the new flat labels), and DataFrame binops lost hierarchical columns when the operands' labels matched but the equals check failed on level-dtype differences (Int8 vs int64 after level-dtype restoration).

Plugin reconciliation: this PR's reshape/metadata fixes turned 61 existing entries into strict XPASSes in the last CI run — 49 verified passing in isolation are pruned; the 12 that only pass/fail depending on suite ordering keep their entries, plus 5 stack/unstack entries re-added that regress to main's ordering behavior with the proxy revert.

@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 44184b0

@galipremsagar

Copy link
Copy Markdown
Contributor Author

Fixed the pandas-tests CI failures (33 = 28 + 5, both plugin bookkeeping — no code changes needed):

  • 28 tests/strings/test_extract.py failures: this branch had removed those xfail entries because the tests passed in local full-suite runs, but CI shows they still fail there (they're part of the known string=object extract set that's been in the failure list since the plugin was created). Restored the entries verbatim from main.
  • 5 tests/frame/test_stack_unstack.py strict XPASSes: the entries kept with reason "passes in isolation; fails under full-suite ordering (method-cache transfer-block contamination)" actually pass in CI's full-suite run — the contamination is a local-only effect. Removed the 5 entries.
  • Also merged latest main (clean).

Net plugin state vs main: 79 entries removed (58 stack/unstack + 21 neighbors), 0 added; the 3 remaining stack/unstack entries are the documented inherent ones. Local re-validation: classic reshape suite green (4652 tests); pandas-tests test_stack_unstack.py clean except the memory-heavy test_unstack_number_of_levels_larger_than_int32_warns pair and test_extract.py None != nan diffs, all of which pass in CI and fail locally only due to a stale local libcudf build (predates #21936/#22178) and GPU memory contention.

@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 69e8bb9

@galipremsagar galipremsagar added the 3 - Ready for Review Ready for review by team label Jul 15, 2026
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 963202f

Fixes 60 pandas unit-test failures in tests/frame/test_stack_unstack.py
under cudf.pandas (64 -> 4; the remaining 4 assert BlockManager
internals or monkeypatch pandas' private _Unstacker).

DataFrame.stack:
* Resolve levels positionally: integer column-level names no longer
  collide with level positions in get_level_values lookups.
* Validate out-of-bounds integer levels (IndexError) and duplicated
  level names (ValueError) like pandas.
* Build the stacked level keys from the column MultiIndex's own
  levels/codes so level dtypes survive (int64 levels with missing
  entries no longer upcast to float64; categorical levels stay
  categorical through the tile step).
* Emit stacked keys in appearance order like pandas, replacing the
  argsort-based reordering that misaligned data for non-involution
  column permutations and NaN keys.
* Attach pandas-faithful levels/codes to the result index eagerly
  (reusing the original MultiIndex's levels) so a later unstack
  restores the original row/column order; legacy dropna keeps them.

unstack/_pivot:
* Order result rows/columns by the removed level's codes (level order,
  missing keys first) instead of sorted values, matching pandas.
* Propagate the source frame's column-axis level names instead of
  hardcoding None; fixes the 'Length of names must match number of
  levels' failure for frames with MultiIndex columns.
* Promote integer columns to float64 when unstack introduces missing
  cells (pandas block semantics); pivot_table/crosstab opt out since
  they fill missing cells afterwards.
* Preserve unused categories of the removed level in the result's
  column levels (pandas GH 17845).
* Validate flat-index level (KeyError) and duplicated index names
  (ValueError) like pandas.

Supporting fixes:
* ColumnAccessor: NaN-containing labels now match under pandas'
  all-NaNs-equal semantics; to_pandas_index restores recorded per-level
  dtypes when the cast round-trips losslessly; the primed/cached pandas
  columns index survives accessor copies so explicit unsorted level
  layouts are not lost on fast-to-slow conversion.
* MultiIndex: lazy codes/levels materialization sorts levels
  (pandas-canonical for per-row-value construction).
* sort_index(axis=1) now honors level= and sort_remaining=.
* GroupBy.agg keeps MultiIndex columns for MultiIndex-column sources.
* NumericalColumn.as_numerical_column no longer mutates the column
  dtype in place on equal-pylibcudf-type casts.
* Bool columns with nulls convert to pandas with np.nan (not None) in
  pandas-compatible mode.
* cudf.pandas: do not bake one instance's transfer-blocking state into
  the class-level cached _MethodProxy (order-dependent test poisoning).

Removes the 60 fixed xfail entries from the pandas-testing plugin and
un-xfails now-passing cudf unstack tests with categorical indexes.
Bool was the only dtype whose default-mode to_pandas emitted None for
missing values; strings, categoricals and ints already produce nan and
datetimes produce NaT, and pandas itself never places None in an
upcast-to-object bool column. Dropping the mode.pandas_compatible gate
makes the conversion consistent across dtypes and with pandas.

This also makes concat of bool and float frames match pandas' float
coercion; the corresponding strict xfail in test_concat.py now passes
and is removed.
The ufunc tests masked expected bool results with None to match the old
to_pandas conversion, with a comment asking whether it should be np.nan
instead; it is now.
* GroupBy.agg: preserve MultiIndex columns in the empty-columns branch.
* ColumnAccessor.to_pandas_index: use missing-aware Index.equals for the
  lossless-cast round-trip guard.
* sort_index(axis=1, level=...): validate out-of-range integer levels
  (matching pandas' IndexError), honor per-level ascending lists, and
  place missing labels per na_position via a null-aware stable
  multi-key sort.
* MultiIndex._level_index_from_level: reject still-negative levels after
  normalization instead of silently indexing from the end; align the
  error message with pandas.
* unstack: use the public level names for the result's column levels and
  keep restoring unused categories when the removed level contains NA
  (-1 codes for NA labels are pandas' canonical representation).
* pivot_table: promote integer values to float64 like pandas when
  missing cells are left unfilled (fill_value=None); keep the integer
  dtype when fill_value is provided. Add a regression test.
@galipremsagar
galipremsagar changed the base branch from main to release/26.08 July 17, 2026 01:48
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 17e81ef

@galipremsagar
galipremsagar requested a review from mroeschke July 17, 2026 13:51
…ts-stack-unstack

# Conflicts:
#	python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test d50efcc

@vyasr

vyasr commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

This is a pretty large PR. Any chance you can split it up?

…ts-stack-unstack

# Conflicts:
#	python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
@galipremsagar

Copy link
Copy Markdown
Contributor Author

Split into six focused PRs as requested, each carrying exactly the xfail-list entries its fix makes pass (attribution verified by running every removed node id against isolated builds containing only that PR's change — details in each description):

Suggested merge order: #23364/#23366/#23367 (independent) → #23365#23370#23368. Closing this PR in favor of the split.

@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Jul 21, 2026
rapids-bot Bot pushed a commit that referenced this pull request Jul 21, 2026
…sts (#23364)

Split out of #23255 (1/6).

`NumericalColumn.as_numerical_column` short-circuits casts between equivalent dtypes (same pylibcudf type, e.g. `float64` → `Float64`), but implemented the shortcut by assigning the target dtype onto `self._dtype` in place. The column object is shared with the caller's Series/DataFrame, so the *source* object silently changed dtype as a side effect of the cast. This returns a fresh column over the same pylibcudf data instead (`nans_to_nulls` first for float → masked casts), and adds a classic regression test.

Fixes 5 pandas-tests (`test_stack_nullable_dtype[*]`, `test_loc_set_nan_in_categorical_series[Float64]`, `test_assert_series_equal_extension_dtype_mismatch`, `test_assert_frame_equal_extension_dtype_mismatch`); their xfail entries are removed. Attribution verified by running the node ids against an isolated build containing only this fix (they pass) and a clean build (they fail).

Independent of the other #23255 split PRs; can merge in any order.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #23364
davidwendt pushed a commit to wjxiz1992/cudf that referenced this pull request Jul 21, 2026
…sts (NVIDIA#23364)

Split out of NVIDIA#23255 (1/6).

`NumericalColumn.as_numerical_column` short-circuits casts between equivalent dtypes (same pylibcudf type, e.g. `float64` → `Float64`), but implemented the shortcut by assigning the target dtype onto `self._dtype` in place. The column object is shared with the caller's Series/DataFrame, so the *source* object silently changed dtype as a side effect of the cast. This returns a fresh column over the same pylibcudf data instead (`nans_to_nulls` first for float → masked casts), and adds a classic regression test.

Fixes 5 pandas-tests (`test_stack_nullable_dtype[*]`, `test_loc_set_nan_in_categorical_series[Float64]`, `test_assert_series_equal_extension_dtype_mismatch`, `test_assert_frame_equal_extension_dtype_mismatch`); their xfail entries are removed. Attribution verified by running the node ids against an isolated build containing only this fix (they pass) and a clean build (they fail).

Independent of the other NVIDIA#23255 split PRs; can merge in any order.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: NVIDIA#23364
rapids-bot Bot pushed a commit that referenced this pull request Jul 21, 2026
Split out of #23255 (6/6).

`sort_index(axis=1)` silently ignored `level=` and `sort_remaining=` and always sorted by the full column labels. Sort by the requested levels (stable multi-key, least significant first), append the remaining levels when `sort_remaining=True`, resolve integer and named levels with pandas' bounds validation, and place missing labels per `na_position` independently of the per-key sort direction.

Fixes 1 pandas-test (`test_stack_mixed_dtype[True]`, which sorts the stacked frame's columns by level); its xfail entry is removed. Attribution verified by running the node id against an isolated build containing only this change (passes) and a clean build (fails).

Independent of the other #23255 split PRs; can merge in any order.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #23367
rapids-bot Bot pushed a commit that referenced this pull request Jul 22, 2026
…23366)

Split out of #23255 (5/6).

`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[<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.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #23366
rapids-bot Bot pushed a commit that referenced this pull request Jul 22, 2026
#23365)

Split out of #23255 (2/6).

Rebuilding a frame's pandas columns `MultiIndex` from tuples re-sorts the levels and re-infers their dtypes, losing the exact source layout: unsorted explicit level orders (which change the behavior of pandas operations that work on level codes, e.g. legacy `stack(sort=True)` after a fast-to-slow conversion under `cudf.pandas`), categorical/object/int64 level dtypes (int64 levels with missing entries upcast to float64), and NaN column labels (fresh `float('nan')` objects hash unequal, so lookups miss).

- Prime the cached `to_pandas_index` with the exact source `pd.MultiIndex` at `DataFrame` construction and propagate it through accessor copies.
- Restore recorded per-level dtypes in `to_pandas_index` when the cast round-trips losslessly.
- Match NaN-containing column labels under pandas' all-NaNs-equal semantics.
- Read level dtypes off `MultiIndex.levels` (`get_level_values` materializes missing entries as NaN and upcasts), also for `cudf.MultiIndex` columns.
- Keep hierarchical columns through DataFrame binops when only level dtypes differ (restored `Int8` vs `int64` fails `Index.equals`).

Fixes 13 pandas-tests (constructor dict-NaN-key, concat keys with specific levels, groupby ordered multi-func aggregate, MultiIndex loc, and several `test_stack_unstack.py` cases); their xfail entries are removed. Attribution verified by running each node id against an isolated build containing only this change (pass) and a clean build (fail).

Independent of the other #23255 split PRs, but the stack (3/6) and unstack (4/6) PRs depend on this one.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #23365
rapids-bot Bot pushed a commit that referenced this pull request Jul 23, 2026
#23370)

Split out of #23255 (3/6), superseding it. **Depends on #23365 (MultiIndex column fidelity)** — 14 of the 28 un-xfailed pandas-tests need both fixes, so this PR's pandas-tests job goes green once #23365 merges.

- Resolve `level` positionally: integer column-level *names* no longer collide with level *positions* (pandas' `Index.get_level_values` resolves integers by name first, so frames with integer level names returned data from the wrong level).
- Validate out-of-bounds integer levels (`IndexError`) and duplicated level names (`ValueError`) with pandas' messages; negative out-of-bounds levels previously wrapped around silently.
- Build the stacked level keys from the column MultiIndex's own levels/codes so per-level dtypes survive: int64 levels with missing entries no longer upcast to float64, and categorical levels stay categorical through the pylibcudf `tile` step (which only sees codes).
- Emit stacked keys in appearance order, matching pandas. This replaces the argsort-based reordering, which misaligned column data for non-involution column permutations (e.g. a 3-cycle) and NaN keys; pandas legacy stack sorts multi-level keys by level *codes*, not values.
- Attach pandas-faithful levels/codes to the result index eagerly (the original index contributes its own levels/codes; flat indexes and the tiled level get appearance-order factorization) so a later `unstack` restores the original row/column order; the legacy `dropna` path preserves them by masking codes instead of gathering the index.

Fixes 28 pandas-tests; their xfail entries are removed. Three classic categorical unstack params are un-xfailed (fixed by this change together with #23365). Attribution verified per node id against isolated builds: 14 pass with only this change, 14 need this plus #23365.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Matthew Murray (https://github.com/Matt711)

URL: #23370
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for Review Ready for review by team bug Something isn't working cudf.pandas Issues specific to cudf.pandas non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants