You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/23 05:05:23 UTC
[PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
rusackas opened a new pull request, #41311:
URL: https://github.com/apache/superset/pull/41311
### SUMMARY
Fixes #21734
Prophet forecast confidence intervals appeared to "disappear" in the chart tooltip whenever the forecast crossed zero. The root cause is in the ECharts time-series forecast tooltip formatter (`superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts`), which used **truthiness** checks on the forecast values:
```ts
if (forecastTrend) { ... } // a trend of exactly 0 is skipped
if (forecastLower && forecastUpper) { } // a bound/height of 0 drops the whole band
```
Because `0` is falsy in JS, a legitimate forecast trend of `0`, a lower bound of `0`, or a zero-height band caused the corresponding values to be silently omitted from the tooltip. This matches the report in #21734 ("if the central estimate crosses 0, they are gone").
The fix switches these to explicit `typeof value === 'number'` checks โ the same pattern already used for the `observation` value just above โ so zero values are formatted and displayed like any other number.
Backend was already verified clean by the earlier test-only TDD guard (#40141): the Prophet wrapper does not clamp at zero. This PR addresses the remaining frontend rendering bug.
### BEFORE/AFTER
- **Before:** a forecast point with `ลท = 0`, or a confidence bound of `0`, rendered no trend/interval in the tooltip.
- **After:** `ลท = 0` and intervals like `(0, 7)` or `(4, 4)` render correctly.
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npx jest plugins/plugin-chart-echarts/test/utils/forecast.test.ts
```
Four new unit tests pin the zero-crossing behavior (trend = 0, lower = 0, zero-height band, all-zero). They fail on `master` and pass with this change.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #21734
- [ ] Required feature flags:
- [x] Changes UI (forecast tooltip rendering)
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
๐ค Generated with [Claude Code](https://claude.com/claude-code)
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4775793881
<!-- Bito Reply -->
The flagged issue is correct. Using `typeof value === 'number'` includes `NaN` and `Infinity`, which can lead to invalid tooltip displays. Using `Number.isFinite(value)` is the appropriate fix to ensure only valid numeric values are processed.
To resolve this, update the guard definitions in `superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts` as follows:
**superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts**
```
const hasTrend = Number.isFinite(forecastTrend);
const hasLower = Number.isFinite(forecastLower);
const hasUpper = Number.isFinite(forecastUpper);
```
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4781011421
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #41311:
URL: https://github.com/apache/superset/pull/41311#discussion_r3457608651
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -98,15 +98,21 @@ export const formatForecastTooltipSeries = ({
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
let value = typeof observation === 'number' ? formatter(observation) : '';
- if (forecastTrend || forecastLower || forecastUpper) {
+ // Use explicit numeric checks rather than truthiness so that legitimate
+ // zero values (e.g. a forecast that crosses zero, or a confidence bound of
+ // exactly 0) are not dropped from the tooltip.
+ const hasTrend = typeof forecastTrend === 'number';
+ const hasLower = typeof forecastLower === 'number';
+ const hasUpper = typeof forecastUpper === 'number';
Review Comment:
Good catch โ the old truthiness check happened to drop `NaN`, and my `typeof` guards let it back through. Switched to `Number.isFinite` so zero stays but `NaN`/`Infinity` are excluded, plus a test for it.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #41311:
URL: https://github.com/apache/superset/pull/41311#discussion_r3458823154
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -98,15 +98,24 @@ export const formatForecastTooltipSeries = ({
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
let value = typeof observation === 'number' ? formatter(observation) : '';
- if (forecastTrend || forecastLower || forecastUpper) {
+ // Use finite-number checks rather than truthiness so that legitimate
+ // zero values (e.g. a forecast that crosses zero, or a confidence bound of
+ // exactly 0) are not dropped from the tooltip, while non-finite values
+ // (NaN/Infinity) are still excluded.
+ const isFiniteNumber = (val: number | undefined): val is number =>
+ typeof val === 'number' && Number.isFinite(val);
+ const hasTrend = isFiniteNumber(forecastTrend);
+ const hasLower = isFiniteNumber(forecastLower);
+ const hasUpper = isFiniteNumber(forecastUpper);
+ if (hasTrend || hasLower || hasUpper) {
// forecast values take the form of "20, y = 30 (10, 40)"
// where the first part is the observation, the second part is the forecast trend
// and the third part is the lower and upper bounds
- if (forecastTrend) {
+ if (isFiniteNumber(forecastTrend)) {
if (value) value += ', ';
value += `ลท = ${formatter(forecastTrend)}`;
}
- if (forecastLower && forecastUpper) {
+ if (isFiniteNumber(forecastLower) && isFiniteNumber(forecastUpper)) {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Semantic duplication in diff</b></div>
<div id="fix">
Lines 114 and 118 call `isFiniteNumber()` directly instead of using the `hasTrend`, `hasLower`, `hasUpper` variables defined at lines 107-109. This duplicates the finite check logic and creates inconsistency โ line 110 correctly uses these variables, but inner conditions re-check the same values. Use the existing boolean variables for consistency.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
// and the third part is the lower and upper bounds
if (hasTrend) {
if (value) value += ', ';
value += `ลท = ${formatter(forecastTrend)}`;
}
if (hasLower && hasUpper) {
````
</div>
</details>
</div>
<small><i>Code Review Run #ff7f95</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4775814149
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41311?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 64.35%. Comparing base ([`5916ec4`](https://app.codecov.io/gh/apache/superset/commit/5916ec4876a1d3594dbbe139b62a2fc5309a4c5a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4033925`](https://app.codecov.io/gh/apache/superset/commit/40339257bdbc6f9696f22fdbd8f2728c08750fb6?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 @@
## master #41311 +/- ##
=======================================
Coverage 64.35% 64.35%
=======================================
Files 2653 2653
Lines 144952 144955 +3
Branches 33433 33433
=======================================
+ Hits 93288 93291 +3
Misses 49978 49978
Partials 1686 1686
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41311/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage ฮ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/41311/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.61% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [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#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41311?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>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41311:
URL: https://github.com/apache/superset/pull/41311
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4775760842
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41311:
URL: https://github.com/apache/superset/pull/41311#discussion_r3457154187
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -98,15 +98,21 @@ export const formatForecastTooltipSeries = ({
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
let value = typeof observation === 'number' ? formatter(observation) : '';
- if (forecastTrend || forecastLower || forecastUpper) {
+ // Use explicit numeric checks rather than truthiness so that legitimate
+ // zero values (e.g. a forecast that crosses zero, or a confidence bound of
+ // exactly 0) are not dropped from the tooltip.
+ const hasTrend = typeof forecastTrend === 'number';
+ const hasLower = typeof forecastLower === 'number';
+ const hasUpper = typeof forecastUpper === 'number';
Review Comment:
**Suggestion:** The new numeric guards treat `NaN` (and `Infinity`) as valid because `typeof value === 'number'` is true for them, so malformed forecast points can now render tooltip text like `ลท = NaN` instead of being skipped. Use finite-number checks (`Number.isFinite`) for forecast values so zero is still allowed but invalid numeric payloads are not displayed. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Timeseries forecast tooltips may show `ลท = NaN` text.
- โ ๏ธ MixedTimeseries forecast tooltips can display NaN confidence bands.
- โ ๏ธ Users may misinterpret corrupted forecasts as meaningful numbers.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. In a unit test (similar to
`superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts`), import
`formatForecastTooltipSeries` from
`superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts` (function defined
around line 86).
2. Call `formatForecastTooltipSeries` with a non-finite forecast value, for example:
```ts
const row = formatForecastTooltipSeries({
seriesName: 'abc',
marker: '<img>',
observation: 10,
forecastTrend: Number.NaN,
formatter: getNumberFormatter(NumberFormats.INTEGER),
});
```
This follows the same usage pattern as the tests at `forecast.test.ts:73-120`.
3. Inside `formatForecastTooltipSeries`, the guards at `forecast.ts:104-106` treat
`forecastTrend` as present because `typeof forecastTrend === 'number'` is true for `NaN`,
so `hasTrend` is `true` and the `if (hasTrend)` block at `forecast.ts:111-113` executes.
4. The formatter is invoked as `formatter(forecastTrend)` at `forecast.ts:113`, producing
a string representation of the non-finite value (typically `"NaN"` or `"โ"`), and the
function returns a tooltip row like `['<img>abc', '10, ลท = NaN']`, which is then rendered
by the tooltip builders in `Timeseries/transformProps.ts:1083-1087` and
`MixedTimeseries/transformProps.ts:23-29`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=be589d45777e4856aa65ccb0703a8c11&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=be589d45777e4856aa65ccb0703a8c11&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
**Line:** 104:106
**Comment:**
*Incorrect Condition Logic: The new numeric guards treat `NaN` (and `Infinity`) as valid because `typeof value === 'number'` is true for them, so malformed forecast points can now render tooltip text like `ลท = NaN` instead of being skipped. Use finite-number checks (`Number.isFinite`) for forecast values so zero is still allowed but invalid numeric payloads are not displayed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41311&comment_hash=83784de351317911497bc4ab003c92a1c95cb99dc99db99421645b42a47d0b4b&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41311&comment_hash=83784de351317911497bc4ab003c92a1c95cb99dc99db99421645b42a47d0b4b&reaction=dislike'>๐</a>
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4778061872
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4777945748
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #41311:
URL: https://github.com/apache/superset/pull/41311#discussion_r3459832386
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts:
##########
@@ -98,15 +98,24 @@ export const formatForecastTooltipSeries = ({
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
let value = typeof observation === 'number' ? formatter(observation) : '';
- if (forecastTrend || forecastLower || forecastUpper) {
+ // Use finite-number checks rather than truthiness so that legitimate
+ // zero values (e.g. a forecast that crosses zero, or a confidence bound of
+ // exactly 0) are not dropped from the tooltip, while non-finite values
+ // (NaN/Infinity) are still excluded.
+ const isFiniteNumber = (val: number | undefined): val is number =>
+ typeof val === 'number' && Number.isFinite(val);
+ const hasTrend = isFiniteNumber(forecastTrend);
+ const hasLower = isFiniteNumber(forecastLower);
+ const hasUpper = isFiniteNumber(forecastUpper);
+ if (hasTrend || hasLower || hasUpper) {
// forecast values take the form of "20, y = 30 (10, 40)"
// where the first part is the observation, the second part is the forecast trend
// and the third part is the lower and upper bounds
- if (forecastTrend) {
+ if (isFiniteNumber(forecastTrend)) {
if (value) value += ', ';
value += `ลท = ${formatter(forecastTrend)}`;
}
- if (forecastLower && forecastUpper) {
+ if (isFiniteNumber(forecastLower) && isFiniteNumber(forecastUpper)) {
Review Comment:
Yep, cleaner โ swapped both inner checks to reuse `hasTrend`/`hasLower`/`hasUpper`. Thanks.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): show forecast tooltip values when they equal zero [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41311:
URL: https://github.com/apache/superset/pull/41311#issuecomment-4779361747
### <span aria-hidden="true">โ
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">๐จ</span> Latest commit | 013b38dea654ec6d119c07ece7ef754fe3d8eae5 |
|<span aria-hidden="true">๐</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a3a80999502990008ae73ec |
|<span aria-hidden="true">๐</span> Deploy Preview | [https://deploy-preview-41311--superset-docs-preview.netlify.app](https://deploy-preview-41311--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">๐ฑ</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">๐ค</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcvLDYMwDADQXXwmwiXl427AGG4wIhLgKAkSBbF7c3uXd0OIuoUMH4AKvpF3txTP_qx9SoeY5tXbt5k1iuOUTVZdsw_mkqglTBJW_Y1TKR1bHpCIWmyIEHFg6a04eP4) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41311--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org