Skip to content

feat(visualization): export the charts drawn over a field or a sequence - #8504

Open
kz930 wants to merge 5 commits into
apache:mainfrom
kz930:feat/standalone-viz-domain-plots
Open

kz930 wants to merge 5 commits into
apache:mainfrom
kz930:feat/standalone-viz-domain-plots

Conversation

@kz930

@kz930 kz930 commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

Ten charts implement StandaloneCodeGenerator: Filled Area, Contour, Dumbbell, Time Series, Quiver, Choropleth Map, 3D Scatter, Volcano, Gantt and Carpet. What they share is the question they answer, which is how a value varies across a domain rather than where a row sits on an axis.

Each emits the plotly its executor emits and writes the page to the file the translator names for it, so a plan holding two charts writes two files rather than one overwriting the other. A chart that cannot be drawn writes the reason to that page rather than printing it: the engine's operator yields a page either way, so an export that printed instead would leave the run with no file where the workflow had one.

#8346 keeps the charts drawn on a coordinate system.

Any related issues, documentation, discussions?

Part of #8325, 17 of 27; that issue lists the set in order.

Two behaviour fixes travel with these charts, both on operators that moved here: the Filled Area Plot requires its line group once the switch that reads it is on, and the Contour Plot says what it cannot contour instead of ending the run. Closes #8283, closes #8076.

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

How was this PR tested?

Each chart asserts the block it emits in its own spec. Contour, Filled Area and Carpet additionally run the generated Python, since what they are worth checking is that the page comes out at all on the inputs that cannot be drawn: collinear points, a group with no line to draw, a table emptied by dropping nulls.

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

Generated-by: Claude Code (Claude Opus 5)

🤖 Generated with Claude Code

Ten charts implement the trait: Filled Area, Contour, Dumbbell, Time
Series, Quiver, Choropleth Map, 3D Scatter, Volcano, Gantt and Carpet.
What they share is the question they answer, which is how a value varies
across a domain rather than where a row sits on an axis.

Split out of the coordinate-system change on review, which had grown past
what one reading can hold.

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

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: @eugenegujing, @aglinxinyuan
    You can notify them by mentioning @eugenegujing, @aglinxinyuan in a comment.

@codecov-commenter

codecov-commenter commented Sep 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 17.02128% with 117 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.40%. Comparing base (4184858) to head (2a3edbb).

Files with missing lines Patch % Lines
...isualization/dumbbellPlot/DumbbellPlotOpDesc.scala 4.00% 24 Missing ⚠️
...isualization/timeSeriesplot/TimeSeriesOpDesc.scala 5.00% 19 Missing ⚠️
...lization/filledAreaPlot/FilledAreaPlotOpDesc.scala 6.66% 14 Missing ⚠️
...or/visualization/carpetPlot/CarpetPlotOpDesc.scala 7.14% 13 Missing ⚠️
...or/visualization/ganttChart/GanttChartOpDesc.scala 8.33% 11 Missing ⚠️
...lization/scatter3DChart/Scatter3dChartOpDesc.scala 8.33% 11 Missing ⚠️
...or/visualization/quiverPlot/QuiverPlotOpDesc.scala 10.00% 9 Missing ⚠️
.../visualization/volcanoPlot/VolcanoPlotOpDesc.scala 10.00% 9 Missing ⚠️
...ualization/choroplethMap/ChoroplethMapOpDesc.scala 14.28% 6 Missing ⚠️
.../visualization/contourPlot/ContourPlotOpDesc.scala 93.75% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (17.02%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8504      +/-   ##
============================================
- Coverage     92.60%   92.40%   -0.21%     
- Complexity     5046     5047       +1     
============================================
  Files          1258     1258              
  Lines         53663    53794     +131     
  Branches       6683     6698      +15     
============================================
+ Hits          49697    49706       +9     
- Misses         2312     2430     +118     
- Partials       1654     1658       +4     
Flag Coverage Δ *Carryforward flag
access-control-service 77.38% <ø> (ø)
agent-service 99.16% <ø> (ø) Carriedforward from ed724eb
amber 87.52% <17.02%> (-0.55%) ⬇️
computing-unit-managing-service 60.48% <ø> (ø)
config-service 87.37% <ø> (ø)
file-service 82.00% <ø> (ø)
frontend 96.59% <ø> (-0.02%) ⬇️ Carriedforward from ed724eb
notebook-migration-service 83.73% <ø> (ø)
pyamber 98.50% <ø> (ø) Carriedforward from ed724eb
workflow-compiling-service 72.43% <ø> (ø)

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

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

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

Compared against main 4184858 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 579 0.354 17,059/21,010/21,010 us 🟢 -7.7% / 🔴 +40.6%
🔴 bs=100 sw=10 sl=64 1,153 0.704 84,646/119,034/119,034 us 🔴 +27.5% / 🔴 +14.3%
⚪ bs=1000 sw=10 sl=64 1,347 0.822 741,676/809,234/809,234 us ⚪ within ±5% / 🟢 +22.6%
Baseline details

Latest main 4184858 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 579 tuples/sec 597 tuples/sec 822.11 tuples/sec -3.0% -29.6%
bs=10 sw=10 sl=64 MB/s 0.354 MB/s 0.364 MB/s 0.502 MB/s -2.7% -29.5%
bs=10 sw=10 sl=64 p50 17,059 us 16,699 us 12,129 us +2.2% +40.6%
bs=10 sw=10 sl=64 p95 21,010 us 22,755 us 15,469 us -7.7% +35.8%
bs=10 sw=10 sl=64 p99 21,010 us 22,755 us 18,357 us -7.7% +14.4%
bs=100 sw=10 sl=64 throughput 1,153 tuples/sec 1,206 tuples/sec 1,058 tuples/sec -4.4% +9.0%
bs=100 sw=10 sl=64 MB/s 0.704 MB/s 0.736 MB/s 0.646 MB/s -4.3% +9.0%
bs=100 sw=10 sl=64 p50 84,646 us 81,791 us 97,399 us +3.5% -13.1%
bs=100 sw=10 sl=64 p95 119,034 us 93,328 us 104,184 us +27.5% +14.3%
bs=100 sw=10 sl=64 p99 119,034 us 93,328 us 117,615 us +27.5% +1.2%
bs=1000 sw=10 sl=64 throughput 1,347 tuples/sec 1,331 tuples/sec 1,099 tuples/sec +1.2% +22.6%
bs=1000 sw=10 sl=64 MB/s 0.822 MB/s 0.812 MB/s 0.671 MB/s +1.2% +22.6%
bs=1000 sw=10 sl=64 p50 741,676 us 745,248 us 946,743 us -0.5% -21.7%
bs=1000 sw=10 sl=64 p95 809,234 us 836,653 us 985,437 us -3.3% -17.9%
bs=1000 sw=10 sl=64 p99 809,234 us 836,653 us 1,012,622 us -3.3% -20.1%
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,345.15,200,128000,579,0.354,17059.08,21009.51,21009.51
1,100,10,64,20,1734.69,2000,1280000,1153,0.704,84646.41,119033.93,119033.93
2,1000,10,64,20,14851.35,20000,12800000,1347,0.822,741675.78,809233.54,809233.54

@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 chart outputs use separate files, and contour errors no longer stop the workflow. Looks good.

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

Filled Area calculates tolerance differently in standalone mode. The native path uses len times 5 divided by 100. The standalone path divides len by 100 first. For 150 groups this gives 7 versus 5. Please use the same formula and add a boundary test.

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

Carpet Plot still differs on bad input. The native path returns an error page for missing columns or values that cannot be converted to numbers. Standalone calls dropna and astype first and crashes. Please keep the same error behavior and add both tests.

The exported block for the filled area plot rounded its tolerance the
other way round: five percent of the groups is (n * 5) // 100, not
(n // 100) * 5, and at 150 groups that is 7 rather than 5. The script
refused three tables the operator draws. The loop around it is now the
operator's own, down to the break that ends it.

The carpet plot dropped nulls before it looked for the columns to drop
them from, so a column that is not in the table reached pandas as a
KeyError and a value that is not a number reached astype as a
ValueError. Both ended the whole exported run, where the operator
answers each of them with a page saying what is wrong. The block now
checks in the operator's order and writes the same pages.

Both are checked by running the exported code rather than by reading
it: the filled area plot at the boundary the tolerance sets, 7 disjoint
groups of 150 drawn and 8 refused, and the carpet plot over a missing
column, a column of words, an empty table, a table of nulls, and a
table it can draw.

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

kz930 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Both are real and both are fixed: Filled Area now uses the operator's (len(grouped) * 5) // 100, and Carpet Plot checks the columns before it drops nulls and catches the conversion, so it writes the same pages the operator yields instead of raising. Both are covered by running the exported code, at the tolerance boundary (7 disjoint groups of 150 drawn, 8 refused) and over a missing column, a column of words, an empty table, a table of nulls and a table it can draw.

@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 Filled Area and Carpet changes address the earlier result and error handling differences. Looks good.

…-integration

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

@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 current delta only tags the Python tests; the chart implementations are unchanged from the previous review. I retested with the shared helpers from merged #8327, #8502, #8340 and #8344 supplied locally.

Command: sbt -batch "WorkflowOperator/testOnly *ContourPlotOpDescSpec *FilledAreaPlotOpDescSpec *CarpetPlotOpDescSpec -- -oDF" "scalafmtCheckAll". On Windows, 40 tests pass and two fail while reading the generated HTML. With PYTHONUTF8=1, all 42 tests and formatting pass.

The unmodified branch also fails compilation in CI on those missing shared helpers. Please refresh it from main before rerunning CI.

The module and the Plotly page are both UTF-8, but the Carpet Plot and
Filled Area drivers read them in the machine's default encoding, so on
Windows the page failed with a UnicodeDecodeError at byte 0x8f.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kz930

kz930 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@carloea2 The UTF-8 reads are fixed in 7b3e434, in both the Carpet Plot and Filled Area drivers. 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 encoding fix passes on Windows. With current upstream main merged locally, all 99 runnable tests across 11 suites and formatting checks pass. The one existing pending test is unchanged. The remaining CI failure occurs while downloading sbt, before compilation. Code looks good; CI still needs a successful run. Leaving the draft unchanged.

@kz930
kz930 marked this pull request as ready for review October 15, 2026 00:17

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

LGTM!

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

Projects

None yet

4 participants