-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(plugin-chart-echarts): fix forecasts on verbose metrics #18252
fix(plugin-chart-echarts): fix forecasts on verbose metrics #18252
Conversation
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
Outdated
Show resolved
Hide resolved
Great catch! It turns out |
Codecov Report
@@ Coverage Diff @@
## master #18252 +/- ##
=======================================
Coverage 66.32% 66.32%
=======================================
Files 1592 1592
Lines 62569 62569
Branches 6295 6295
=======================================
+ Hits 41500 41501 +1
Misses 19416 19416
+ Partials 1653 1652 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
…8252) * fix(plugin-chart-echarts): fix forecasts on verbose metrics * oops! 🙊 * check for DTTM_ALIAS
…8252) * fix(plugin-chart-echarts): fix forecasts on verbose metrics * oops! 🙊 * check for DTTM_ALIAS
…8252) * fix(plugin-chart-echarts): fix forecasts on verbose metrics * oops! 🙊 * check for DTTM_ALIAS
SUMMARY
This fixes a bug in the ECharts timeseries chart, where using a metric with a verbose name caused the forecast and observations to be on separate series (one with the verbose name, and the other with the raw name). A test is also added that ensures verbose names are correctly handled going forward. In addition, all references to "prophet" are replaced with "forecast" in preparation for decoupling the forecasting functionality from Prophet.
AFTER
Now the metric with a verbose name displays correctly in both the chart and the tooltip:
The forecast section is also split up into one control per line:
BEFORE
Previously, the predictions would show up as different series with their non-verbose name, both on the chart and the tooltip
Before, the forecast section headers wrapping caused the controls to be misaligned:
TESTING INSTRUCTIONS
COUNT(*)
metric as your primary metricADDITIONAL INFORMATION