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

   ### What changes were proposed in this PR?
   
   **Background (#7146).** The Filled Area Plot operator draws stacked area 
lines, one per Line Group, which only makes sense when the groups share an x 
axis. `FilledAreaPlotOpDesc.performTableCheck()` therefore emits a Python guard 
that suppresses the chart when too many groups have x-value sets disjoint from 
the others; the comment above it documents the rule as "more than 5 percents of 
the groups have disjoint sets of x attributes". When the guard fires, the run 
still completes successfully — the operator just yields fallback HTML instead 
of the chart, with no warning anywhere.
   
   **The defect.** The guard computes the threshold as `(len(grouped) // 100) * 
5`. The integer division floors first, so the tolerance is 0 for any chart with 
fewer than 100 line groups, and a single disjoint group suppresses the whole 
chart — e.g. a 40-group chart with 1 disjoint group (2.5%, well under the 
documented 5%) was suppressed. The two expressions only agree when the group 
count is an exact multiple of 100. This has been latent since the operator was 
introduced in #2086: an `X_values` typo in the same block kept the accumulated 
x-value set from ever growing, so the guard fired constantly for an unrelated 
reason; #6894 fixed that typo, which made the tolerance arithmetic the deciding 
factor for the first time.
   
   **The change.** Swap the operand order to `(len(grouped) * 5) // 100`, plus 
four behavior-neutral cleanups to the same emitted block: the per-group 
`set(...unique())` is hoisted into one local (was built twice per group), `== 
None` becomes `is None`, the always-true `elif not ...` becomes `else`, and the 
loop `break`s once the error is set.
   
   Below 20 groups both expressions yield 0, which is correct (1/19 = 5.3% 
exceeds 5%), so the fix only changes behavior at 20+ line groups.
   
   ### Any related issues, documentation, discussions?
   
   Fixes #7146. Adjacent to #6728 / #6894, which fixed an `X_values` typo in 
the same block but did not touch the tolerance arithmetic.
   
   ### How was this PR tested?
   
   `performTableCheck()` previously had no test coverage. This PR adds 10 tests 
to `FilledAreaPlotOpDescSpec` (TDD: written red first, green after the fix):
   
   - 9 assertions on the generated guard, including the tolerance expression, a 
single `.unique()` per iteration, `break` placement after the error assignment, 
and that user-provided column names are never emitted verbatim.
   - 1 runtime test that executes the generated Python (real pandas groupby + 
plotly rendering; only the `pytexera` import seam is stubbed) across 9 boundary 
datasets: 19 groups/1 disjoint still suppressed (5.3% > 5%), 20/1 and 40/2 at 
exactly 5.0% render, 40/1 renders (the reported bug), 40/3 suppressed, plus 
all-disjoint, single-group, empty-table, and missing-column cases. It cancels 
(not fails) when no python with pandas+plotly is available.
   
   Full spec: 20/20 passing. Red-check verified: reverting only the tolerance 
line makes the runtime test fail on the 40/1 and exactly-5% cases.
   
   Checks from [CONTRIBUTING.md] all pass locally: `sbt 
WorkflowOperator/scalafmtCheck` and `WorkflowOperator/Test/scalafmtCheck` (no 
violations), `sbt "WorkflowOperator/scalafixAll --check"` (clean), and the full 
`sbt WorkflowOperator/test` module suite — 1967 succeeded, 0 failed, 2 pending. 
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Co-authored by: Claude Code (Claude Fable 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to