fix(plugin-chart-echarts): fix forecasts on verbose metrics (#18252)

* fix(plugin-chart-echarts): fix forecasts on verbose metrics * oops! 🙊 * check for DTTM_ALIAS
2022-02-02 13:51:51 +02:00 · 2022-02-02 13:51:51 +02:00 · 2929bb1680
parent 7ad38d5ba1
commit 2929bb1680
7 changed files with 137 additions and 71 deletions
--- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/forecastInterval.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/forecastInterval.tsx
@ -75,6 +75,8 @@ export const forecastIntervalControls: ControlPanelSectionConfig = {
          ),
        },
      },
+    ],
+    [
      {
        name: 'forecastSeasonalityYearly',
        config: {
@ -111,6 +113,8 @@ export const forecastIntervalControls: ControlPanelSectionConfig = {
          ),
        },
      },
+    ],
+    [
      {
        name: 'forecastSeasonalityDaily',
        config: {
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@ -46,10 +46,10 @@ import {
 import { extractAnnotationLabels } from '../utils/annotation';
 import {
  extractForecastSeriesContext,
-  extractProphetValuesFromTooltipParams,
-  formatProphetTooltipSeries,
-  rebaseTimeseriesDatum,
-} from '../utils/prophet';
+  extractForecastValuesFromTooltipParams,
+  formatForecastTooltipSeries,
+  rebaseForecastDatum,
+} from '../utils/forecast';
 import { defaultGrid, defaultTooltip, defaultYAxis } from '../defaults';
 import {
  getPadding,
@ -130,11 +130,11 @@ export default function transformProps(
  }: EchartsMixedTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };

  const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
-  const rebasedDataA = rebaseTimeseriesDatum(data1, verboseMap);
+  const rebasedDataA = rebaseForecastDatum(data1, verboseMap);
  const rawSeriesA = extractSeries(rebasedDataA, {
    fillNeighborValue: stack ? 0 : undefined,
  });
-  const rebasedDataB = rebaseTimeseriesDatum(data2, verboseMap);
+  const rebasedDataB = rebaseForecastDatum(data2, verboseMap);
  const rawSeriesB = extractSeries(rebasedDataB, {
    fillNeighborValue: stackB ? 0 : undefined,
  });
@ -321,19 +321,19 @@ export default function transformProps(
        const xValue: number = richTooltip
          ? params[0].value[0]
          : params.value[0];
-        const prophetValue: any[] = richTooltip ? params : [params];
+        const forecastValue: any[] = richTooltip ? params : [params];

        if (richTooltip && tooltipSortByMetric) {
-          prophetValue.sort((a, b) => b.data[1] - a.data[1]);
+          forecastValue.sort((a, b) => b.data[1] - a.data[1]);
        }

        const rows: Array<string> = [`${tooltipTimeFormatter(xValue)}`];
-        const prophetValues =
-          extractProphetValuesFromTooltipParams(prophetValue);
+        const forecastValues =
+          extractForecastValuesFromTooltipParams(forecastValue);

-        Object.keys(prophetValues).forEach(key => {
-          const value = prophetValues[key];
-          const content = formatProphetTooltipSeries({
+        Object.keys(forecastValues).forEach(key => {
+          const value = forecastValues[key];
+          const content = formatForecastTooltipSeries({
            ...value,
            seriesName: key,
            formatter: primarySeries.has(key) ? formatter : formatterSecondary,
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@ -38,7 +38,7 @@ import {
  EchartsTimeseriesSeriesType,
  TimeseriesChartTransformedProps,
 } from './types';
-import { ForecastSeriesEnum, ProphetValue } from '../types';
+import { ForecastSeriesEnum, ForecastValue } from '../types';
 import { parseYAxisBound } from '../utils/controls';
 import {
  currentSeries,
@ -51,10 +51,10 @@ import { extractAnnotationLabels } from '../utils/annotation';
 import {
  extractForecastSeriesContext,
  extractForecastSeriesContexts,
-  extractProphetValuesFromTooltipParams,
-  formatProphetTooltipSeries,
-  rebaseTimeseriesDatum,
-} from '../utils/prophet';
+  extractForecastValuesFromTooltipParams,
+  formatForecastTooltipSeries,
+  rebaseForecastDatum,
+} from '../utils/forecast';
 import { defaultGrid, defaultTooltip, defaultYAxis } from '../defaults';
 import {
  getPadding,
@ -126,7 +126,7 @@ export default function transformProps(
    yAxisTitlePosition,
  }: EchartsTimeseriesFormData = { ...DEFAULT_FORM_DATA, ...formData };
  const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
-  const rebasedData = rebaseTimeseriesDatum(data, verboseMap);
+  const rebasedData = rebaseForecastDatum(data, verboseMap);
  const xAxisCol = verboseMap[xAxisOrig] || xAxisOrig || DTTM_ALIAS;
  const rawSeries = extractSeries(rebasedData, {
    fillNeighborValue: stack && !forecastEnabled ? 0 : undefined,
@ -333,19 +333,19 @@ export default function transformProps(
        const xValue: number = richTooltip
          ? params[0].value[0]
          : params.value[0];
-        const prophetValue: any[] = richTooltip ? params : [params];
+        const forecastValue: any[] = richTooltip ? params : [params];

        if (richTooltip && tooltipSortByMetric) {
-          prophetValue.sort((a, b) => b.data[1] - a.data[1]);
+          forecastValue.sort((a, b) => b.data[1] - a.data[1]);
        }

        const rows: Array<string> = [`${tooltipFormatter(xValue)}`];
-        const prophetValues: Record<string, ProphetValue> =
-          extractProphetValuesFromTooltipParams(prophetValue);
+        const forecastValues: Record<string, ForecastValue> =
+          extractForecastValuesFromTooltipParams(forecastValue);

-        Object.keys(prophetValues).forEach(key => {
-          const value = prophetValues[key];
-          const content = formatProphetTooltipSeries({
+        Object.keys(forecastValues).forEach(key => {
+          const value = forecastValues[key];
+          const content = formatForecastTooltipSeries({
            ...value,
            seriesName: key,
            formatter,
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@ -50,7 +50,7 @@ import {
 } from 'echarts/types/src/component/marker/MarkAreaModel';
 import { MarkLine1DDataItemOption } from 'echarts/types/src/component/marker/MarkLineModel';

-import { extractForecastSeriesContext } from '../utils/prophet';
+import { extractForecastSeriesContext } from '../utils/forecast';
 import { ForecastSeriesEnum, LegendOrientation } from '../types';
 import { EchartsTimeseriesSeriesType } from './types';

--- a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts
@ -68,7 +68,7 @@ export enum LegendType {
  Plain = 'plain',
 }

-export type ProphetValue = {
+export type ForecastValue = {
  marker: TooltipMarker;
  observation?: number;
  forecastTrend?: number;
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
@ -16,17 +16,13 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import {
-  TimeseriesDataRecord,
-  NumberFormatter,
-  DTTM_ALIAS,
-} from '@superset-ui/core';
+import { DataRecord, DTTM_ALIAS, NumberFormatter } from '@superset-ui/core';
 import { CallbackDataParams, OptionName } from 'echarts/types/src/util/types';
 import { TooltipMarker } from 'echarts/types/src/util/format';
 import {
  ForecastSeriesContext,
  ForecastSeriesEnum,
-  ProphetValue,
+  ForecastValue,
 } from '../types';
 import { sanitizeHtml } from './series';

@ -55,10 +51,10 @@ export const extractForecastSeriesContexts = (
    return { ...agg, [context.name]: currentContexts };
  }, {} as { [key: string]: ForecastSeriesEnum[] });

-export const extractProphetValuesFromTooltipParams = (
+export const extractForecastValuesFromTooltipParams = (
  params: (CallbackDataParams & { seriesId: string })[],
-): Record<string, ProphetValue> => {
-  const values: Record<string, ProphetValue> = {};
+): Record<string, ForecastValue> => {
+  const values: Record<string, ForecastValue> = {};
  params.forEach(param => {
    const { marker, seriesId, value } = param;
    const context = extractForecastSeriesContext(seriesId);
@ -68,21 +64,21 @@ export const extractProphetValuesFromTooltipParams = (
        values[context.name] = {
          marker: marker || '',
        };
-      const prophetValues = values[context.name];
+      const forecastValues = values[context.name];
      if (context.type === ForecastSeriesEnum.Observation)
-        prophetValues.observation = numericValue;
+        forecastValues.observation = numericValue;
      if (context.type === ForecastSeriesEnum.ForecastTrend)
-        prophetValues.forecastTrend = numericValue;
+        forecastValues.forecastTrend = numericValue;
      if (context.type === ForecastSeriesEnum.ForecastLower)
-        prophetValues.forecastLower = numericValue;
+        forecastValues.forecastLower = numericValue;
      if (context.type === ForecastSeriesEnum.ForecastUpper)
-        prophetValues.forecastUpper = numericValue;
+        forecastValues.forecastUpper = numericValue;
    }
  });
  return values;
 };

-export const formatProphetTooltipSeries = ({
+export const formatForecastTooltipSeries = ({
  seriesName,
  observation,
  forecastTrend,
@ -90,7 +86,7 @@ export const formatProphetTooltipSeries = ({
  forecastUpper,
  marker,
  formatter,
-}: ProphetValue & {
+}: ForecastValue & {
  seriesName: string;
  marker: TooltipMarker;
  formatter: NumberFormatter;
@ -113,30 +109,34 @@ export const formatProphetTooltipSeries = ({
  return `${row.trim()}`;
 };

-export function rebaseTimeseriesDatum(
-  data: TimeseriesDataRecord[],
+export function rebaseForecastDatum(
+  data: DataRecord[],
  verboseMap: Record<string, string> = {},
 ) {
-  const keys = data.length > 0 ? Object.keys(data[0]) : [];
+  const keys = data.length ? Object.keys(data[0]) : [];

-  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
  return data.map(row => {
-    const newRow: TimeseriesDataRecord = { [DTTM_ALIAS]: '' };
+    const newRow: DataRecord = {};
    keys.forEach(key => {
      const forecastContext = extractForecastSeriesContext(key);
-      const lowerKey = `${forecastContext.name}${ForecastSeriesEnum.ForecastLower}`;
-      let value = row[key] as number;
+      const verboseKey =
+        key !== DTTM_ALIAS && verboseMap[forecastContext.name]
+          ? `${verboseMap[forecastContext.name]}${forecastContext.type}`
+          : key;
+
+      // check if key is equal to lower confidence level. If so, extract it
+      // from the upper bound
+      const lowerForecastKey = `${forecastContext.name}${ForecastSeriesEnum.ForecastLower}`;
+      let value = row[key] as number | null;
      if (
        forecastContext.type === ForecastSeriesEnum.ForecastUpper &&
-        keys.includes(lowerKey) &&
+        keys.includes(lowerForecastKey) &&
        value !== null &&
-        row[lowerKey] !== null
+        row[lowerForecastKey] !== null
      ) {
-        value -= row[lowerKey] as number;
+        value -= row[lowerForecastKey] as number;
      }
-      const newKey =
-        key !== DTTM_ALIAS && verboseMap[key] ? verboseMap[key] : key;
-      newRow[newKey] = value;
+      newRow[verboseKey] = value;
    });
    // eslint-disable-next-line @typescript-eslint/no-unsafe-return
    return newRow;
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
@ -19,10 +19,10 @@
 import { getNumberFormatter, NumberFormats } from '@superset-ui/core';
 import {
  extractForecastSeriesContext,
-  extractProphetValuesFromTooltipParams,
-  formatProphetTooltipSeries,
-  rebaseTimeseriesDatum,
-} from '../../src/utils/prophet';
+  extractForecastValuesFromTooltipParams,
+  formatForecastTooltipSeries,
+  rebaseForecastDatum,
+} from '../../src/utils/forecast';
 import { ForecastSeriesEnum } from '../../src/types';

 describe('extractForecastSeriesContext', () => {
@ -46,16 +46,22 @@ describe('extractForecastSeriesContext', () => {
  });
 });

-describe('rebaseTimeseriesDatum', () => {
+describe('rebaseForecastDatum', () => {
  it('should subtract lower confidence level from upper value', () => {
    expect(
-      rebaseTimeseriesDatum([
+      rebaseForecastDatum([
        {
          __timestamp: new Date('2001-01-01'),
          abc: 10,
          abc__yhat_lower: 1,
          abc__yhat_upper: 20,
        },
+        {
+          __timestamp: new Date('2001-01-01'),
+          abc: 10,
+          abc__yhat_lower: -10,
+          abc__yhat_upper: 20,
+        },
        {
          __timestamp: new Date('2002-01-01'),
          abc: 10,
@ -76,6 +82,12 @@ describe('rebaseTimeseriesDatum', () => {
        abc__yhat_lower: 1,
        abc__yhat_upper: 19,
      },
+      {
+        __timestamp: new Date('2001-01-01'),
+        abc: 10,
+        abc__yhat_lower: -10,
+        abc__yhat_upper: 30,
+      },
      {
        __timestamp: new Date('2002-01-01'),
        abc: 10,
@ -90,12 +102,62 @@ describe('rebaseTimeseriesDatum', () => {
      },
    ]);
  });
+
+  it('should rename all series based on verboseMap but leave __timestamp alone', () => {
+    expect(
+      rebaseForecastDatum(
+        [
+          {
+            __timestamp: new Date('2001-01-01'),
+            abc: 10,
+            abc__yhat_lower: 1,
+            abc__yhat_upper: 20,
+          },
+          {
+            __timestamp: new Date('2002-01-01'),
+            abc: 10,
+            abc__yhat_lower: null,
+            abc__yhat_upper: 20,
+          },
+          {
+            __timestamp: new Date('2003-01-01'),
+            abc: 10,
+            abc__yhat_lower: 1,
+            abc__yhat_upper: null,
+          },
+        ],
+        {
+          abc: 'Abracadabra',
+          __timestamp: 'Time',
+        },
+      ),
+    ).toEqual([
+      {
+        __timestamp: new Date('2001-01-01'),
+        Abracadabra: 10,
+        Abracadabra__yhat_lower: 1,
+        Abracadabra__yhat_upper: 19,
+      },
+      {
+        __timestamp: new Date('2002-01-01'),
+        Abracadabra: 10,
+        Abracadabra__yhat_lower: null,
+        Abracadabra__yhat_upper: 20,
+      },
+      {
+        __timestamp: new Date('2003-01-01'),
+        Abracadabra: 10,
+        Abracadabra__yhat_lower: 1,
+        Abracadabra__yhat_upper: null,
+      },
+    ]);
+  });
 });

-describe('extractProphetValuesFromTooltipParams', () => {
+describe('extractForecastValuesFromTooltipParams', () => {
  it('should extract the proper data from tooltip params', () => {
    expect(
-      extractProphetValuesFromTooltipParams([
+      extractForecastValuesFromTooltipParams([
        {
          marker: '<img>',
          seriesId: 'abc',
@ -140,10 +202,10 @@ describe('extractProphetValuesFromTooltipParams', () => {

 const formatter = getNumberFormatter(NumberFormats.INTEGER);

-describe('formatProphetTooltipSeries', () => {
+describe('formatForecastTooltipSeries', () => {
  it('should generate a proper series tooltip', () => {
    expect(
-      formatProphetTooltipSeries({
+      formatForecastTooltipSeries({
        seriesName: 'abc',
        marker: '<img>',
        observation: 10.1,
@ -151,7 +213,7 @@ describe('formatProphetTooltipSeries', () => {
      }),
    ).toEqual('<img>abc: 10');
    expect(
-      formatProphetTooltipSeries({
+      formatForecastTooltipSeries({
        seriesName: 'qwerty',
        marker: '<img>',
        observation: 10.1,
@ -162,7 +224,7 @@ describe('formatProphetTooltipSeries', () => {
      }),
    ).toEqual('<img>qwerty: 10, ŷ = 20 (5, 12)');
    expect(
-      formatProphetTooltipSeries({
+      formatForecastTooltipSeries({
        seriesName: 'qwerty',
        marker: '<img>',
        forecastTrend: 20,
@ -172,7 +234,7 @@ describe('formatProphetTooltipSeries', () => {
      }),
    ).toEqual('<img>qwerty: ŷ = 20 (5, 12)');
    expect(
-      formatProphetTooltipSeries({
+      formatForecastTooltipSeries({
        seriesName: 'qwerty',
        marker: '<img>',
        observation: 10.1,
@ -182,7 +244,7 @@ describe('formatProphetTooltipSeries', () => {
      }),
    ).toEqual('<img>qwerty: 10 (6, 13)');
    expect(
-      formatProphetTooltipSeries({
+      formatForecastTooltipSeries({
        seriesName: 'qwerty',
        marker: '<img>',
        forecastLower: 7,