feat(visualization): export the tables, ranges and embedded documents - #8366
Draft
kz930 wants to merge 6 commits into
Draft
feat(visualization): export the tables, ranges and embedded documents#8366kz930 wants to merge 6 commits into
kz930 wants to merge 6 commits into
Conversation
Nine of them, and what they have in common is that the output is not a plot drawn from numbers: a table rendered as a figure, a range a reader moves, an image or an HTML document passed through, and the waterfall, which reads as a chart but is built by accumulating rows rather than plotting them. The waterfall plots every row and appends the total as a bar of its own, rather than consuming the last row to make one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Automated Reviewer SuggestionsBased on the
|
This was referenced Sep 2, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8366 +/- ##
============================================
- Coverage 94.11% 94.00% -0.11%
- Complexity 4811 4853 +42
============================================
Files 1197 1204 +7
Lines 48813 49065 +252
Branches 5906 5948 +42
============================================
+ Hits 45939 46124 +185
- Misses 1420 1465 +45
- Partials 1454 1476 +22
*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:
|
Contributor
📊 Arrow Flight E2E bench(no arrow-flight-e2e.csv in artifact) |
27 tasks
carloea2
approved these changes
Sep 4, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
The table and embedded output exports look good.
kz930
marked this pull request as draft
September 4, 2026 17:12
The translator no longer emits plotly into every script; it asks the operators in the plan what they need beyond pandas. Five of the nine here draw with plotly and mix in `PlotlyStandaloneCode`, which the hierarchy and graph charts introduce. The other four state nothing, which is the point of asking: an image, an HTML document, a nested table and a passed-through URL name none of plotly's modules, so a script built from them runs wherever pandas is installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
approved these changes
Sep 4, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
I checked the new import declarations. They look good.
Figure Factory Table imports `plotly.figure_factory` inside its own generated code, and uses none of the three modules the mixin declares. Mixing it in would put an import in the script that the script never reads. The mixin states what a script has to import at module scope. An operator that imports what it needs inside its own code has nothing to add there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
carloea2
approved these changes
Sep 4, 2026
carloea2
left a comment
carloea2
left a comment
Contributor
There was a problem hiding this comment.
The import cleanup looks good.
…e default Both say `producesDataFrame = true`, which the trait already returns. They sit among charts that override it to false, which is what made saying it look like a distinction; the comment beside it said the same thing a second time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every chart wrote output.html and output.json, and the whole plan runs as one program in one directory, so a workflow ending in two charts kept a single picture. The name is a placeholder now, outputHtml and outputJson, and the translator hands each operator a stem of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The engine's operator says 'Figure Factory Table is not available' on one line; the exported script still said 'Figure factory table' across three, so the same refusal reached a reader as two different pages. Tables Plot had the same split without the wording difference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@carloea2 this moved after your approval: each chart names its own output files, both paths now write the same reason page, and a producesDataFrame override that only restated the default is gone. Would you take another look when you have a moment? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Nine of them, and what they have in common is that the output is not a plot
drawn from numbers: a table rendered as a figure, a range a reader moves, an
image or an HTML document passed through, and the waterfall, which reads as a
chart but is built by accumulating rows rather than plotting them.
The waterfall plots every row and appends the total as a bar of its own,
rather than consuming the last row to make one.
Any related issues, documentation, discussions?
Part of #8325, 15 of 27; that issue lists the set in order. It needs #8327 for the trait, so it does not compile until that lands, and the rows these operators add to the verification runner follow with the harness rather than as whole new files here.
Closes #7974.
Closes #8418, the task this change is the whole of.
How was this PR tested?
Each operator asserts the block it emits in its own spec. Once the harness lands, each is also run through the engine and through its generated script, on every configuration its schema offers, and the two answers compared.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)