Repository navigation
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8481 +/- ##
============================================
- Coverage 92.30% 92.30% -0.01%
- Complexity 5046 5064 +18
============================================
Files 1260 1261 +1
Lines 53887 53865 -22
Branches 6711 6707 -4
============================================
- Hits 49743 49718 -25
- Misses 2486 2488 +2
- Partials 1658 1659 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 475 | 0.29 | 19,792/28,899/28,899 us | 🔴 -8.8% / 🔴 +97.8% |
| 🟢 | bs=100 sw=10 sl=64 | 981 | 0.599 | 98,585/142,607/142,607 us | 🟢 -14.6% / 🔴 +42.3% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,160 | 0.708 | 855,001/930,764/930,764 us | ⚪ within ±5% / 🔴 -5.4% |
Baseline details
Latest main 7426431 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 475 tuples/sec | 520 tuples/sec | 912.36 tuples/sec | -8.7% | -47.9% |
| bs=10 sw=10 sl=64 | MB/s | 0.29 MB/s | 0.318 MB/s | 0.557 MB/s | -8.8% | -47.9% |
| bs=10 sw=10 sl=64 | p50 | 19,792 us | 20,013 us | 11,391 us | -1.1% | +73.7% |
| bs=10 sw=10 sl=64 | p95 | 28,899 us | 26,632 us | 14,608 us | +8.5% | +97.8% |
| bs=10 sw=10 sl=64 | p99 | 28,899 us | 26,632 us | 17,250 us | +8.5% | +67.5% |
| bs=100 sw=10 sl=64 | throughput | 981 tuples/sec | 994 tuples/sec | 1,161 tuples/sec | -1.3% | -15.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.599 MB/s | 0.607 MB/s | 0.709 MB/s | -1.3% | -15.5% |
| bs=100 sw=10 sl=64 | p50 | 98,585 us | 95,489 us | 91,471 us | +3.2% | +7.8% |
| bs=100 sw=10 sl=64 | p95 | 142,607 us | 167,041 us | 100,192 us | -14.6% | +42.3% |
| bs=100 sw=10 sl=64 | p99 | 142,607 us | 167,041 us | 119,565 us | -14.6% | +19.3% |
| bs=1000 sw=10 sl=64 | throughput | 1,160 tuples/sec | 1,172 tuples/sec | 1,226 tuples/sec | -1.0% | -5.4% |
| bs=1000 sw=10 sl=64 | MB/s | 0.708 MB/s | 0.716 MB/s | 0.748 MB/s | -1.1% | -5.4% |
| bs=1000 sw=10 sl=64 | p50 | 855,001 us | 848,493 us | 887,282 us | +0.8% | -3.6% |
| bs=1000 sw=10 sl=64 | p95 | 930,764 us | 905,453 us | 924,352 us | +2.8% | +0.7% |
| bs=1000 sw=10 sl=64 | p99 | 930,764 us | 905,453 us | 951,858 us | +2.8% | -2.2% |
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,421.02,200,128000,475,0.290,19791.59,28899.44,28899.44
1,100,10,64,20,2038.24,2000,1280000,981,0.599,98584.58,142607.22,142607.22
2,1000,10,64,20,17245.15,20000,12800000,1160,0.708,855001.32,930763.50,930763.50
carloea2
left a comment
There was a problem hiding this comment.
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
|
@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
left a comment
There was a problem hiding this comment.
The all missing column case is handled now. Looks good.
…-integration Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
carloea2
left a comment
There was a problem hiding this comment.
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 <noreply@anthropic.com>
carloea2
left a comment
There was a problem hiding this comment.
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 <noreply@anthropic.com>
carloea2
left a comment
There was a problem hiding this comment.
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.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
scoreinto four equal-width bins:Each row's bin beside the value it came from, named by the range it fell in rather than by a number:
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)