Skip to content

feat(operator): cut a numeric column into bins - #8481

Open
kz930 wants to merge 15 commits into
apache:mainfrom
kz930:feat/binning
Open

kz930 wants to merge 15 commits into
apache:mainfrom
kz930:feat/binning

Conversation

@kz930

@kz930 kz930 commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

An operator that cuts a numeric column into bins and adds the bin a row fell in as a column of its own.

Two cuts, because they answer different questions. Equal width divides the span into bins of one size; equal frequency cuts at the quantiles, so the bins hold about as many rows as each other. The second is why the operator reads the whole table before it emits: a quantile is not known until the last row has arrived.

The bin reads as its own range, (2.5, 5.0], which is a label to group by rather than a number to do arithmetic on.

Any related issues, documentation, discussions?

Not part of #8325: that set makes the operators Texera already had exportable. This adds a new one, which implements the trait from the start rather than gaining it. The trait is in #8327, which has merged.

Closes #8479, the task this change is the whole of.

The same gap as #8478, on the other kind of column. Grouping by a continuous number puts almost every row in a group of its own, so counts per age bracket or per price band could not be asked for; a user wanting them had to write a Python UDF.

How was this PR tested?

Ten unit tests in the operator's own spec, over the schema it declares and the Python it emits on both paths. The parity harness runs it on four configurations, one per cut plus a hostile column name and an empty cell, comparing the engine's answer against the exported script's.

The operator in the editor, cutting score into four equal-width bins:

config

Each row's bin beside the value it came from, named by the range it fell in rather than by a number:

result

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

Generated-by: Claude Code (Opus 5)

@github-actions

github-actions Bot commented Sep 10, 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.

@github-actions github-actions Bot added feature dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI common platform Non-amber Scala service paths labels Sep 10, 2026
@codecov-commenter

codecov-commenter commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.54545% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.80%. Comparing base (6db4a64) to head (ccf3205).

Files with missing lines Patch % Lines
.../texera/amber/operator/binning/BinningOpDesc.scala 93.75% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8481      +/-   ##
============================================
- Coverage     91.80%   91.80%   -0.01%     
- Complexity     5025     5041      +16     
============================================
  Files          1279     1281       +2     
  Lines         56557    56612      +55     
  Branches       7419     7423       +4     
============================================
+ Hits          51924    51973      +49     
- Misses         2768     2770       +2     
- Partials       1865     1869       +4     
Flag Coverage Δ *Carryforward flag
access-control-service 77.44% <ø> (ø)
agent-service 99.53% <ø> (ø) Carriedforward from a0238cf
amber 85.80% <94.54%> (+<0.01%) ⬆️
computing-unit-managing-service 64.05% <ø> (ø)
config-service 87.73% <ø> (ø)
file-service 82.31% <ø> (ø)
frontend 96.51% <ø> (ø)
notebook-migration-service 83.77% <ø> (ø)
pyamber 98.50% <ø> (ø) Carriedforward from a0238cf
workflow-compiling-service 78.30% <ø> (ø)

*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 Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 6db4a64 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 354 0.216 27,677/33,647/33,647 us 🟢 -7.9% / 🔴 +139.7%
🔴 bs=100 sw=10 sl=64 785 0.479 121,000/177,721/177,721 us 🔴 +10.5% / 🔴 +84.5%
🟢 bs=1000 sw=10 sl=64 969 0.591 1,040,659/1,061,185/1,061,185 us 🟢 -5.2% / 🔴 +19.1%
Baseline details

Latest main 6db4a64 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 354 tuples/sec 368 tuples/sec 880.16 tuples/sec -3.8% -59.8%
bs=10 sw=10 sl=64 MB/s 0.216 MB/s 0.224 MB/s 0.537 MB/s -3.6% -59.8%
bs=10 sw=10 sl=64 p50 27,677 us 26,701 us 11,544 us +3.7% +139.7%
bs=10 sw=10 sl=64 p95 33,647 us 36,545 us 14,186 us -7.9% +137.2%
bs=10 sw=10 sl=64 p99 33,647 us 36,545 us 17,122 us -7.9% +96.5%
bs=100 sw=10 sl=64 throughput 785 tuples/sec 794 tuples/sec 1,151 tuples/sec -1.1% -31.8%
bs=100 sw=10 sl=64 MB/s 0.479 MB/s 0.485 MB/s 0.703 MB/s -1.2% -31.8%
bs=100 sw=10 sl=64 p50 121,000 us 119,666 us 89,900 us +1.1% +34.6%
bs=100 sw=10 sl=64 p95 177,721 us 160,771 us 96,346 us +10.5% +84.5%
bs=100 sw=10 sl=64 p99 177,721 us 160,771 us 110,344 us +10.5% +61.1%
bs=1000 sw=10 sl=64 throughput 969 tuples/sec 966 tuples/sec 1,189 tuples/sec +0.3% -18.5%
bs=1000 sw=10 sl=64 MB/s 0.591 MB/s 0.59 MB/s 0.726 MB/s +0.2% -18.6%
bs=1000 sw=10 sl=64 p50 1,040,659 us 1,035,891 us 873,675 us +0.5% +19.1%
bs=1000 sw=10 sl=64 p95 1,061,185 us 1,119,225 us 918,440 us -5.2% +15.5%
bs=1000 sw=10 sl=64 p99 1,061,185 us 1,119,225 us 953,075 us -5.2% +11.3%
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,564.19,200,128000,354,0.216,27676.90,33646.92,33646.92
1,100,10,64,20,2546.38,2000,1280000,785,0.479,121000.19,177721.06,177721.06
2,1000,10,64,20,20637.85,20000,12800000,969,0.591,1040659.48,1061185.07,1061185.07

@carloea2 carloea2 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.

Additional boundary cases reproduced with compiled local tests.

The same gap as the timestamp parts, on the other kind of column. Grouping by a
continuous number puts almost every row in a group of its own, so counts per age
bracket or per price band could not be asked for; a user wanting them had to
write a Python UDF.

Two cuts, because they answer different questions. Equal width divides the span
into bins of one size; equal frequency cuts at the quantiles, so the bins hold
about as many rows as each other. The second is why the operator reads the whole
table before it emits: a quantile is not known until the last row has arrived.

The bin reads as its own range, "(2.5, 5.0]", which is a label to group by
rather than a number to do arithmetic on.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@github-actions github-actions Bot removed dependencies Pull requests that update a dependency file platform Non-amber Scala service paths labels Sep 10, 2026
An equal-width cut has no edges to find when every cell of the column is
empty, and none when no row arrived at all, so pandas raises before the
suffix that keeps a hole a hole can run. Both renderings now skip the cut
in that case and let the suffix turn the holes into the empty bin labels
they already are.

The equal-frequency cut does not raise on the same table, but it goes
through the same guard: one shape of answer for a table with nothing in
it, whichever way it was asked to cut.

Two tests run the exported script over an all-empty column, an empty
table and a column with one hole, since the defect is what pandas does at
runtime rather than what the operator writes.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@kz930

kz930 commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@carloea2 the finding is fixed: a column with nothing in it skips the cut, so the rows keep null bin labels. Would you take another look when you have a moment?

@carloea2 carloea2 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.

The all missing column case is handled now. Looks good.

…-integration

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

@carloea2 carloea2 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.

All 13 existing tests and formatting checks pass. A new parity test using the real table-operator lifecycle fails on stored NaNs in nullable Arrow input; the mixed-value control passes.

A nullable Float64 column read from Arrow can hold NaN as a value, which
notna() reports present, so an all-NaN column reached pd.cut and raised on
duplicate edges while the engine path left it uncut. The guard now tests a
float64 view. Adds a stored-NaN case to the runtime test.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@kz930

kz930 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2 The stored-NaN case is fixed in 7d0ebe5, with a parity test for it. Could you take another look?

@carloea2 carloea2 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.

The stored NaN fix passes. All 14 focused tests pass, including my original reproduction run through the real UDF lifecycle and generated export for both equal-width and equal-frequency binning. Mixed finite values agree; all stored NaNs produce empty bins. The UDF probe uses the #8766 SDK. Formatting passes. No new finding in this update.

sbt -batch 'WorkflowOperator/testOnly *BinningOpDescSpec -- -oDF' 'scalafmtCheckAll'

Python's open() defaults to the locale's encoding, which is cp1252 on
Windows, while the spec writes these files as UTF-8.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@carloea2 carloea2 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.

All 13 focused tests and formatting checks pass on a fresh detached checkout. The binning implementation is unchanged from the stored-NaN fix verified earlier; this update fixes the test driver's encoding. The remaining CI failure is LoopIntegrationSpec, covered by #8938. Looks good. Leaving the draft status unchanged.

kz930 added a commit to kz930/texera that referenced this pull request Oct 19, 2026

@carloea2 carloea2 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.

Binning fails on valid LONG values.

kz930 and others added 2 commits November 4, 2026 02:14
pandas builds edges in float64, which cannot count by one past 2**53, so
an equal-width cut raised on duplicate edges and a quantile cut merged
the rows into one bin. An integer column is now cut as its distance from
the value nearest zero, and that value is added back to the labels.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Cutting every integer column as a distance changed the labels pandas
rounds, so 723.062 could read 723.0625. The shifted cut is now used only
where the plain one raises or loses a bin.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@kz930

kz930 commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2 Large LONG values now keep separate bins in both paths, and ordinary columns keep pandas' labels. Could you take another look?

@carloea2 carloea2 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.

Equal-width binning still loses precision at the Int64 limits. The branch also conflicts with current main.

kz930 and others added 2 commits November 4, 2026 19:06
…e same bin

Comparing category counts missed Int64 values near the limit, where the plain
cut keeps all bins but puts every row in one.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
# Conflicts:
#	common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/LogicalOp.scala
@kz930

kz930 commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2 may you take another look?

@carloea2 carloea2 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.

Retested both generated paths at the positive and negative Int64 limits. The 52 Python cases and 13 Scala tests pass.

@kz930
kz930 marked this pull request as ready for review November 11, 2026 01:53

@carloea2 carloea2 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.

All 13 BinningOpDescSpec tests pass locally. The macOS frontend job failed on the registration modal test timing out after 20 seconds; 7,235 tests passed. @mengw15, could you rerun that job? https://github.com/apache/texera/actions/runs/42942174175/job/130266447647

@kz930

kz930 commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

The new run after ccf3205 is all green.

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

common feature frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cut a numeric column into bins

3 participants