Support skipna in groupby reductions (first/last, sum/prod/mean/median/min/max, idxmin/idxmax)

[galipremsagar]

Description

GroupBy reductions ignored their skipna argument: nulls were always dropped, regardless of skipna. This PR aligns groupby skipna handling with pandas across the reductions:

• first / last — Aggregation.first/last take skipna; with skipna=False the actual first/last element of each group is returned even when it is null (previously the first/last non-null value was returned). Threaded through _reduce via a small _FirstLastAggSpec callable agg-spec whose __str__/__name__ report the op name so validity checks and result naming are unchanged.

• sum / prod / mean / median / min / max — with skipna=False, a group containing any null in a column now yields a null result for that (group, column), matching pandas (libcudf otherwise always drops nulls). Implemented in _reduce by masking the result where a column's non-null count is less than the group size (using size() rather than the size aggregation, which is unsupported for string columns).

• idxmin / idxmax — now raise ValueError("idxmin/idxmax with skipna=False"), matching pandas, which cannot represent the label of a NA (previously cudf returned an incorrect positional result).

This drops the now-passing skipna xfail entries in the cudf.pandas pandas-test plugin for tests/groupby/test_reductions.py (test_first_last_skipna, test_mean_skipna, test_sum_skipna, test_multifunc_skipna, test_idxmin_idxmax_extremes_skipna).

Intentionally still xfailed (not skipna=False bugs)

• test_sum_skipna_object[False] — inherent cudf.pandas None-vs-NaN difference for object-dtype nulls (the skipna logic is correct; only null representation differs).
• test_multifunc_skipna[True-prod-values3] — an all-null prod should return the empty-product identity 1.0; cudf returns NA. This is min_count/empty-reduction semantics (skipna=True), not the skipna=False behavior fixed here.

var/std do not yet accept skipna (their explicit methods omit it and fall back to pandas under cudf.pandas), so they are out of scope here.

Tests

Added cuDF unit tests in tests/groupby/test_reductions.py:

• test_groupby_first_last_skipna / test_groupby_series_first_last_skipna
• test_groupby_reduction_skipna_false (sum/prod/mean/median/min/max × nullable + numpy dtypes)
• test_groupby_idxmin_idxmax_skipna_false_raises (DataFrame and Series groupby)

Each fails without the corresponding fix.

Verification

• pandas-testing/.../test_reductions.py under cudf.pandas (with plugin): 1311 passed, 53 xfailed, 0 failed, 0 XPASS.
• Full cuDF groupby unit suite: 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.

[copy-pr-bot]

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.

[coderabbitai]

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: 6972b54c-1f5e-4b3c-bd39-9ff63a122c2e

📥 Commits

Reviewing files that changed from the base of the PR and between 3f9b71f and a660d5e.

📒 Files selected for processing (1)

• python/cudf/cudf/tests/groupby/test_reductions.py

💤 Files with no reviewable changes (1)

• python/cudf/cudf/tests/groupby/test_reductions.py

---

📝 Walkthrough

Summary by CodeRabbit

• New Features
  • Added a skipna parameter to groupby.first() and groupby.last() (default True) to control how nulls affect results.
• Bug Fixes
  • Improved groupby null-handling for reductions when skipna=False, ensuring groups containing nulls yield null results where appropriate.
  • groupby.idxmin() / groupby.idxmax() now reject skipna=False with an error (matching pandas behavior).
• Tests
  • Added regression coverage for first/last, multiple reductions, and idxmin/idxmax (including as_index=False), plus updated expected-failure records.

Walkthrough

Adds skipna: bool = True to Aggregation.first and Aggregation.last, threads it through groupby reductions, adds null propagation for selected reductions when skipna=False, and updates tests and expected-failure metadata.

Changes

GroupBy skipna support

Layer / File(s)	Summary
Aggregation.first/last skipna binding python/cudf/cudf/core/_internals/aggregation.py	Aggregation.first and Aggregation.last accept skipna: bool = True and select NullPolicy.EXCLUDE or NullPolicy.INCLUDE for the underlying nth_element call.
GroupBy._reduce rework, _FirstLastAggSpec, and idxmin/idxmax wiring python/cudf/cudf/core/groupby/groupby.py	_NULL_PROPAGATING_REDUCTIONS enumerates reductions that propagate null when skipna=False. _FirstLastAggSpec threads skipna to Aggregation.first/last. GroupBy._reduce uses both for null masking. _wrap_idxmin_idxmax gains how and raises ValueError for skipna=False; all call sites updated.
New skipna tests and pandas-testing-plugin updates python/cudf/cudf/tests/groupby/test_reductions.py, python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py	Parametrized tests cover first/last skipna across dtypes, Series groupby, null-propagating reductions with skipna=False, and idxmin/idxmax exception parity. Resolved expected-failure entries are removed or updated in the plugin.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

improvement

Suggested reviewers

• vyasr
• Matt711
• wence-

Possibly related PRs

• rapidsai/cudf#22783: Also changes python/cudf/cudf/core/groupby/groupby.py around GroupBy._wrap_idxmin_idxmax and skipna handling for idxmin/idxmax.
• rapidsai/cudf#22904: Also updates python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py expected-failure mappings for groupby reduction tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.39% 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: adding skipna support to groupby reductions and idxmin/idxmax handling.
Description check	✅ Passed	The description directly matches the changeset, detailing the skipna behavior fixes, xfail cleanup, and added tests.
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.

_{✏️ 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.}

[coderabbitai]

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/groupby/groupby.py`:
- Around line 1454-1465: The code at line 1459 compares non_null_counts[name]
Series values against group_sizes, but when as_index=False, self.size() returns
a DataFrame instead of a Series, causing a shape/type mismatch in the
comparison. Extract the actual group size values from the group_sizes DataFrame
when it is a DataFrame (which occurs when as_index=False) before using it in the
comparison operation within the all_non_null DataFrame comprehension. Ensure the
extracted values can be properly broadcast with the Series comparison for each
column name.

🪄 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: cb2cd638-ba2d-4421-a67b-6d80149e14cb

📥 Commits

Reviewing files that changed from the base of the PR and between 96896b1 and 1b20000.

📒 Files selected for processing (4)

• python/cudf/cudf/core/_internals/aggregation.py
• python/cudf/cudf/core/groupby/groupby.py
• python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
• python/cudf/cudf/tests/groupby/test_reductions.py

[galipremsagar]

Thanks @mroeschke. Removed the option_context blocks from the new tests; the skipna logic in groupby.py runs regardless of pandas compatible mode, so they were not needed. For first/last I dropped the sort parametrization and pinned sort=True, since the only thing that needed compatible mode there was group output ordering, which is unrelated to skipna. I also merged the two reduction tests into one that reuses the as_index fixture from conftest.py. The remaining tests parametrize on first/last and skipna, which the existing conftest fixtures do not cover.

[galipremsagar]

/merge