Skip to content

feat(visualization): drop rows with missing values before laying out a Dumbbell Plot - #7567

Merged
aglinxinyuan merged 2 commits into
apache:mainfrom
kz930:fix/7562-dumbbell-null-compared-column
Aug 14, 2026
Merged

feat(visualization): drop rows with missing values before laying out a Dumbbell Plot#7567
aglinxinyuan merged 2 commits into
apache:mainfrom
kz930:fix/7562-dumbbell-null-compared-column

Conversation

@kz930

@kz930 kz930 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Dumbbell Plot took the distinct values of the compared column and sorted them. An empty cell reaches the operator as None, so unique() returned a list with a None in it and sorted compared None against a string, ending the run with a TypeError.

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.

It now drops rows missing any of the three columns it reads, which is what the other visualization operators do: twenty-four of them open their generated Python with dropna(subset=[...]) #remove missing values, and Dumbbell Plot was the one that did not. A table left empty by the drop renders the operator's own error rather than an exception, matching how it already reports an empty input.

Any related issues, documentation, discussions?

Closes #7562

How was this PR tested?

DumbbellPlotOpDescSpec gains a case asserting the generated Python drops on all three configured columns before it sorts. It fails on the previous behavior, 7 passed / 1 failed before the change and 8 / 0 after.

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

Generated-by: Claude Code (Claude Opus 5)

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

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

  • Contributors with relevant context: @officialasishkumar, @aglinxinyuan
    You can notify them by mentioning @officialasishkumar, @aglinxinyuan in a comment.

@Yicong-Huang Yicong-Huang added the release/v1.2 back porting to release/v1.2 label Aug 11, 2026
@github-actions
github-actions Bot requested a review from xuang7 August 11, 2026 20:44
@github-actions

Copy link
Copy Markdown
Contributor

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch Analysis
release/v1.2 Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7.

Auto-label run.

@kz930
kz930 force-pushed the fix/7562-dumbbell-null-compared-column branch from 5f4b4b0 to ef32791 Compare August 11, 2026 20:46
…a Dumbbell Plot

Dumbbell Plot took the distinct values of the compared column and sorted them.
An empty cell reaches the operator as None, so `unique()` returned a list with a
None in it and `sorted` compared None against a string, ending the run with a
TypeError.

An empty value is ordinary input here. A blank CSV cell arrives as null:
univocity returns null for an empty field, and AttributeTypeUtils.parseField
passes it through by design.

It now drops rows missing any of the three columns it reads, which is what the
other visualization operators do: twenty-four of them open their generated
Python with `dropna(subset=[...]) #remove missing values`, and Dumbbell Plot was
the one that did not. A table left empty by the drop renders the operator's own
error rather than an exception, matching how it already reports an empty input.

The spec gains the case; it fails on the previous behavior.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Aug 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.13%. Comparing base (324278e) to head (150e471).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7567      +/-   ##
============================================
- Coverage     90.14%   90.13%   -0.02%     
+ Complexity     4417     4413       -4     
============================================
  Files          1174     1174              
  Lines         46964    46965       +1     
  Branches       5262     5262              
============================================
- Hits          42334    42330       -4     
- Misses         2876     2877       +1     
- Partials       1754     1758       +4     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 98.62% <ø> (ø) Carriedforward from d420e20
amber 86.71% <100.00%> (-0.03%) ⬇️
computing-unit-managing-service 72.46% <ø> (ø)
config-service 77.31% <ø> (ø)
file-service 68.90% <ø> (ø)
frontend 91.44% <ø> (ø) Carriedforward from d420e20
notebook-migration-service 78.89% <ø> (ø)
pyamber 97.57% <ø> (ø) Carriedforward from d420e20
workflow-compiling-service 57.89% <ø> (ø)

*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.

@kz930
kz930 force-pushed the fix/7562-dumbbell-null-compared-column branch from ef32791 to 83187c0 Compare August 11, 2026 20:51
@kz930 kz930 changed the title fix(visualization): drop rows with missing values before laying out a Dumbbell Plot feat(visualization): drop rows with missing values before laying out a Dumbbell Plot Aug 11, 2026
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 324278e 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 365 0.222 26,829/34,546/34,546 us 🔴 -9.8% / 🔴 +111.3%
🔴 bs=100 sw=10 sl=64 786 0.48 124,246/156,514/156,514 us 🟢 -21.7% / 🔴 +45.5%
🟢 bs=1000 sw=10 sl=64 923 0.563 1,080,012/1,155,567/1,155,567 us 🟢 -5.9% / 🔴 +11.5%
Baseline details

Latest main 324278e from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 365 tuples/sec 402 tuples/sec 774.89 tuples/sec -9.2% -52.9%
bs=10 sw=10 sl=64 MB/s 0.222 MB/s 0.246 MB/s 0.473 MB/s -9.8% -53.1%
bs=10 sw=10 sl=64 p50 26,829 us 25,554 us 12,738 us +5.0% +110.6%
bs=10 sw=10 sl=64 p95 34,546 us 32,476 us 16,348 us +6.4% +111.3%
bs=10 sw=10 sl=64 p99 34,546 us 32,476 us 18,848 us +6.4% +83.3%
bs=100 sw=10 sl=64 throughput 786 tuples/sec 795 tuples/sec 1,005 tuples/sec -1.1% -21.8%
bs=100 sw=10 sl=64 MB/s 0.48 MB/s 0.485 MB/s 0.613 MB/s -1.0% -21.7%
bs=100 sw=10 sl=64 p50 124,246 us 116,484 us 100,970 us +6.7% +23.1%
bs=100 sw=10 sl=64 p95 156,514 us 199,891 us 107,605 us -21.7% +45.5%
bs=100 sw=10 sl=64 p99 156,514 us 199,891 us 116,429 us -21.7% +34.4%
bs=1000 sw=10 sl=64 throughput 923 tuples/sec 932 tuples/sec 1,030 tuples/sec -1.0% -10.4%
bs=1000 sw=10 sl=64 MB/s 0.563 MB/s 0.569 MB/s 0.629 MB/s -1.1% -10.5%
bs=1000 sw=10 sl=64 p50 1,080,012 us 1,069,957 us 991,433 us +0.9% +8.9%
bs=1000 sw=10 sl=64 p95 1,155,567 us 1,228,108 us 1,036,668 us -5.9% +11.5%
bs=1000 sw=10 sl=64 p99 1,155,567 us 1,228,108 us 1,070,470 us -5.9% +7.9%
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,548.65,200,128000,365,0.222,26829.22,34546.15,34546.15
1,100,10,64,20,2544.64,2000,1280000,786,0.480,124246.23,156514.33,156514.33
2,1000,10,64,20,21667.06,20000,12800000,923,0.563,1080011.53,1155566.63,1155566.63

@xuang7
xuang7 removed their request for review August 11, 2026 21:04
@xuang7 xuang7 removed fix release/v1.2 back porting to release/v1.2 labels Aug 11, 2026
@kz930

kz930 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@aglinxinyuan May you take a look at this?

@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 13, 2026
@aglinxinyuan
aglinxinyuan removed this pull request from the merge queue due to a manual request Aug 13, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 13, 2026
@aglinxinyuan
aglinxinyuan enabled auto-merge August 14, 2026 08:02
@github-actions github-actions Bot added the fix label Aug 14, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 14, 2026
@aglinxinyuan
aglinxinyuan added this pull request to the merge queue Aug 14, 2026
Merged via the queue into apache:main with commit 490e57f Aug 14, 2026
31 checks passed
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.

Dumbbell Plot fails on an empty value in the compared column

5 participants