Skip to content

Support PEP 515 underscores in string-to-float astype - #24224

Open
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/string-float-underscores
Open

fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/string-float-underscores

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

What

  • Support PEP 515 underscores when casting strings to float dtypes.
  • Keep invalid underscore placements rejected.

Why

Integer casts already support this syntax, but float casts still reject valid values such as 1_2.3_4.

Implementation

  • Reuse the existing two-pass underscore cleanup for integer and floating-point casts.
  • Add float32 and float64 coverage for valid and invalid forms.

@fallintoplace
fallintoplace requested a review from a team as a code owner September 18, 2026 09:08
@fallintoplace
fallintoplace requested a review from vyasr September 18, 2026 09:08
@copy-pr-bot

copy-pr-bot Bot commented Sep 18, 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 the Python Affects Python cuDF API. label Sep 18, 2026
@coderabbitai

coderabbitai Bot commented Sep 18, 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: 37644c2d-86e0-4911-b090-7baa0c6a67ee

📥 Commits

Reviewing files that changed from the base of the PR and between 9a601e2 and 31eac4a.

📒 Files selected for processing (2)
  • python/cudf/cudf/core/column/string.py
  • python/cudf/cudf/tests/series/methods/test_astype.py

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Improved conversion of strings containing valid digit-separated underscores to integer and floating-point values.
    • Underscores in valid numeric positions are now handled consistently with pandas.
    • Invalid underscore placement continues to be rejected, with floating-point validation correctly identifying malformed values.
  • Tests

    • Added coverage for valid and invalid underscore placement during floating-point string conversion.

Walkthrough

The numeric string conversion path now accepts valid digit-separated underscores for integer and floating-point targets. Floating-point validation uses normalized strings. Tests cover valid and invalid floating-point underscore placement against pandas.

Changes

Numeric string casting

Layer / File(s) Summary
Normalize and validate numeric strings
python/cudf/cudf/core/column/string.py, python/cudf/cudf/tests/series/methods/test_astype.py
as_numerical_column removes valid digit-separated underscores before numeric conversion. Floating-point validation uses the normalized data. Tests cover valid decimal, exponent, and integer-like strings, plus invalid underscore placements, for float32 and float64.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 31eac

The float underscore conversion change has no remaining actionable issue from the reviewed behavior and is ready to merge.

🚥 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 describes the main change: support for PEP 515 underscores in string-to-float astype conversions.
Description check ✅ Passed The description directly explains the float-casting change, invalid underscore handling, implementation, and test coverage.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

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

@mroeschke

Copy link
Copy Markdown
Contributor

/ok to test 31eac4a

@mroeschke mroeschke added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants