Fix cudf.pandas series accessor failures across numpy/nullable/arrow dtypes - #23056
Merged
Conversation
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:
📝 WalkthroughWalkthroughThis PR updates cuDF categorical, datetime, timedelta, and accessor validation behavior to better match pandas dtype semantics and error messages. It also updates related tests and pandas-testing-plugin failure mappings. ChangesAccessor and datetime parity
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 1
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/series.py (1)
5176-5242: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStale docstring examples now show the wrong dtype (
int64) after theseconds/microseconds/nanosecondscasts toint32.The class docstring (lines ~5193, 5227, 5234, 5241) and the individual
seconds/microseconds/nanosecondsproperty docstrings (lines ~5311, 5318, 5353, 5388) still showdtype: int64for these attributes, but the new.astype(get_dtype_of_same_kind(self.series.dtype, np.dtype("int32")))casts make the actual outputint32. If these docstrings are exercised via doctest in CI, they will now fail; at minimum they are misleading documentation.📝 Proposed fix for stale doctest output
>>> seconds_series.dt.seconds 0 1 1 2 2 3 - dtype: int64 + dtype: int32 >>> series = cudf.Series([12231312123, 1231231231, 1123236768712, 2135656, ... 3244334234], dtype='timedelta64[ms]') ... >>> series.dt.seconds 0 48912 1 21631 2 36768 3 2135 4 47534 - dtype: int64 + dtype: int32 >>> series.dt.microseconds 0 123000 1 231000 2 712000 3 656000 4 234000 - dtype: int64 + dtype: int32 >>> s.dt.nanoseconds 0 0 1 0 2 0 3 0 4 0 - dtype: int64 + dtype: int32And in the
seconds,microseconds, andnanosecondsproperty docstrings respectively:>>> s.dt.seconds 0 48912 1 21631 2 36768 3 2135 4 47534 - dtype: int64 + dtype: int32 >>> s.dt.microseconds 0 123000 1 231000 2 712000 3 656000 4 234000 - dtype: int64 + dtype: int32>>> s.dt.microseconds 0 123000 1 231000 2 712000 3 656000 4 234000 - dtype: int64 + dtype: int32>>> s.dt.nanoseconds 0 123 1 231 2 712 3 656 4 234 - dtype: int64 + dtype: int32Also applies to: 5283-5323, 5325-5358, 5360-5393
🤖 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/series.py` around lines 5176 - 5242, The TimedeltaProperties docs are stale after the dtype cast change: the class docstring and the `seconds`, `microseconds`, and `nanoseconds` property docstrings still show `dtype: int64`, but the actual outputs from these accessors now use `int32`. Update the doctest examples in `TimedeltaProperties` and the individual property docstrings to match the new `int32` result so the documented output stays accurate and doctests continue to pass.
🤖 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/accessors/categorical.py`:
- Around line 93-101: The categorical codes dtype logic in the accessor should
match pandas at the boundary where the category count reaches 127. Update the
`CategoricalAccessor.codes` path so the cast uses
`min_signed_type(len(self._column.dtype.categories) + 1)` instead of the raw
category count, keeping `Series._from_column` aligned with pandas’ widening
behavior. Add or adjust a regression test around `cat.codes` for a 127-category
`Categorical` to verify the dtype is widened correctly.
---
Outside diff comments:
In `@python/cudf/cudf/core/series.py`:
- Around line 5176-5242: The TimedeltaProperties docs are stale after the dtype
cast change: the class docstring and the `seconds`, `microseconds`, and
`nanoseconds` property docstrings still show `dtype: int64`, but the actual
outputs from these accessors now use `int32`. Update the doctest examples in
`TimedeltaProperties` and the individual property docstrings to match the new
`int32` result so the documented output stays accurate and doctests continue to
pass.
🪄 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: 5589c791-1966-4018-99db-7f58f15136c1
📒 Files selected for processing (14)
python/cudf/cudf/core/accessors/categorical.pypython/cudf/cudf/core/accessors/lists.pypython/cudf/cudf/core/accessors/string.pypython/cudf/cudf/core/accessors/struct.pypython/cudf/cudf/core/column/datetime.pypython/cudf/cudf/core/index.pypython/cudf/cudf/core/series.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.pypython/cudf/cudf/tests/indexes/datetimeindex/test_attributes.pypython/cudf/cudf/tests/series/accessors/test_cat.pypython/cudf/cudf/tests/series/accessors/test_dt.pypython/cudf/cudf/tests/series/accessors/test_list.pypython/cudf/cudf/tests/series/accessors/test_str.pypython/cudf/cudf/tests/series/accessors/test_struct.py
💤 Files with no reviewable changes (1)
- python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Contributor
Author
|
/okay to test 0684181 |
Contributor
Author
|
/okay to test 136ad66 |
Contributor
Author
|
/okay to test 5ae8aa1 |
mroeschke
reviewed
Jul 2, 2026
Contributor
Author
|
/okay to test d388f10 |
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/series.py (1)
5271-5379: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the timedelta accessor doctests
seconds,microseconds, andnanosecondsstill showdtype: int64, but these accessors now returnint32and should match pandas. Please update the doctest outputs in all three examples.🤖 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/series.py` around lines 5271 - 5379, The timedelta accessor doctests in the Series.dt properties `seconds`, `microseconds`, and `nanoseconds` still expect `int64` output even though these accessors now return `int32`. Update the example outputs in each of these docstrings in `cudf/core/series.py` so the doctests match the actual returned dtype and stay aligned with pandas 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.
Outside diff comments:
In `@python/cudf/cudf/core/series.py`:
- Around line 5271-5379: The timedelta accessor doctests in the Series.dt
properties `seconds`, `microseconds`, and `nanoseconds` still expect `int64`
output even though these accessors now return `int32`. Update the example
outputs in each of these docstrings in `cudf/core/series.py` so the doctests
match the actual returned dtype and stay aligned with pandas behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 74037659-cd2a-4e75-9cee-52ebbeafe392
📒 Files selected for processing (7)
python/cudf/cudf/core/accessors/categorical.pypython/cudf/cudf/core/column/categorical.pypython/cudf/cudf/core/dtypes.pypython/cudf/cudf/core/mixins/__init__.pypython/cudf/cudf/core/mixins/no_new_attributes.pypython/cudf/cudf/core/series.pypython/cudf/cudf/tests/series/accessors/test_dt.py
🚧 Files skipped from review as they are similar to previous changes (1)
- python/cudf/cudf/tests/series/accessors/test_dt.py
mroeschke
approved these changes
Jul 13, 2026
Contributor
Author
|
/merge |
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
3 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
Fixes a batch of
cudf.pandasfailures intests/series/accessors(and the corresponding cudf-classic behavior) so the datetime/timedelta/categorical/string/list/struct accessors match pandas. Each fix is validated across the numpy, pandas-nullable, and pyarrow (ArrowDtype) dtype families.Datetime / timedelta component dtypes
Series.dt.dayofweek/day_of_week/weekdaynow returnint32for numpy-backed datetimes (previouslyint64) andint64[pyarrow]forArrowDtypetimestamps, matching pandas.Series.dt.seconds/microseconds/nanosecondsnow returnint32(previouslyint64);daysstaysint64, matching pandas.Series.dt.isocalendar()now returnsUInt32columns for numpy datetimes (previously numpyuint32); theArrowDtypepath staysint64[pyarrow].DatetimeIndex.microsecondmillisecond foldlibcudf's
MICROSECONDcomponent excludes the whole-millisecond part, but pandas folds milliseconds intomicrosecond(e.g.0.001002s -> 1002, not2).DatetimeIndex.microsecondnow addsmillisecond * 1000to match pandas andSeries.dt.microsecond.Timezone-aware rounding
DatetimeTZColumnrounding (ceil/floor/round) now operates on the local wall-clock time and re-localizes to the original timezone, matching pandas semantics (test_dt_round_tz)..dtaccessor is not writeableBaseDatelikePropertiesnow mimics pandas'NoNewAttributesMixin: once constructed, setting a new attribute on the.dtaccessor raisesAttributeError(test_dt_accessor_no_new_attributes).cat.codessigned dtypeSeries.cat.codesnow uses a signed integer dtype (int8/int16/...) matching pandas, rather than an unsigned one.Accessor error messages
The
.str,.list, and.structaccessor error messages now match pandas' wording..strmirrors pandas' inferred-type naming (integer/floating/boolean/datetime64/timedelta64).Tests
cudf.pandasxfail list (pandas-testing-plugin.py).test_dt.py,test_cat.py,test_str.py,test_list.py,test_struct.py,datetimeindex/test_attributes.py).Checklist