Match pandas semantics for null reduction results and mode ordering - #23328
Merged
rapids-bot[bot] merged 8 commits intoJul 21, 2026
Merged
Conversation
Fixes 34 pandas-tests failures in tests/reductions/test_reductions.py (plus 16 more across frame/arrays/extension/groupby/replace tests): - Datetime/timedelta reductions on empty or all-null columns now return the pd.NaT singleton instead of a unit-qualified numpy NaT (_get_nan_for_dtype), matching pandas identity semantics. - Empty/all-null reductions of pandas nullable extension dtypes return the dtype's NA instead of the result-dtype nan (e.g. Int64.mean() on an empty series is now <NA>, not float64 nan). - ColumnBase.any() implements Kleene logic for pandas nullable extension dtypes with skipna=False: a no-True result with nulls present is <NA>. Numpy dtypes keep NaN-sentinel-truthy behavior. - ColumnBase.any/all guard the reduce result against pd.NA before calling np.isnan to avoid ambiguous-bool errors. - Series.mode(dropna=False) sorts nulls first for numpy datetime/timedelta, DatetimeTZ and categorical dtypes (pandas sorts on the underlying representation where NaT/null codes sort first); arrow and nullable dtypes keep nulls last. - StringColumn.all() only short-circuits True for all-null columns; a partially-null column with skipna=False falls back so empty strings are evaluated as falsy. Removes 50 now-passing xfail entries from the pandas-testing plugin and documents the 4 remaining reductions failures as inherent (float32 GPU sum accumulation order; None-vs-NaN indistinguishability in object data).
github-actions
Bot
added
Python
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThe PR aligns nullable and datetime reduction behavior with pandas missing-value semantics, corrects string Reduction and missing-value alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/column.py (1)
1331-1337: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGate the all-null shortcut to nullable dtypes only
all(skipna=False)should mirror pandas here: returnTruefor numpy-backed all-null columns, and reserve_get_nan_for_dtype(self.dtype)for nullable extension dtypes. Add a regression test for an all-null float column.🤖 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/column.py` around lines 1331 - 1337, Update the all-null branch in Column.all so _get_nan_for_dtype(self.dtype) is used only for nullable extension dtypes; return True for numpy-backed all-null columns when skipna=False, while preserving the existing skipna=True behavior. Add a regression test covering all-null float columns.
🤖 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.
Outside diff comments:
In `@python/cudf/cudf/core/column/column.py`:
- Around line 1331-1337: Update the all-null branch in Column.all so
_get_nan_for_dtype(self.dtype) is used only for nullable extension dtypes;
return True for numpy-backed all-null columns when skipna=False, while
preserving the existing skipna=True behavior. Add a regression test covering
all-null float columns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bf131e1e-0f9f-4458-9c2c-6abe8951e290
📒 Files selected for processing (6)
python/cudf/cudf/core/column/column.pypython/cudf/cudf/core/column/string.pypython/cudf/cudf/core/series.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.pypython/cudf/cudf/tests/series/methods/test_reductions.pypython/cudf/cudf/utils/dtypes.py
galipremsagar
added
bug
Contributor
Author
|
/okay to test b5e18b1 |
Covers the behaviors fixed in the previous commit, asserting against live pandas results: - min/max on empty/all-null datetime and timedelta Series/Index return the pd.NaT singleton (identity with the pandas result). - mean/var/std/min/max on empty/all-null nullable dtypes return pd.NA. - Kleene any/all logic for nullable dtypes across the full data x skipna matrix, and the NaN-sentinel-truthy behavior of numpy float dtypes with skipna=False. - any/all on a nullable float column holding actual NaN values (exercises the pd.NA guard on the reduce result). - String all(skipna=False) on a partially-null column is not computed natively (raises NotImplementedError) while any(skipna=False) is truthy, matching the pandas-3 str-dtype NaN sentinel semantics. - mode(dropna=False) null position matches pandas across numpy datetime/timedelta, ordered/unordered categorical, nullable, float, and arrow timestamp/duration dtypes.
…nto pandas-tests-reductions-fixes
Contributor
Author
|
/okay to test c8565be |
Contributor
Author
|
/okay to test 51a6b4c |
mroeschke
approved these changes
Jul 20, 2026
Contributor
Author
|
/ok to test f089b92 |
Contributor
Author
|
/okay to test ac34ba1 |
galipremsagar
added
the
5 - Ready to Merge
Contributor
Author
|
/merge |
davidwendt
pushed a commit
to wjxiz1992/cudf
that referenced
this pull request
Jul 21, 2026
…VIDIA#23328) Running `tests/reductions/test_reductions.py` from the pandas test suite under `cudf.pandas` showed 38 failures. This PR fixes the 34 that are fixable in cudf classic and documents the 4 inherent ones. The same root causes also fixed 16 more pandas tests across `frame/`, `arrays/boolean`, `arrays/timedeltas`, `extension/test_arrow.py`, `groupby/test_reductions.py`, and `frame/methods/test_replace.py`. ### Fixes - **`pd.NaT` singleton for temporal null reductions** (`_get_nan_for_dtype`): min/max/median etc. on empty or all-null datetime/timedelta columns returned a unit-qualified `np.datetime64('NaT')`/`np.timedelta64('NaT')`; pandas returns the `pd.NaT` singleton and its tests assert identity (`result is NaT`). - **`<NA>` for empty/all-null reductions of nullable dtypes** (`ColumnBase._reduce`): `Series([], dtype="Int64").mean()`/`.var()` returned `np.float64(nan)` because the all-null branch used the *result* dtype (float64); pandas returns `pd.NA`. `sum`/`product` identities (0/1) are unchanged, matching pandas. - **Kleene logic for `any(skipna=False)`** (`ColumnBase.any`): with nulls present it returned `True` unconditionally. For pandas nullable extension dtypes, a no-True result with nulls present is now `<NA>` (matching `all`); numpy dtypes keep the NaN-sentinel-truthy behavior. - **`pd.NA` guard before `np.isnan`** in `any`/`all` to avoid "boolean value of NA is ambiguous" now that `_reduce` can return `pd.NA`. - **`Series.mode(dropna=False)` null position**: pandas sorts mode results on the underlying representation, so NaT (`INT64_MIN` as i8) and the categorical null code (-1) sort *first* while float NaN and arrow/nullable `<NA>` sort *last*. cudf sorted nulls last everywhere. Nulls-first now applies to numpy datetime/timedelta, `DatetimeTZDtype`, and categorical dtypes only (arrow timestamp/duration keep nulls last, verified against pandas). - **`StringColumn.all()`**: no longer short-circuits `True` for *partially*-null columns with `skipna=False` — the result depends on the truthiness of the non-null strings (e.g. `all([NaN, ""], skipna=False)` is `False`); it now falls through so `cudf.pandas` computes the correct result. All-null columns still return `True`. - `test_timedelta_reductions` updated to assert `pd.NaT` identity like `test_datetime_reductions` already does. ### Pandas-testing plugin - Removed **50** now-passing xfail entries. - Replaced the "TODO" reasons on the 4 remaining `test_reductions.py` entries with real ones: `test_sum_overflow_float[float32-*]` (GPU tree-reduction accumulates float32 in a different order than numpy pairwise summation) and `test_any_all_object_dtype_missing[any-data0/1]` (None-vs-np.nan distinction is lost when object data becomes a nulled bool column). ### Testing - `tests/reductions/` in CI mode: 498 passed, 10 xfailed, no strict-XPASS. - Full pandas-tests suite in CI mode (206k tests): no failures attributable to this change; every removed xfail entry verified as strict-XPASS solo (not just under xdist, to rule out GPU-contention fallback). - Classic cudf sweep (series/dataframe/indexes/groupby/reshape/text/dtypes/general_functions/window, ~71k tests): failure set identical to unmodified baseline (the only failures are pre-existing groupby-JIT ones). - Raw (plugin-less) before/after comparison over all affected pandas-test files confirmed 11 newly-passing tests and no newly-failing ones. The `decimal128` stack/unstack failures that appeared in some xdist runs reproduce identically on unmodified cudf (order-dependent, pre-existing). Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Matthew Roeschke (https://github.com/mroeschke) URL: NVIDIA#23328
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Running
tests/reductions/test_reductions.pyfrom the pandas test suite undercudf.pandasshowed 38 failures. This PR fixes the 34 that are fixable in cudf classic and documents the 4 inherent ones. The same root causes also fixed 16 more pandas tests acrossframe/,arrays/boolean,arrays/timedeltas,extension/test_arrow.py,groupby/test_reductions.py, andframe/methods/test_replace.py.Fixes
pd.NaTsingleton for temporal null reductions (_get_nan_for_dtype): min/max/median etc. on empty or all-null datetime/timedelta columns returned a unit-qualifiednp.datetime64('NaT')/np.timedelta64('NaT'); pandas returns thepd.NaTsingleton and its tests assert identity (result is NaT).<NA>for empty/all-null reductions of nullable dtypes (ColumnBase._reduce):Series([], dtype="Int64").mean()/.var()returnednp.float64(nan)because the all-null branch used the result dtype (float64); pandas returnspd.NA.sum/productidentities (0/1) are unchanged, matching pandas.any(skipna=False)(ColumnBase.any): with nulls present it returnedTrueunconditionally. For pandas nullable extension dtypes, a no-True result with nulls present is now<NA>(matchingall); numpy dtypes keep the NaN-sentinel-truthy behavior.pd.NAguard beforenp.isnaninany/allto avoid "boolean value of NA is ambiguous" now that_reducecan returnpd.NA.Series.mode(dropna=False)null position: pandas sorts mode results on the underlying representation, so NaT (INT64_MINas i8) and the categorical null code (-1) sort first while float NaN and arrow/nullable<NA>sort last. cudf sorted nulls last everywhere. Nulls-first now applies to numpy datetime/timedelta,DatetimeTZDtype, and categorical dtypes only (arrow timestamp/duration keep nulls last, verified against pandas).StringColumn.all(): no longer short-circuitsTruefor partially-null columns withskipna=False— the result depends on the truthiness of the non-null strings (e.g.all([NaN, ""], skipna=False)isFalse); it now falls through socudf.pandascomputes the correct result. All-null columns still returnTrue.test_timedelta_reductionsupdated to assertpd.NaTidentity liketest_datetime_reductionsalready does.Pandas-testing plugin
test_reductions.pyentries with real ones:test_sum_overflow_float[float32-*](GPU tree-reduction accumulates float32 in a different order than numpy pairwise summation) andtest_any_all_object_dtype_missing[any-data0/1](None-vs-np.nan distinction is lost when object data becomes a nulled bool column).Testing
tests/reductions/in CI mode: 498 passed, 10 xfailed, no strict-XPASS.decimal128stack/unstack failures that appeared in some xdist runs reproduce identically on unmodified cudf (order-dependent, pre-existing).