Skip to content

fix(pyamber): coerce numpy scalar types (np.integer / np.bool_) in cast_to_schema#6278

Open
eugenegujing wants to merge 3 commits into
apache:mainfrom
eugenegujing:fix/numpy-scalar-coercion
Open

fix(pyamber): coerce numpy scalar types (np.integer / np.bool_) in cast_to_schema#6278
eugenegujing wants to merge 3 commits into
apache:mainfrom
eugenegujing:fix/numpy-scalar-coercion

Conversation

@eugenegujing

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Pandas-based Python operators naturally produce numpy scalar types at the Tuple output boundary, and the Python worker rejects them:

  • df["x"].sum() / .max() / .count() return numpy.int64
  • df["x"].any() or any numpy comparison returns numpy.bool_

On output, Tuple.finalize(schema) runs cast_to_schema() then a strict isinstance(value, expected_python_type) check in validate_schema(). Because numpy's integer and bool scalar types are not subclasses of Python int / bool (isinstance(np.int64(5), int) and isinstance(np.bool_(True), bool) are both False), the check fails and the operator crashes with TypeError: Unmatched type for field ..., even though the value is semantically correct (np.int64(5) is exactly the integer 5; the field is declared INTEGER).

This is a direct follow-up to #6053, which added float → int coercion for INT/LONG fields. That fix only handled np.float64 (which happens to subclass Python float); numpy integer and bool scalars were still rejected. This PR extends the same mechanism in cast_to_schema() to cover them:

  • INT/LONG fields: accept numpy.integer scalars by normalizing them (and integral floats) into a candidate int, then applying the existing INTEGRAL_TYPE_RANGES bounds check once. int() is exact for numpy integers, so there is no precision loss. Out-of-range values are left unchanged so validate_schema() still fails loudly (no int32 overflow / silent corruption).
  • BOOL fields: convert numpy.bool_ to Python bool in a separate branch gated on the BOOL target type, so bool and int never cross-coerce (np.bool_ is not numpy.integer, and a plain Python bool — an int subclass — is never coerced into an integer field).

validate_schema() is unchanged; it remains the strict backstop, so any type this PR does not explicitly coerce still fails loudly rather than passing silently.

Out of scope: np.float32 is not a Python float subclass, so an integral np.float32 into an INT field is still rejected. It is intentionally left out of this PR to keep the change focused; it can be folded into the same float-normalization path in a follow-up if desired.

Any related issues, documentation, discussions?

How was this PR tested?

Added unit tests to amber/src/test/python/core/models/test_tuple.py.

Results:

# Python unit tests
pytest src/test/python/core/models/test_tuple.py -q      ->  84 passed
ruff check / ruff format --check                          ->  clean

# Backend suite
sbt scalafmtCheckAll                                      ->  exit 0
sbt "scalafixAll --check"                                 ->  exit 0
sbt test                                                ->  1717 passed, 0 failed

Adversarial check: reverting cast_to_schema() to the pre-fix version turns exactly the coercion cases red while the guard cases stay green; restoring the fix returns to all-green. Every pass-case asserts the concrete Python type, so a test cannot pass without the fix.

Also reproduced end-to-end in the platform: CSV File ScanPython UDF (yield {"total_age": table["age"].sum()} / {"has_senior": (table["age"] > 60).any()}) crashed with Unmatched type ... numpy.int64 / numpy.bool before the fix and runs correctly after.

Was this PR authored or co-authored using generative AI tooling?

Co-authored by: Claude Code (Claude Fable 5)

…st_to_schema

Pandas-based Python operators naturally produce numpy scalars:
df["x"].sum() returns np.int64 and (df["x"] > n).any() returns np.bool_. Neither subclasses Python int/bool, so Tuple.finalize() failed the strict isinstance check in validate_schema() with "TypeError: Unmatched type", crashing the operator even though the values are semantically correct.

Extend the coercion in cast_to_schema(), mirroring the integral-float fix from apache#6053:
- INT/LONG fields: accept np.integer scalars by normalizing them (and integral floats) into a candidate int, then applying the existing INTEGRAL_TYPE_RANGES bounds check once; out-of-range values are left unchanged so validation still fails loudly.

- BOOL fields: convert np.bool_ to Python bool in a separate branch gated on the target type, so bool and int never cross-coerce.

validate_schema() is unchanged. np.float32 (not a Python float subclass) is intentionally out of scope; noted in the PR description.
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • No candidates found from git blame history.

@codecov-commenter

codecov-commenter commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.65%. Comparing base (a6c85b3) to head (7e30137).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6278   +/-   ##
=========================================
  Coverage     60.64%   60.65%           
  Complexity     3368     3368           
=========================================
  Files          1133     1133           
  Lines         44141    44147    +6     
  Branches       4825     4825           
=========================================
+ Hits          26771    26777    +6     
  Misses        15908    15908           
  Partials       1462     1462           
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø) Carriedforward from 0c2d4ad
agent-service 44.59% <ø> (ø) Carriedforward from 0c2d4ad
amber 66.13% <ø> (ø) Carriedforward from 0c2d4ad
computing-unit-managing-service 0.00% <ø> (ø) Carriedforward from 0c2d4ad
config-service 52.30% <ø> (ø) Carriedforward from 0c2d4ad
file-service 63.97% <ø> (ø) Carriedforward from 0c2d4ad
frontend 52.40% <ø> (ø) Carriedforward from 0c2d4ad
notebook-migration-service 78.57% <ø> (ø) Carriedforward from 0c2d4ad
pyamber 91.19% <100.00%> (+0.01%) ⬆️
workflow-compiling-service 55.14% <ø> (ø) Carriedforward from 0c2d4ad

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 0 better · 🔴 10 worse · ⚪ 5 noise (<±5%) · 0 without baseline

Compared against main 18ccda9 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 373 0.227 26,703/36,201/36,201 us 🔴 +19.1% / 🔴 +144.7%
🔴 bs=100 sw=10 sl=64 778 0.475 129,210/160,638/160,638 us 🔴 +9.0% / 🔴 +55.7%
🔴 bs=1000 sw=10 sl=64 916 0.559 1,094,284/1,165,958/1,165,958 us 🔴 +5.6% / 🔴 +17.5%
Baseline details

Latest main 18ccda9 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 373 tuples/sec 418 tuples/sec 814.14 tuples/sec -10.8% -54.2%
bs=10 sw=10 sl=64 MB/s 0.227 MB/s 0.255 MB/s 0.497 MB/s -11.0% -54.3%
bs=10 sw=10 sl=64 p50 26,703 us 22,427 us 12,120 us +19.1% +120.3%
bs=10 sw=10 sl=64 p95 36,201 us 34,986 us 14,792 us +3.5% +144.7%
bs=10 sw=10 sl=64 p99 36,201 us 34,986 us 18,232 us +3.5% +98.6%
bs=100 sw=10 sl=64 throughput 778 tuples/sec 836 tuples/sec 1,050 tuples/sec -6.9% -25.9%
bs=100 sw=10 sl=64 MB/s 0.475 MB/s 0.51 MB/s 0.641 MB/s -6.9% -25.9%
bs=100 sw=10 sl=64 p50 129,210 us 119,350 us 96,227 us +8.3% +34.3%
bs=100 sw=10 sl=64 p95 160,638 us 147,332 us 103,174 us +9.0% +55.7%
bs=100 sw=10 sl=64 p99 160,638 us 147,332 us 111,033 us +9.0% +44.7%
bs=1000 sw=10 sl=64 throughput 916 tuples/sec 941 tuples/sec 1,072 tuples/sec -2.7% -14.6%
bs=1000 sw=10 sl=64 MB/s 0.559 MB/s 0.575 MB/s 0.655 MB/s -2.8% -14.6%
bs=1000 sw=10 sl=64 p50 1,094,284 us 1,058,955 us 950,645 us +3.3% +15.1%
bs=1000 sw=10 sl=64 p95 1,165,958 us 1,104,567 us 991,936 us +5.6% +17.5%
bs=1000 sw=10 sl=64 p99 1,165,958 us 1,104,567 us 1,022,498 us +5.6% +14.0%
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,536.87,200,128000,373,0.227,26703.29,36200.69,36200.69
1,100,10,64,20,2569.79,2000,1280000,778,0.475,129210.24,160637.52,160637.52
2,1000,10,64,20,21837.37,20000,12800000,916,0.559,1094283.76,1165958.03,1165958.03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR targets Python worker schema finalization for pandas-derived outputs by extending Tuple.cast_to_schema() to normalize numpy scalar types (notably numpy.integer and numpy.bool_) into Python builtins so Tuple.validate_schema() can remain strict while common pandas UDF outputs no longer crash.

Changes:

  • Extend cast_to_schema() to coerce numpy.integer into int for INT/LONG fields and numpy.bool_ into bool for BOOL fields.
  • Add unit tests covering numpy scalar coercion and guardrails (rejecting cross bool↔int coercion and out-of-range cases).
  • Update docstring/comments to reflect the expanded coercion behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
amber/src/main/python/core/models/tuple.py Adds numpy scalar coercion branches inside cast_to_schema() prior to strict schema validation.
amber/src/test/python/core/models/test_tuple.py Adds regression/unit tests for numpy integer/bool coercion and boundary behaviors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread amber/src/main/python/core/models/tuple.py Outdated
Comment thread amber/src/test/python/core/models/test_tuple.py Outdated
Comment thread amber/src/main/python/core/models/tuple.py Outdated
eugenegujing and others added 2 commits July 8, 2026 20:36
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Eugene Gu <eugenegujing@outlook.com>
…float64 window

Follow-up to a PR review finding. cast_to_schema() range-checked both integral floats and numpy integers against INTEGRAL_TYPE_RANGES, whose LONG bound (2**53-1) exists only because float64 cannot exactly represent integers above 2**53. numpy integers are exact, so that cap wrongly rejected valid int64 values in (2**53, 2**63) — e.g. database/snowflake IDs and large sums from pandas reductions — while the same value as a plain Python int is accepted.

Add NUMPY_INTEGRAL_RANGES (INT -> int32, LONG -> int64) for the numpy.integer path; integral floats keep the 2**53 window. Tests updated accordingly.
@xuang7 xuang7 added release/v1.2 back porting to release/v1.2 and removed release/v1.2 back porting to release/v1.2 labels Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python worker crashes with "Unmatched type" when a field value is a numpy scalar (np.int64 / np.bool_)

4 participants