Skip to content

Fix string reductions and isin across string dtype flavors - #23214

Merged
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
galipremsagar:string-flavor-reduction-isin-fixes
Jul 13, 2026
Merged

Fix string reductions and isin across string dtype flavors#23214
rapids-bot[bot] merged 4 commits into
NVIDIA:mainfrom
galipremsagar:string-flavor-reduction-isin-fixes

Conversation

@galipremsagar

Copy link
Copy Markdown
Contributor

Description

Fixes several string-dtype behaviors to match pandas across all string dtype flavors (object, the four pd.StringDtype storage/na_value combinations, and pd.ArrowDtype string types), plus a cudf.pandas proxy fix. These resolve 21 xfailed pandas unit tests in the pandas-testing plugin.

  • StringColumn.sum empty identity: summing an empty or all-null series now returns the additive identity — 0 for object dtype, "" for string dtypes — matching pandas (tests/arrays/string_/test_string.py::test_reduce_empty).
  • min/max NA sentinel identity: _get_nan_for_dtype returns the dtype's exact na_value singleton for pd.StringDtype and arrow string dtypes, so min/max(skipna=False) satisfies result is dtype.na_value (pd.NA for "string" dtypes and ArrowDtype, the np.nan float singleton for "str" dtypes). Other kind-"O" extension dtypes (categorical, arrow decimal/binary) intentionally keep the float NaN fallback, since pandas coerces their skew/cov/corr results to a float NaN.
  • isin across string dtype flavors: StringColumn._process_values_for_isin aligns string-typed values with the column's dtype (mirroring the numeric override), so isin matches on element values instead of returning all-False when the flavor differs.
  • cudf.pandas object-ndarray identity: _transform_arg returns an object-dtype ndarray as-is when no element needed transforming. Rebuilding an equivalent copy broke aliasing checks such as np.may_share_memory(np.asarray(x), x), which numpy.random.Generator.permutation uses to decide whether to defensively copy before an in-place shuffle — under pandas copy-on-write this raised ValueError: array is read-only.

Test coverage added:

  • Classic tests in tests/series/methods/test_reductions.py and tests/series/methods/test_isin.py, parametrized over object dtype, all four pd.StringDtype flavors, and pd.ArrowDtype(pa.string())/pa.large_string(), comparing against pandas wherever pandas does not raise or diverge (divergences are documented inline).
  • cudf.pandas tests: a _transform_arg identity unit test and an end-to-end Generator.permutation test validated against real pandas with the same seed.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

- StringColumn.sum on an empty or all-null series returns the additive
  identity: 0 for object dtype, "" for string dtypes, matching pandas.
- _get_nan_for_dtype returns the dtype's exact na_value singleton for
  pd.StringDtype and arrow string dtypes so reductions like
  min/max(skipna=False) satisfy `result is dtype.na_value`. Other
  kind-"O" extension dtypes (categorical, arrow decimal/binary) keep
  the float NaN fallback that pandas' skew/cov/corr coercion matches.
- StringColumn._process_values_for_isin aligns string-typed values with
  the column's dtype so isin matches element values across all string
  dtype flavors instead of returning all-False on flavor mismatch.
- cudf.pandas _transform_arg returns object-dtype ndarrays as-is when
  no element needed transforming, preserving buffer identity for
  aliasing checks such as np.may_share_memory; fixes
  "ValueError: array is read-only" from numpy Generator.permutation
  on string Series under pandas copy-on-write.
- Remove 21 pandas-testing plugin entries that now pass.
- Add classic tests covering object dtype, all four pd.StringDtype
  flavors, and pd.ArrowDtype string/large_string, plus cudf.pandas
  tests for the ndarray identity fix.
@galipremsagar
galipremsagar requested a review from a team as a code owner July 10, 2026 13:41
@galipremsagar
galipremsagar requested review from bdice and mroeschke July 10, 2026 13:41
@copy-pr-bot

copy-pr-bot Bot commented Jul 10, 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 Jul 10, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jul 10, 2026
@galipremsagar galipremsagar added bug Something isn't working non-breaking Non-breaking change labels Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review 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: 67b835ac-d55b-4e24-bd97-6e39bc2ac79a

📥 Commits

Reviewing files that changed from the base of the PR and between 6a4d6ae and c581ad2.

📒 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

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved string-column sum, min, and max behavior for empty and null-containing data.
    • Fixed isin comparisons across different string dtypes and null values.
    • Preserved NumPy object-array identity when no conversion is needed.
    • Improved handling of missing-value sentinels across string and nullable dtypes.
  • Tests

    • Added coverage for string reductions, isin, permutations, null handling, and object-array identity across supported storage formats.

Walkthrough

Updates nullable result handling, string reductions and isin dtype alignment, object-array proxy transformation, pandas compatibility mappings, and corresponding parametrized tests.

Changes

String dtype and proxy behavior alignment

Layer / File(s) Summary
Nullable result contracts
python/cudf/cudf/utils/dtypes.py, python/cudf/cudf/core/column/numerical_base.py, python/cudf/cudf/tests/series/methods/test_reductions.py
_get_nan_for_dtype now returns typed nullable sentinels for string and extension dtypes, numerical early returns no longer suppress the return type, and string reduction tests cover empty, null, and object-dtype cases.
String reduction and membership behavior
python/cudf/cudf/core/column/string.py, python/cudf/cudf/tests/series/methods/test_isin.py
Empty string reductions use dtype-specific identities, object-string isin values are cast to the left-hand dtype, and string dtype/null-handling tests cover these behaviors.
Proxy array identity preservation
python/cudf/cudf/pandas/fast_slow_proxy.py, python/cudf/cudf_pandas_tests/test_fast_slow_proxy.py, python/cudf/cudf_pandas_tests/test_cudf_pandas.py
Object arrays are returned unchanged when transformations preserve element identity, rebuilt when proxy elements require conversion, and validated through string permutation tests.
Pandas compatibility mappings
python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
Known-failure entries are removed for categorical replacement, string operations, permutation, reductions, and string copy-view cases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested labels: improvement

Suggested reviewers: wence-, mroeschke, brandon-b-miller, Matt711

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes to string reductions and isin behavior across string dtype flavors.
Description check ✅ Passed The description is directly aligned with the PR's string-dtype fixes, proxy behavior change, and added tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test 6a4d6ae

CI's full-suite run surfaced four strict XPASSes beyond the files
swept locally: the copy_view test_series_array_string_dtype
parametrizations pass due to the _transform_arg ndarray identity fix
(np.shares_memory(np.asarray(ser), ser.values) now holds), and
test_replace_categorical_ea_dtype_different_cats_raises passes due to
the isin dtype alignment fix.
@galipremsagar

Copy link
Copy Markdown
Contributor Author

/okay to test c581ad2

@galipremsagar

Copy link
Copy Markdown
Contributor Author

/merge

@galipremsagar galipremsagar added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Jul 13, 2026
@rapids-bot
rapids-bot Bot merged commit 3688829 into NVIDIA:main Jul 13, 2026
126 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working cudf.pandas Issues specific to cudf.pandas non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants