Skip to content

Preserve nulls in Arrow timestamp comparison results - #24079

Open
galipremsagar wants to merge 3 commits into
NVIDIA:mainfrom
galipremsagar:fix-arrow-timestamp-comparison-nulls
Open

Preserve nulls in Arrow timestamp comparison results#24079
galipremsagar wants to merge 3 commits into
NVIDIA:mainfrom
galipremsagar:fix-arrow-timestamp-comparison-nulls

Conversation

@galipremsagar

Copy link
Copy Markdown
Contributor

Description

Preserve nulls in nullable Arrow timestamp comparison results. DatetimeColumn currently fills every boolean comparison null with False (True for !=), applying NumPy NaT behavior to Arrow-backed results too. Use the same nullable-output guard already used for timedeltas.

  • Preserve existing NumPy datetime comparison semantics.
  • Expand Arrow timestamp/duration regression coverage across four units, six comparisons, and scalar/Series operands.
  • Re-enable all 24 affected pandas timestamp comparison cases: remove 21 xfails, three unconditional skips, and the original shard-only skip from NODEIDS_TO_SKIP_WHEN_SHARDED (Shard pandas tests across runners in PR and nightly CI #22992).

Independent main-based branch; no CPU fallback added.

Validation

  • Expanded direct datetimelike regression matrix: 96 passed.
  • Entire direct Series binary-operation suite: 4,799 passed, 108 skipped, 62 xfailed.
  • Full upstream Arrow module, shard 0: 12,534 passed, 1,040 skipped, 253 xfailed.
  • Full upstream Arrow module, shard 1: 12,581 passed, 1,039 skipped, 270 xfailed.
  • All 24 re-enabled timestamp comparisons genuinely pass; 72 timezone variants also pass.
  • All 24 timestamp comparison operations preserve nulls with fallback disabled.
  • All applicable pre-commit hooks passed, including mypy.

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

DatetimeColumn comparisons filled every null with False (True for ne), including nullable Arrow boolean outputs. Match the existing timedelta behavior by filling only non-nullable comparison outputs, preserving NumPy NaT semantics and Arrow null propagation.

Expand Arrow-backed datetimelike comparison coverage to timestamp and duration, all four units, six comparison operators, and scalar/Series operands. Remove 21 expected-failure markers and 3 unconditional skips across the 24 pandas timestamp comparison tests, plus the original shard-only skip.
@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: 89cf5688-f238-4c3a-9c83-2658bb5e4110

📥 Commits

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

📒 Files selected for processing (3)
  • python/cudf/cudf/core/column/datetime.py
  • python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
  • python/cudf/cudf/tests/series/test_binops.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; 1 remains after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Fixed datetime equality comparisons involving pandas nullable data types so missing values remain null instead of being converted to boolean results.
    • Improved compatibility for timestamp and duration comparisons across multiple time resolutions, including comparisons with both series and scalar values.
    • Standardized comparison behavior across all supported operators.

Walkthrough

The datetime comparison path now preserves nulls for pandas-nullable equality results. Tests cover Arrow timestamp and duration comparisons across units, operators, and operand forms. Obsolete timestamp test overrides were removed.

Changes

Datetime comparison behavior

Layer / File(s) Summary
Preserve nullable equality results
python/cudf/cudf/core/column/datetime.py
Datetime equality results preserve nulls for pandas-nullable extension dtypes. Other boolean results retain existing null filling.
Expand comparison coverage
python/cudf/cudf/tests/series/test_binops.py
Parameterized tests cover Arrow timestamp and duration types across supported units, comparison operators, and Series or scalar operands.
Remove obsolete test overrides
python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Removed timestamp comparison TODO failures, flaky markers, and a sharded-CI skip.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 86edf

Datetime comparisons now retain nulls for pandas-nullable results while preserving existing behavior for non-nullable outputs. The related compatibility coverage and test-status cleanup support merge readiness with no active material risk identified.

Suggested reviewers: vyasr, bdice

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. 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 concisely describes the main change: preserving nulls in Arrow timestamp comparison results.
Description check ✅ Passed The description directly explains the null-preservation change, expanded test coverage, re-enabled tests, and validation results.
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 bug Something isn't working non-breaking Non-breaking change labels Sep 9, 2026
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test ca73a96

@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 86edfc5

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

Labels

bug Something isn't working cudf.pandas Issues specific to cudf.pandas 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