You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "kz930 (via GitHub)" <gi...@apache.org> on 2026/08/02 21:44:50 UTC
[PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
kz930 opened a new pull request, #7257:
URL: https://github.com/apache/texera/pull/7257
### What changes were proposed in this pull request?
Three colour settings — Line Chart's Line Color, Continuous Error Bands' Fill Color, and Figure Factory Table's Font Color — were plain strings whose only statement of what is legal lived in a description. Each now declares a `pattern` mirroring `ColorValidator` in plotly's `_plotly_utils/basevalidators.py`: 3- or 6-digit hex, an `rgb`/`rgba`/`hsl`/`hsla`/`hsv`/`hsva` call, a `var(--…)` variable, or a colour name. Empty stays legal, since every one of these paths omits the colour argument when the field is blank.
Two details are deliberate. Letters are matched through character classes rather than an inline `(?i)`, because the browser compiles the pattern with `new RegExp`. And `\s*` sits between every element, because plotly strips spaces before matching and really does accept `#ff ffff`; rejecting those would make the field stricter than the library it feeds. The colour-name branch is lexical, so a misspelling still reaches plotly — matching exactly would mean copying plotly's 148 CSS names into the annotation.
### Why are the changes needed?
Nothing inspected the value before the run, so a mistyped colour aborted a workflow inside plotly with an error naming a plotly property rather than the field the user filled in: `ValueError: Invalid value of type 'builtins.str' received for the 'color' property of scatter.line`.
### Does this PR introduce any user-facing change?
Yes. The three fields now reject `1`, `#12`, `#ggg`, `#ffff`, `rgb(1,2)` and `rgb(-1,2,3)` in the form. Every value that previously produced a chart still does.
### How was this patch tested?
`WorkflowOperator/compile`, `scalafmtCheckAll`, and the three operators' descriptor specs (18 tests). The pattern was also read back out of the generated schema, compiled with `new RegExp`, and checked against sixteen values — ten accepted, six rejected — matching plotly's own verdict on each except a misspelled colour name, as noted above.
Closes #7252
Generated-by: Claude Code (Claude Opus 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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5160508338
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 3 worse · ⚪ 12 noise (<±5%) · 0 without baseline
> Compared against main `436b37e` 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](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/30768579030)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 393 | 0.24 | 25,041/30,701/30,701 us | 🔴 +17.5% / 🔴 +102.8% |
| ⚪ | bs=100 sw=10 sl=64 | 812 | 0.496 | 119,994/169,508/169,508 us | ⚪ within ±5% / 🔴 +60.1% |
| ⚪ | bs=1000 sw=10 sl=64 | 942 | 0.575 | 1,061,294/1,141,362/1,141,362 us | ⚪ within ±5% / 🔴 +12.3% |
<details><summary>Baseline details</summary>
Latest main `436b37e` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 393 tuples/sec | 441 tuples/sec | 790.88 tuples/sec | -10.9% | -50.3% |
| bs=10 sw=10 sl=64 | MB/s | 0.24 MB/s | 0.269 MB/s | 0.483 MB/s | -10.8% | -50.3% |
| bs=10 sw=10 sl=64 | p50 | 25,041 us | 21,306 us | 12,348 us | +17.5% | +102.8% |
| bs=10 sw=10 sl=64 | p95 | 30,701 us | 31,691 us | 15,390 us | -3.1% | +99.5% |
| bs=10 sw=10 sl=64 | p99 | 30,701 us | 31,691 us | 18,935 us | -3.1% | +62.1% |
| bs=100 sw=10 sl=64 | throughput | 812 tuples/sec | 840 tuples/sec | 1,009 tuples/sec | -3.3% | -19.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.496 MB/s | 0.513 MB/s | 0.616 MB/s | -3.3% | -19.4% |
| bs=100 sw=10 sl=64 | p50 | 119,994 us | 116,920 us | 99,753 us | +2.6% | +20.3% |
| bs=100 sw=10 sl=64 | p95 | 169,508 us | 161,706 us | 105,895 us | +4.8% | +60.1% |
| bs=100 sw=10 sl=64 | p99 | 169,508 us | 161,706 us | 113,263 us | +4.8% | +49.7% |
| bs=1000 sw=10 sl=64 | throughput | 942 tuples/sec | 937 tuples/sec | 1,037 tuples/sec | +0.5% | -9.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.575 MB/s | 0.572 MB/s | 0.633 MB/s | +0.5% | -9.2% |
| bs=1000 sw=10 sl=64 | p50 | 1,061,294 us | 1,068,117 us | 976,194 us | -0.6% | +8.7% |
| bs=1000 sw=10 sl=64 | p95 | 1,141,362 us | 1,146,355 us | 1,016,411 us | -0.4% | +12.3% |
| bs=1000 sw=10 sl=64 | p99 | 1,141,362 us | 1,146,355 us | 1,045,212 us | -0.4% | +9.2% |
</details>
<details><summary>Raw CSV</summary>
```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,509.46,200,128000,393,0.240,25041.02,30700.79,30700.79
1,100,10,64,20,2462.91,2000,1280000,812,0.496,119994.20,169507.96,169507.96
2,1000,10,64,20,21225.28,20000,12800000,942,0.575,1061294.17,1141362.32,1141362.32
```
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "xuang7 (via GitHub)" <gi...@apache.org>.
xuang7 merged PR #7257:
URL: https://github.com/apache/texera/pull/7257
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5160496566
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7257?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 82.31%. Comparing base ([`436b37e`](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b1b57f7`](https://app.codecov.io/gh/apache/texera/commit/b1b57f7863acde8824e509e1b494aa75bde3aab2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7257 +/- ##
============================================
- Coverage 82.64% 82.31% -0.34%
+ Complexity 4080 4055 -25
============================================
Files 1162 1162
Lines 46282 46351 +69
Branches 5160 5162 +2
============================================
- Hits 38250 38154 -96
- Misses 6341 6516 +175
+ Partials 1691 1681 -10
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.10% <ø> (+1.10%)` | :arrow_up: | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.65% <ø> (ø)` | | Carriedforward from [436b37e](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.30% <ø> (ø)` | | Carriedforward from [436b37e](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.65% <ø> (-41.96%)` | :arrow_down: | |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.02% <ø> (-1.96%)` | :arrow_down: | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.80% <ø> (ø)` | | Carriedforward from [436b37e](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.55% <ø> (ø)` | | Carriedforward from [436b37e](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.36% <ø> (ø)` | | Carriedforward from [436b37e](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7257/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.31% <ø> (ø)` | | Carriedforward from [436b37e](https://app.codecov.io/gh/apache/texera/commit/436b37e9b62e6be2ef8ea911ec30387dd7e80415?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7257?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5160484017
<!-- backport-auto-label-report -->
### 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](https://github.com/apache/texera/actions/runs/30768571712)._
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5160483836
<!-- texera:template-compliance -->
👋 Thanks for opening this pull request, @kz930!
It looks like the pull request description doesn't quite follow our template yet:
- The **What changes were proposed in this PR?** section is missing; please keep the template's headings.
- The **How was this PR tested?** section is missing; please keep the template's headings.
- The **Was this PR authored or co-authored using generative AI tooling?** section is missing; please keep the template's headings.
Filling out the template helps reviewers understand and triage your contribution faster. Please edit the description to complete it. This message will disappear automatically once the template is followed.
You can find the template prompts by editing the description, or see [CONTRIBUTING.md](https://github.com/apache/texera/blob/main/CONTRIBUTING.md) for the full contribution flow.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5160484224
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@eugenegujing`, `@carloea2`
You can notify them by mentioning `@eugenegujing`, `@carloea2` in a comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5184830308
Added `ColorPatternSpec`: it reads the pattern back out of the generated schema for each of the four fields that carry it, then checks the ten accepted values — one per branch — against six rejected near-misses. I also confirmed those same verdicts under `new RegExp` and against plotly itself.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(visualization): constrain the colour settings to what plotly accepts [texera]
Posted by "kz930 (via GitHub)" <gi...@apache.org>.
kz930 commented on PR #7257:
URL: https://github.com/apache/texera/pull/7257#issuecomment-5185031405
Refined to one test per case: four fields × sixteen values, each reported by name, so a regression names every value that broke instead of stopping at the first.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org