Repository navigation
feat(workflow-operator): volcano plot labels follow columns - #9040
eugenegujing wants to merge 7 commits into
Conversation
Name the y column and its axis -log10(<p-value column>) instead of the fixed "-log10(pvalue)". Add optional Title, X Label, Y Label and Color Bar Label properties that override the column-derived text when set; an empty or blank field keeps the default.
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9040 +/- ##
============================================
- Coverage 91.85% 91.72% -0.14%
- Complexity 5020 5033 +13
============================================
Files 1279 1269 -10
Lines 56192 55782 -410
Branches 7320 7202 -118
============================================
- Hits 51615 51164 -451
- Misses 2741 2778 +37
- Partials 1836 1840 +4
*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 | 349 | 0.213 | 24,988/45,295/45,295 us | 🔴 +59.6% / 🔴 +211.8% |
| 🔴 | bs=100 sw=10 sl=64 | 827 | 0.505 | 119,105/146,882/146,882 us | 🔴 +6.7% / 🔴 +52.7% |
| 🟢 | bs=1000 sw=10 sl=64 | 973 | 0.594 | 1,024,334/1,114,589/1,114,589 us | 🟢 -8.1% / 🔴 +23.1% |
Baseline details
Latest main d6c9050 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 349 tuples/sec | 465 tuples/sec | 901.09 tuples/sec | -24.9% | -61.3% |
| bs=10 sw=10 sl=64 | MB/s | 0.213 MB/s | 0.284 MB/s | 0.55 MB/s | -25.0% | -61.3% |
| bs=10 sw=10 sl=64 | p50 | 24,988 us | 20,328 us | 11,350 us | +22.9% | +120.2% |
| bs=10 sw=10 sl=64 | p95 | 45,295 us | 28,372 us | 14,525 us | +59.6% | +211.8% |
| bs=10 sw=10 sl=64 | p99 | 45,295 us | 28,372 us | 17,637 us | +59.6% | +156.8% |
| bs=100 sw=10 sl=64 | throughput | 827 tuples/sec | 874 tuples/sec | 1,205 tuples/sec | -5.4% | -31.3% |
| bs=100 sw=10 sl=64 | MB/s | 0.505 MB/s | 0.533 MB/s | 0.735 MB/s | -5.3% | -31.3% |
| bs=100 sw=10 sl=64 | p50 | 119,105 us | 111,652 us | 88,882 us | +6.7% | +34.0% |
| bs=100 sw=10 sl=64 | p95 | 146,882 us | 146,120 us | 96,205 us | +0.5% | +52.7% |
| bs=100 sw=10 sl=64 | p99 | 146,882 us | 146,120 us | 120,754 us | +0.5% | +21.6% |
| bs=1000 sw=10 sl=64 | throughput | 973 tuples/sec | 958 tuples/sec | 1,245 tuples/sec | +1.6% | -21.8% |
| bs=1000 sw=10 sl=64 | MB/s | 0.594 MB/s | 0.585 MB/s | 0.76 MB/s | +1.5% | -21.8% |
| bs=1000 sw=10 sl=64 | p50 | 1,024,334 us | 1,023,482 us | 862,870 us | +0.1% | +18.7% |
| bs=1000 sw=10 sl=64 | p95 | 1,114,589 us | 1,212,500 us | 905,485 us | -8.1% | +23.1% |
| bs=1000 sw=10 sl=64 | p99 | 1,114,589 us | 1,212,500 us | 948,588 us | -8.1% | +17.5% |
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,572.88,200,128000,349,0.213,24988.31,45294.63,45294.63
1,100,10,64,20,2417.41,2000,1280000,827,0.505,119105.14,146881.78,146881.78
2,1000,10,64,20,20547.69,20000,12800000,973,0.594,1024334.26,1114588.79,1114588.79
kz930
left a comment
There was a problem hiding this comment.
The color bar always shows the effect column, which is the same column as the x axis, so is a separate Color Bar Label needed, or could it follow X Label?
…label Drop the Color Bar Label property; the points are colored by the effect column that is on the x axis, so the color bar now carries the X Label text in both the engine template and the exported script.
…is-labels # Conflicts: # common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/volcanoPlot/VolcanoPlotOpDesc.scala
Remove the cross-path label test and the export saved-workflow test, fold the engine color bar check into the label test, compare the engine whitespace and saved-workflow tests against the default operator, and file the two required-field tests back under generatePythonCode.
What changes were proposed in this PR?
Root cause:
VolcanoPlotOpDesc.generatePythonCodehard-codes the y column as the literal"-log10(pvalue)"and plots it under that name, so the y axis always says-log10(pvalue)even when the selected column ispadj. The title is the literal"Volcano Plot"and the x axis and color bar show the raw effect column name. None of this can be changed from the property panel.The y column is now named after the selected p-value column, so the axis and the hover text say which column was transformed. Three optional label properties are added, empty by default:
Volcano Plot-log10(<p-value column>)The color bar has no label of its own: the points are colored by the effect column, the same column that is on the x axis, so the color bar carries the X Label text (or the effect column name when X Label is empty). If a later change lets the color encode a different column, a label for it would come with that change.
The labels go through
fig.update_layout(xaxis_title, yaxis_title, coloraxis_colorbar_title_text), same as Line Chart's X Label / Y Label. Each label is anEncodableString, so quotes and non-ASCII text are fine. A whitespace-only field counts as empty. The hover box still shows raw column names (plotly behaviour, same as Line Chart).Behavior change for saved workflows: a workflow with
padjas the p-value column now renders-log10(padj)instead of-log10(pvalue). That is the fix.Since #8504 merged, the operator also has an export path,
generateStandaloneCode(), which writes the same figure as a standalone script. It carried the same hard-coded"-log10(pvalue)"andtitle="Volcano Plot", so this PR applies the same change there: the y column is named after the selected p-value column and the three labels go throughfig.update_layoutas escaped Python literals (the export has no decoder, sopyStringLiteralescapes quotes and backslashes). The engine template and the exported script now label the figure the same way.Rebase note:
coloraxis_colorbar_title_textis emitted unconditionally, in both templates. Once the PR for the significance coloring (#9039) lands, the categorical mode has nocoloraxis, so whichever branch rebases second needs to make that entry conditional.Figures below are the operator's generated Python from
mainand from this branch, run on the 430-gene example workflow from the issue; the third one has the title and both axis labels filled in.Any related issues, documentation, discussions?
Closes #9037
PR #8504 (merged) added the
generateStandaloneCode()export path with the same hard-coded"-log10(pvalue)"; the mirroring described above is the follow-up it needed.How was this PR tested?
New tests in
VolcanoPlotOpDescSpec, written before the source change:generateStandaloneCode: the y column and axis are named after the p-value column (including a name with a quote,p"adj), the labels fall back to the column-derived defaults, a set label is emitted as an escaped literal and the default title is gone, the color bar carries the X Label text, and whitespace-only keeps the defaultSmoke run: the generated code was executed outside Texera (pytexera stubbed, pandas + plotly 5.24.1) on the 430-gene mock table. Defaults give title
Volcano Plot, xlfc_shrunk, y-log10(padj), color barlfc_shrunk; with the labels set the figure shows the typed text; whitespace-only title and y label fall back to the defaults. The exported script for the default and the all-labels configuration parses withast.parse.Was this PR authored or co-authored using generative AI tooling?
Yes. Generated-by: Claude Code (claude-fable-5-1). Co-authored with Claude; the author reviewed the change before submission.