You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "kz930 (via GitHub)" <gi...@apache.org> on 2026/08/11 23:29:42 UTC

[PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

kz930 opened a new pull request, #7583:
URL: https://github.com/apache/texera/pull/7583

   ### What changes were proposed in this PR?
   
   Every Sklearn operator ended the execution when a cell it reads was empty, with an error raised by scikit-learn's own input validation rather than by the operator: `ValueError: Input X contains NaN.` Nothing in the family looked at missing values, so the message named `X`, a variable inside generated code, and pointed at neither the column nor the row. The training and classifier operators feed every column except the target into the estimator, so a blank in a column that has nothing to do with the model ended the run just the same.
   
   An empty value is ordinary input here. A blank CSV cell arrives as null, since univocity returns null for an empty field and `AttributeTypeUtils.parseField` passes it through by design. Skipping is what the rest of the codebase does with a value that is not there: twenty-four visualization operators open their generated Python with `dropna`, `COUNT(column)` counts only non-null rows, CONCAT and MIN pass over them, and `FilterPredicate` answers false for every condition but IS_NULL / IS_NOT_NULL. This family was the only one with no answer at all.
   
   What skipping means follows what each operator emits.
   
   | Operators | Output | Columns read | Change |
   | --- | --- | --- | --- |
   | `SklearnTrainingOpDesc` subclasses | one row holding the model | every column but the target | drop on the whole table, or on the text and target pair when Count Vectorizer is on |
   | `SklearnClassifierOpDesc` subclasses | one row holding the model | every column but the target, on both ports | the same statement serves both ports |
   | `SklearnTestingOpDesc` | one row per model, with metric columns | every column but the target | drop before scoring |
   | `SklearnAdvancedBaseDesc` subclasses | one row per parameter combination | the named features and the ground truth | drop on those columns only |
   | `SklearnPredictionOpDesc` | each input row, plus a result column | every column but the ground truth | keep the row, leave the result empty |
   
   The last row differs deliberately. Prediction adds a column to the user's rows, so dropping would take the user's row out of the output along with the value the model had nothing to say about. That is what the Hugging Face inference operators already do with a row they cannot process.
   
   Two of the drops name their columns rather than taking the whole table. With Count Vectorizer on, training and classifier read only the text and the target, and the advanced trainers read a named list of features, so a blank in a column they never touch should not cost the row.
   
   Fitting on fewer rows changes the model, the way COUNT over a column with nulls changes the count. That trade is one this codebase has already made everywhere else, and it beats ending the run.
   
   ### Any related issues, documentation, discussions?
   
   Closes #7582
   
   ### How was this PR tested?
   
   Each changed operator gained a case in its existing spec asserting the drop appears in the generated Python, next to the cases already asserting on that output: the two paths for training, the shared statement for the classifier, the scorer's drop, the advanced trainers' named-column drop, and prediction's kept row with an empty result. The full `WorkflowOperator` suite passes, 2192 tests.
   
   The behaviour itself was checked against the reproduction in the issue, a four-row CSV with one blank cell run through `CSV File Scan` into `Training: Bernoulli Naive Bayes`, which ended the execution before this change.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Claude Opus 5)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): answer an empty cell by what the estimator can do with it [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #7583:
URL: https://github.com/apache/texera/pull/7583


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5407037381

   /request-review @aglinxinyuan


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): answer an empty cell by what the estimator can do with it [texera]

Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5458578472

   @aglinxinyuan May you take a look at it please? Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): answer an empty cell by what the estimator can do with it [texera]

Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5458409137

   scikit-learn was bumped from 1.5.0 to 1.7.2 in #6184, and `amber/operator-requirements.txt` on main pins 1.7.2, where both ExtraTreeClassifier and ExtraTreesClassifier fit on a NaN. On the second point, the prediction now goes out uncast in ef15017f2, so the declared type of the output column is the only thing naming that column's type.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5260052761

   <!-- texera-reviewer-suggestion -->
   ### Automated Reviewer Suggestions
   
   Based on the `git blame` history of the changed files, we recommend the following reviewers:
   
   - **Contributors with relevant context:** `@aglinxinyuan`, `@Ma77Ball`, `@carloea2`
     You can notify them by mentioning `@aglinxinyuan`, `@Ma77Ball`, `@carloea2` in a comment.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5391738047

   @aglinxinyuan May you take a look at it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5260379852

   @aglinxinyuan @carloea2 May you take a look at it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5260071606

   ## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7583?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 87.87%. Comparing base ([`cb6e5c6`](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`17e5144`](https://app.codecov.io/gh/apache/texera/commit/17e5144118d0ccb49fe99810d91c2b56eb6498d5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               main    #7583      +/-   ##
   ============================================
   - Coverage     87.87%   87.87%   -0.01%     
     Complexity     4275     4275              
   ============================================
     Files          1176     1176              
     Lines         46890    46889       -1     
     Branches       5228     5228              
   ============================================
   - Hits          41205    41204       -1     
     Misses         3961     3961              
     Partials       1724     1724              
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
   |---|---|---|---|
   | [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [agent-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `98.62% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [amber](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.87% <ø> (-0.01%)` | :arrow_down: | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `60.38% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [config-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.97% <ø> (ø)` | | |
   | [file-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.05% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [frontend](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `89.95% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [pyamber](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.47% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7583/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.31% <ø> (ø)` | | Carriedforward from [cb6e5c6](https://app.codecov.io/gh/apache/texera/commit/cb6e5c649978215188ea92937ec583be9990c49e?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   *This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7583?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(sklearn): skip rows with missing values instead of ending the run [texera]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7583:
URL: https://github.com/apache/texera/pull/7583#issuecomment-5260101895

   <!-- texera-benchmarks-comment -->
   ## ⚠️ Benchmark changes need a look
   
   🟢 2 better · 🔴 8 worse · ⚪ 5 noise (<±5%) · 0 without baseline
   
   > Compared against main `cb6e5c6` 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](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/31546679860)
   
   | | config | throughput | MB/s | latency | max Δ latest / 7d |
   |---|---|---:|---:|---:|---:|
   | 🔴 | bs=10 sw=10 sl=64 | 372 | 0.227 | 26,601/33,399/33,399 us | 🔴 +24.3% / 🔴 +111.3% |
   | 🔴 | bs=100 sw=10 sl=64 | 774 | 0.472 | 126,620/167,794/167,794 us | 🔴 +9.7% / 🔴 +55.3% |
   | ⚪ | bs=1000 sw=10 sl=64 | 905 | 0.553 | 1,108,295/1,170,084/1,170,084 us | ⚪ within ±5% / 🔴 +12.7% |
   
   <details><summary>Baseline details</summary>
   
   Latest main `cb6e5c6` from same runner
   
   | config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
   |---|---|---:|---:|---:|---:|---:|
   | bs=10 sw=10 sl=64 | throughput | 372 tuples/sec | 417 tuples/sec | 784.3 tuples/sec | -10.8% | -52.6% |
   | bs=10 sw=10 sl=64 | MB/s | 0.227 MB/s | 0.254 MB/s | 0.479 MB/s | -10.6% | -52.6% |
   | bs=10 sw=10 sl=64 | p50 | 26,601 us | 21,393 us | 12,590 us | +24.3% | +111.3% |
   | bs=10 sw=10 sl=64 | p95 | 33,399 us | 35,349 us | 15,991 us | -5.5% | +108.9% |
   | bs=10 sw=10 sl=64 | p99 | 33,399 us | 35,349 us | 18,694 us | -5.5% | +78.7% |
   | bs=100 sw=10 sl=64 | throughput | 774 tuples/sec | 816 tuples/sec | 1,002 tuples/sec | -5.1% | -22.8% |
   | bs=100 sw=10 sl=64 | MB/s | 0.472 MB/s | 0.498 MB/s | 0.612 MB/s | -5.2% | -22.8% |
   | bs=100 sw=10 sl=64 | p50 | 126,620 us | 120,582 us | 101,285 us | +5.0% | +25.0% |
   | bs=100 sw=10 sl=64 | p95 | 167,794 us | 152,935 us | 108,068 us | +9.7% | +55.3% |
   | bs=100 sw=10 sl=64 | p99 | 167,794 us | 152,935 us | 118,235 us | +9.7% | +41.9% |
   | bs=1000 sw=10 sl=64 | throughput | 905 tuples/sec | 918 tuples/sec | 1,030 tuples/sec | -1.4% | -12.1% |
   | bs=1000 sw=10 sl=64 | MB/s | 0.553 MB/s | 0.56 MB/s | 0.629 MB/s | -1.3% | -12.0% |
   | bs=1000 sw=10 sl=64 | p50 | 1,108,295 us | 1,090,152 us | 991,882 us | +1.7% | +11.7% |
   | bs=1000 sw=10 sl=64 | p95 | 1,170,084 us | 1,151,417 us | 1,038,496 us | +1.6% | +12.7% |
   | bs=1000 sw=10 sl=64 | p99 | 1,170,084 us | 1,151,417 us | 1,068,265 us | +1.6% | +9.5% |
   
   </details>
   
   <details><summary>Raw CSV</summary>
   
   ```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,538.16,200,128000,372,0.227,26600.94,33398.87,33398.87
   1,100,10,64,20,2584.91,2000,1280000,774,0.472,126619.65,167793.75,167793.75
   2,1000,10,64,20,22093.25,20000,12800000,905,0.553,1108295.44,1170083.88,1170083.88
   ```
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org