Skip to content

Reenable fixed GroupBy value_counts tests in pandas shards - #24080

Open
galipremsagar wants to merge 3 commits into
NVIDIA:mainfrom
galipremsagar:enable-groupby-value-counts-shards
Open

Reenable fixed GroupBy value_counts tests in pandas shards#24080
galipremsagar wants to merge 3 commits into
NVIDIA:mainfrom
galipremsagar:enable-groupby-value-counts-shards

Conversation

@galipremsagar

Copy link
Copy Markdown
Contributor

Description

Remove nine stale GroupBy value_counts entries from NODEIDS_TO_SKIP_WHEN_SHARDED (#22992).

The original CI failures were KeyError: Column not found: both: these tests create a GroupBy, add a column to its parent DataFrame, then select that column through the existing GroupBy. #22998 already fixed this stale-parent intermediate-proxy bug and added regression coverage, but these shard-only skips remained.

Independent main-based, marker-only change.

Validation

  • All nine cases pass individually and in both broader module shards.
  • Independent full value_counts module: 184 passed, 14 xfailed, 25 explicitly non-strict xpassed, no failures.
  • Both broader stack/value_counts shards with the separate descriptor fix integrated: 619 passed / 584 passed, no failures. Existing upstream xfails/non-strict xpasses remain unchanged.
  • All applicable pre-commit hooks passed.

Tested with pandas 3.0.3 and source Python using installed cuDF 26.10 native libraries (main is 26.12), with an external hook for the installed extension and exact GPU mask API rename. No environment or vendored pandas test changes were made. Matching native-library validation remains for CI.

These nine skips report KeyError for a column added after groupby creation. Intermediate proxies have re-derived against mutated parents since d227e8d (NVIDIA#22998), which removed related ordinary xfails but left these shard-only skips.

All nine cases pass in isolation and both full module shards. The full value_counts module passes independently; both stack/value_counts shards also pass when integrated with the per-instance descriptor-dispatch fix.
@galipremsagar
galipremsagar requested a review from a team as a code owner September 9, 2026 17:33
@copy-pr-bot

copy-pr-bot Bot commented Sep 9, 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 Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e4f96214-b108-4cce-94b0-d8fe4fbd88ee

📥 Commits

Reviewing files that changed from the base of the PR and between 0d92fc8 and 36258c0.

📒 Files selected for processing (1)
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
💤 Files with no reviewable changes (1)
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py

Included review availability: Your plan provides up to 12 included reviews per hour; 0 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Tests
    • Expanded pandas compatibility test coverage for groupby.value_counts scenarios.
    • Previously skipped cases are now included in the test suite, helping validate behavior across additional grouping and counting workflows.

Walkthrough

The pandas testing plugin no longer skips nine groupby.value_counts test cases for frame and series groupby operations.

Changes

Groupby value-counts test enablement

Layer / File(s) Summary
Remove groupby value-counts skips
python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Removed skip registrations for nine test_against_frame_and_seriesgroupby value-counts tests.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 3a69e

Nine previously skipped GroupBy value-count tests are enabled, with the supplied test results showing they pass in targeted and broader test runs. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: re-enabling fixed GroupBy value_counts tests in pandas shards.
Description check ✅ Passed The description directly explains the removal of nine stale skips, the earlier fix, and the validation results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@galipremsagar galipremsagar added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 9, 2026
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test eb51e89

@galipremsagar galipremsagar added improvement Improvement / enhancement to an existing function and removed improvement Improvement / enhancement to an existing function labels Sep 9, 2026
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 3a69e57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cudf.pandas Issues specific to cudf.pandas improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant