Fix `DataFrame` and `GroupBy.value_counts` to match pandas ordering, `dropna`, and categorical handling by galipremsagar · Pull Request #22903 · NVIDIA/cudf

galipremsagar · 2026-06-16T14:09:44Z

Description

Reworks value_counts for DataFrame.value_counts and DataFrameGroupBy/SeriesGroupBy.value_counts so cudf matches pandas, and fixes a related cudf.pandas proxy bug. Together these resolve a number of pandas-compatibility test failures in tests/groupby/methods/test_value_counts.py.

Ordering of tied counts (`DataFrame` and groupby `value_counts`)

cudf's groupby does not preserve first-appearance order, so rows with equal counts were ordered differently from pandas. Both code paths now track the first row index of each unique combination and order by it, with a running sequence column that keeps the optional value sort (sort=) and the group-key sort stable for ties — cudf's value sort is not guaranteed stable.

`GroupBy.value_counts` correctness

Independent dropna: the groupby dropna (group keys) and the value_counts dropna (subset) are now applied independently, matching pandas, by grouping with dropna=False and filtering afterward.
Unobserved categorical combinations: when a subset column is categorical, pandas emits every category combination (count 0). cudf's groupby only emits observed combinations, so the result is expanded to the full product of the observed group keys × subset categories.
Grouper(freq=...) keys: the grouping's actual (binned) key values are used instead of the raw object columns, so a frequency grouper that floors its key produces the correct group labels.
Fixes normalize (divide each count by its per-group total) and as_index=False output.

The now-passing array-keyed, categorical, and time-grouper test_value_counts cases are removed from the cudf.pandas xfail list.

Checklist

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

copy-pr-bot · 2026-06-16T14:09:49Z

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.

galipremsagar · 2026-06-16T14:12:42Z

/okay to test f588575

galipremsagar · 2026-06-16T20:40:39Z

/okay to test 0b418b2

galipremsagar · 2026-06-17T00:58:51Z

/okay to test 13fd44b

coderabbitai · 2026-06-17T02:36:03Z

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

Two independent changes: (1) DataFrame.value_counts and DataFrameGroupBy.value_counts are rewritten to use position-tracking bookkeeping columns for pandas-compatible stable tie-breaking, categorical cartesian expansion, and independent dropna handling per group key vs subset; (2) _IntermediateProxy gains parent-staleness detection so slow↔fast conversions re-derive values when parent proxies' wrapped objects change.

Changes

value_counts stable ordering

Layer / File(s)	Summary
`DataFrame.value_counts` stable ordering `python/cudf/cudf/core/dataframe.py`	Replaces `groupby().size()` with `groupby().agg()` that also computes minimum first-appearance position per group, sorts by that position for pandas-compatible tied-count ordering, and always returns a MultiIndex result named `"count"` or `"proportion"`.
`DataFrameGroupBy.value_counts` rewrite and docstring `python/cudf/cudf/core/groupby/groupby.py`	Adds `cast` to typing imports; updates docstring examples for `ascending` and `as_index=False` cases; replaces counting/sorting/normalization logic with actual grouping-key column derivation, `__count`/`__pos`/`__seq` bookkeeping, independent `dropna` filtering, categorical cartesian expansion, per-group normalize via `transform("sum")`, and a duplicate output-label guard.
Remove resolved xfail entries `python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py`	Deletes `test_against_frame_and_seriesgroupby`, `test_value_counts_sort_categorical`, and other test-failure mappings from the pandas-testing plugin now that the implementation fixes resolve those failures; also removes DataFrame.value_counts and iloc test xfail entries.

IntermediateProxy parent staleness tracking

Layer / File(s)	Summary
`_collect_parent_proxies` helper and `_IntermediateProxy` state `python/cudf/cudf/pandas/fast_slow_proxy.py`	Adds `_collect_parent_proxies` to iteratively traverse method-chain args/kwargs (nested structures and object-dtype arrays) gathering `_FastSlowProxy` instances via an identity-based `seen` set; `_IntermediateProxy._fsproxy_wrap` now stores collected parents and snapshots their wrapped-object identities; `_fsproxy_record_parent_ids`/`_fsproxy_parents_changed` detect staleness; unpickling re-collects and re-snapshots parents.
slow→fast and fast→slow re-derivation `python/cudf/cudf/pandas/fast_slow_proxy.py`	Both conversion property paths now call `_fsproxy_parents_changed`; if any parent's wrapped object changed, the value is re-derived from the updated parent proxies and the parent-id snapshot is refreshed before returning.
Test: groupby reflects parent frame mutation `python/cudf/cudf_pandas_tests/test_cudf_pandas.py`	Adds `test_groupby_reflects_parent_frame_mutation`, which creates groupby objects before adding a new column to the parent DataFrame and asserts post-mutation aggregation results are consistent between pandas and cudf-pandas.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

rapidsai/cudf#22883: Edits the same pandas-testing-plugin.py failure mappings by removing stale test node IDs, including value_counts-related entries.

Suggested reviewers

vyasr
rjzamora
mroeschke
brandon-b-miller

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% 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 and specifically summarizes the main changes: fixing DataFrame and GroupBy.value_counts methods to match pandas ordering, dropna handling, and categorical handling.
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.
Description check	✅ Passed	The description accurately matches the value_counts and cudf.pandas proxy changes in the patch.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

galipremsagar · 2026-06-18T18:45:53Z

/okay to test 4a8368a

galipremsagar · 2026-06-18T18:47:37Z

pre-commit.ci autofix

copy-pr-bot · 2026-06-18T18:48:48Z

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.

galipremsagar · 2026-06-18T18:52:35Z

/okay to test 8a7f906

galipremsagar · 2026-06-18T19:50:09Z

/okay to test 239f1e5

galipremsagar · 2026-06-19T04:45:04Z

/okay to test ca11e4d

galipremsagar · 2026-06-24T01:21:31Z

/okay to test 4f9c5e2

Per review, split the `_IntermediateProxy` parent-staleness fix (and its unit test plus the iloc xfail removals it unblocks) out of this PR; it now lives in NVIDIA#22998. This PR keeps only the DataFrame/GroupBy value_counts changes.

galipremsagar · 2026-06-25T19:22:19Z

Thanks @mroeschke.

Removed the redundant local CategoricalDtype import; it is already imported at the top of groupby.py.
Added the TODO noting the conversion to host objects can be dropped once MultiIndex.from_product supports GPU inputs.
Split the intermediate proxy staleness fix (fast_slow_proxy.py, its unit test, and the two test_iloc_setitem_axis_argument xfail removals it unblocks) out into Re-derive cudf.pandas intermediate proxies when their parents change #22998. This PR now keeps only the DataFrame and GroupBy value_counts changes.

galipremsagar · 2026-06-25T19:25:30Z

/okay to test 0bce1ba

galipremsagar · 2026-06-25T19:25:36Z

/merge

vyasr · 2026-06-27T00:00:44Z

@galipremsagar the pandas-tests failure here looks relevant. The other two issues are flaky ones.

galipremsagar · 2026-06-28T14:07:01Z

@galipremsagar the pandas-tests failure here looks relevant. The other two issues are flaky ones.

I have been seeing the same tests fail in other PRs when the following nvrtc error shows up in other jobs:

 RuntimeError: Failed to dlopen /pyenv/versions/3.11.15/lib/python3.11/site-packages/nvidia/cuda_nvrtc/lib/libnvrtc.so.12: /pyenv/versions/3.11.15/lib/python3.11/site-packages/nvidia/cuda_nvrtc/lib/libnvrtc.so.12: ELF load command address/offset not page-aligned

Trying a rerun.

galipremsagar · 2026-06-28T14:10:38Z

/okay to test 82b6c87

galipremsagar · 2026-06-28T17:37:43Z

/okay to test 02cee7e

galipremsagar · 2026-06-28T17:37:56Z

/merge

galipremsagar · 2026-06-29T14:53:09Z

/okay to test 7ea6954

galipremsagar · 2026-06-29T16:08:56Z

/merge

…22998) Split out of #22903 per review. ## Problem An `_IntermediateProxy` (e.g. a `groupby`, `rolling`, or accessor object) caches its wrapped object at creation time. When a parent proxy (the originating frame/series, or a proxy passed as an argument such as a grouping key) is mutated or replaced *after* the intermediate is created, the intermediate keeps returning the stale snapshot instead of reflecting the live parent. For example, a column added to a frame after a groupby was created is not visible through that groupby, which diverges from pandas' live-reference semantics: ```python df = xpd.DataFrame({"a": ["x", "y", "x", "y"], "b": [1, 2, 3, 4]}) gb = df.groupby(df["a"].values) # falls back to slow path -> proxy created in slow state df["c"] = df["b"] * 10 gb["c"].sum() # previously failed to see column "c" ``` ## Fix Snapshot the identity of each parent proxy's wrapped object when the intermediate is created, and on `_fsproxy_slow_to_fast` / `_fsproxy_fast_to_slow` re-derive the intermediate from the originating call when a parent has changed. Parent collection (`_collect_parent_proxies`) is intentionally iterative (an explicit stack) rather than a recursive closure: a recursive closure would hold a cell referencing itself, forming a reference cycle that delays reference-counted teardown and breaks code relying on prompt teardown (e.g. the pandas `Series.str`-accessor circular-reference test). ## Tests Adds `test_groupby_reflects_parent_frame_mutation`, and removes the now-passing `test_iloc_setitem_axis_argument[True/False]` xfails from the pandas-tests plugin. Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #22998

update

ec07aad

github-actions Bot assigned galipremsagar Jun 16, 2026

Merge branch 'main' into value_counts

f588575

github-actions Bot added Python Affects Python cuDF API. cudf.pandas Issues specific to cudf.pandas labels Jun 16, 2026

github-project-automation Bot added this to cuDF Python Jun 16, 2026

galipremsagar added bug Something isn't working non-breaking Non-breaking change labels Jun 16, 2026

GPUtester moved this to In Progress in cuDF Python Jun 16, 2026

galipremsagar changed the title ~~fix~~ Fix DataFrame and GroupBy.value_counts to match pandas ordering, dropna, and categorical handling Jun 16, 2026

galipremsagar added 2 commits June 16, 2026 20:40

fix reference issue

bb09823

Merge branch 'main' into value_counts

0b418b2

Merge branch 'main' into value_counts

13fd44b

Merge branch 'main' into value_counts

cae7355

galipremsagar marked this pull request as ready for review June 17, 2026 02:27

galipremsagar requested a review from a team as a code owner June 17, 2026 02:27

galipremsagar requested review from bdice and vyasr June 17, 2026 02:27

Merge branch 'main' into value_counts

01adae7

Merge

4a8368a

[pre-commit.ci] auto code formatting

8a7f906

fix

ca11e4d

Merge branch 'main' into value_counts

4f9c5e2

mroeschke reviewed Jun 25, 2026

View reviewed changes

Comment thread python/cudf/cudf/core/groupby/groupby.py Outdated

Comment thread python/cudf/cudf/core/groupby/groupby.py

Comment thread python/cudf/cudf/pandas/fast_slow_proxy.py

Address reviews

8a21d6a

galipremsagar mentioned this pull request Jun 25, 2026

Re-derive cudf.pandas intermediate proxies when their parents change #22998

Merged

galipremsagar requested a review from mroeschke June 25, 2026 19:22

Merge branch 'main' into value_counts

0bce1ba

mroeschke approved these changes Jun 25, 2026

View reviewed changes

galipremsagar added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Jun 25, 2026

Merge branch 'main' into value_counts

82b6c87

galipremsagar added 2 commits June 28, 2026 17:21

update

6d183c4

Merge branch 'main' into value_counts

02cee7e

Merge branch 'main' into value_counts

7ea6954

rapids-bot Bot merged commit d5e2bc5 into NVIDIA:main Jun 29, 2026
126 checks passed

github-project-automation Bot moved this from In Progress to Done in cuDF Python Jun 29, 2026

Uh oh!

Conversation

galipremsagar commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Ordering of tied counts (DataFrame and groupby value_counts)

GroupBy.value_counts correctness

Checklist

Uh oh!

copy-pr-bot Bot commented Jun 16, 2026

Uh oh!

galipremsagar commented Jun 16, 2026

Uh oh!

galipremsagar commented Jun 16, 2026

Uh oh!

galipremsagar commented Jun 17, 2026

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

galipremsagar commented Jun 18, 2026

Uh oh!

galipremsagar commented Jun 18, 2026

Uh oh!

copy-pr-bot Bot commented Jun 18, 2026

Uh oh!

galipremsagar commented Jun 18, 2026

Uh oh!

galipremsagar commented Jun 18, 2026

Uh oh!

galipremsagar commented Jun 19, 2026

Uh oh!

galipremsagar commented Jun 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

galipremsagar commented Jun 25, 2026

Uh oh!

galipremsagar commented Jun 25, 2026

Uh oh!

galipremsagar commented Jun 25, 2026

Uh oh!

vyasr commented Jun 27, 2026

Uh oh!

galipremsagar commented Jun 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galipremsagar commented Jun 28, 2026

Uh oh!

galipremsagar commented Jun 28, 2026

Uh oh!

galipremsagar commented Jun 28, 2026

Uh oh!

galipremsagar commented Jun 29, 2026

Uh oh!

galipremsagar commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

galipremsagar commented Jun 16, 2026 •

edited

Loading

Ordering of tied counts (`DataFrame` and groupby `value_counts`)

`GroupBy.value_counts` correctness

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

galipremsagar commented Jun 28, 2026 •

edited

Loading