Skip to content

Align fractional float casts to signed integers in the main API - #4037

Open
Lingikaushikreddy wants to merge 1 commit into
narwhals-dev:mainfrom
Lingikaushikreddy:lingikaushikreddy/fix-float-integer-casts
Open

Lingikaushikreddy wants to merge 1 commit into
narwhals-dev:mainfrom
Lingikaushikreddy:lingikaushikreddy/fix-float-integer-casts

Conversation

@Lingikaushikreddy

@Lingikaushikreddy Lingikaushikreddy commented Oct 6, 2026 •

Copy link
Copy Markdown

Description

Align fractional floating-point casts to signed integers across backends in the main API by truncating toward zero. For example, 1.9 becomes 1 and -1.9 becomes -1. Apply truncation only to floating-point source expressions, preserve safe overflow checks in Arrow, and retain the native behavior of the stable v1 and v2 APIs. Regression coverage includes series, full-frame casts, lazy expressions, grouped aggregates, and windows.

What type of PR is this?

  • 🐛 Bug Fix
  • ✅ Test
  • 📝 Documentation

Related issues

Closes #4019.

Validation

  • Cast tests across all ten constructors: 358 passed, 91 skipped, 11 expected failures.
  • Full suite: 16,549 passed, 340 skipped, 980 expected failures.
  • Prek hooks, Pyright, mypy, Pyrefly, 486 doctests, and strict documentation build passed.
  • The coverage gate did not pass: total coverage was 99.99%, with three uncovered unchanged lines in _duckdb/utils.py and tests/version_test.py. Changed measured files had 100% coverage.

AI assistance

  • No AI tools were used for this PR.
  • AI tools were used.

Tool/model: OpenAI Codex, GPT-6 family; the exact serving variant is not exposed in this session. AI assistance was used for investigation, implementation, regression tests, documentation, automated validation, code review, and this description. Human line-by-line review remains pending. Submitted through the user's browser at their explicit direction.

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes
  • First-contribution screenshot of the prescribed local pytest command attached

Summary by CodeRabbit

  • New Features
    • In the main namespace, casting floating-point values to signed integers now truncates toward zero across supported backends, including in expressions and window operations. Stable namespaces retain backend-native behavior.
  • Documentation
    • Clarified float-to-integer casting behavior and backend differences.

Local test evidence

The exact command in the first-contribution checklist passed with 16,549 passed, 340 skipped, and 980 expected failures (exit status 0, project default local coverage threshold 80%). This screenshot displays the actual saved local output; the separate 100% coverage gate remains unmet at 99.99%.

clipboard

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 123f5f51-e88f-497c-970b-d7c364fe6611
📥 Commits

Reviewing files that changed from the base of the PR and between 1acc082 and da67de7.

📒 Files selected for processing (10)
  • docs/backcompat.md
  • src/narwhals/_arrow/series.py
  • src/narwhals/_arrow/utils.py
  • src/narwhals/_duckdb/expr.py
  • src/narwhals/_ibis/expr.py
  • src/narwhals/_pandas_like/series.py
  • src/narwhals/_spark_like/expr.py
  • src/narwhals/expr.py
  • src/narwhals/series.py
  • tests/expr_and_series/cast_test.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The main namespace now truncates floating-point values toward zero before signed-integer casts in the updated backend paths. Stable namespaces retain backend-native behavior. Documentation and tests cover the cast behavior across backends.

Changes

Float-to-integer casts

Layer / File(s) Summary
Document cast behavior and update eager casts
docs/backcompat.md, src/narwhals/expr.py, src/narwhals/series.py, src/narwhals/_arrow/series.py, src/narwhals/_arrow/utils.py, src/narwhals/_pandas_like/series.py, tests/expr_and_series/cast_test.py
The documentation describes truncation toward zero in the main namespace and backend-native behavior in stable namespaces. Arrow and PyArrow-backed pandas series truncate float values before signed-integer casts in the main namespace. Tests cover values, nulls, overflow, stable APIs, Float16, and NaN.
Update expression cast paths
src/narwhals/_duckdb/expr.py, src/narwhals/_ibis/expr.py, src/narwhals/_spark_like/expr.py, tests/expr_and_series/cast_test.py
DuckDB, Ibis, and SQLFrame expression casts truncate float values toward zero before signed-integer casts in the main namespace. Tests cover windowed, grouped, and ungrouped aggregate casts.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: fbruzzesi

Fixed issue severity: <fixed_issue_severity>Medium</fixed_issue_severity>

Merge Risk: ⚪ Minimal · up to da67d

The change makes float-to-signed-integer casts truncate toward zero in the main namespace, documents the difference from the stable namespaces, and adds cross-backend tests. No merge-blocking risk was identified from the supplied evidence.

Security Architecture Review

Security architecture risk: 🔵 Low · up to da67d

The change is contained and version-gated, but affects public cast results across several execution paths. Stable APIs retain existing behavior, and no security-boundary regression was found in the reviewed paths. Effects on downstream applications remain outside the available evidence.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The conversion change can affect all values selected by matching main-namespace casts, including grouped and windowed outputs. The evidence bounds the implementation change to supplied series and expressions; it does not establish how downstream applications use those values in authorization or other sensitive decisions.

Trust Boundaries and Controls

  • observed — Public casts retain existing dtype validation. The inspected SQL-backend changes operate on native expression objects using fixed numeric operations; DuckDB uses the constant function name trunc. The changed blocks do not add interpolation of caller-controlled SQL text or a new authority-bearing sink.

Resilience and Maintainability Implications

  • observed — The eager conversion paths construct result objects rather than updating the source series. Truncation and final conversion complete before returning the new wrapper, so these changed blocks do not introduce a shared-state transition or partially committed source update.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 9 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR meets the coding requirements in #4019. The main API truncates floating-point inputs toward zero before signed-integer casts across the affected backends, including Arrow-backed pandas and Duck…
Out of Scope Changes check ✅ Passed The implementation, tests, and cast documentation all support the cross-backend casting fix in #4019. The available change summary shows no unrelated changes. A repository diff read failed because the…
Title check ✅ Passed The title clearly summarizes the main change: aligning fractional float-to-signed-integer casts in the main API.
Description check ✅ Passed The description explains the behavior change, identifies the bug-fix, test, and documentation categories, links the related issue, reports validation results, discloses the coverage-gate result and AI…
Full details: Docstring Coverage

Explanation

Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 9 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: casting a float column to a signed integer type truncates on pandas and Polars, raises on PyArrow, and rounds on DuckDB

1 participant