You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/05/18 11:40:58 UTC
[superset] branch master updated: feat: Make time shifted series colors match the original series (#24048)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new df4d16a7ee feat: Make time shifted series colors match the original series (#24048)
df4d16a7ee is described below
commit df4d16a7eec3ba06bf1d5e9f016cb0d86b9693e5
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Thu May 18 08:40:50 2023 -0300
feat: Make time shifted series colors match the original series (#24048)
---
.../src/operators/utils/index.ts | 1 +
.../src/operators/utils/isDerivedSeries.ts | 7 +-
.../utils/{isDerivedSeries.ts => timeOffset.ts} | 42 ++++----
.../operators/utils/timeOffset.test.ts} | 20 ++--
.../src/MixedTimeseries/transformProps.ts | 112 +++++++++++++--------
.../src/Timeseries/transformProps.ts | 56 +++++++----
.../src/Timeseries/transformers.ts | 5 +-
7 files changed, 147 insertions(+), 96 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
index 67f230dd63..1d91a6965f 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
@@ -21,4 +21,5 @@ export { getMetricOffsetsMap } from './getMetricOffsetsMap';
export { isTimeComparison } from './isTimeComparison';
export { isDerivedSeries } from './isDerivedSeries';
export { extractExtraMetrics } from './extractExtraMetrics';
+export { getOriginalSeries, hasTimeOffset } from './timeOffset';
export { TIME_COMPARISON_SEPARATOR } from './constants';
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
index 67815756f8..ddb3e425df 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
@@ -23,7 +23,7 @@ import {
QueryFormData,
ComparisonType,
} from '@superset-ui/core';
-import { isString } from 'lodash';
+import { hasTimeOffset } from './timeOffset';
export const isDerivedSeries = (
series: JsonObject,
@@ -33,9 +33,6 @@ export const isDerivedSeries = (
if (comparisonType !== ComparisonType.Values) {
return false;
}
-
const timeCompare: string[] = ensureIsArray(formData?.time_compare);
- return isString(series.name)
- ? !!timeCompare.find(timeOffset => series.name.endsWith(timeOffset))
- : false;
+ return hasTimeOffset(series, timeCompare);
};
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts
similarity index 53%
copy from superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
copy to superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts
index 67815756f8..b11572c6dd 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts
@@ -17,25 +17,33 @@
* specific language governing permissions and limitationsxw
* under the License.
*/
-import {
- ensureIsArray,
- JsonObject,
- QueryFormData,
- ComparisonType,
-} from '@superset-ui/core';
+import { JsonObject } from '@superset-ui/core';
import { isString } from 'lodash';
-export const isDerivedSeries = (
+export const hasTimeOffset = (
series: JsonObject,
- formData: QueryFormData,
-): boolean => {
- const comparisonType = formData.comparison_type;
- if (comparisonType !== ComparisonType.Values) {
- return false;
- }
-
- const timeCompare: string[] = ensureIsArray(formData?.time_compare);
- return isString(series.name)
- ? !!timeCompare.find(timeOffset => series.name.endsWith(timeOffset))
+ timeCompare: string[],
+): boolean =>
+ isString(series.name)
+ ? !!timeCompare.find(
+ timeOffset =>
+ // offset is represented as <offset>, group by list
+ series.name.includes(`${timeOffset},`) ||
+ // offset is represented as <metric>__<offset>
+ series.name.includes(`__${timeOffset}`),
+ )
: false;
+
+export const getOriginalSeries = (
+ seriesName: string,
+ timeCompare: string[],
+): string => {
+ let result = seriesName;
+ timeCompare.forEach(compare => {
+ // offset is represented as <offset>, group by list
+ result = result.replace(`${compare},`, '');
+ // offset is represented as <metric>__<offset>
+ result = result.replace(`__${compare}`, '');
+ });
+ return result.trim();
};
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts
similarity index 60%
copy from superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
copy to superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts
index 67f230dd63..bee8c5bbbd 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts
@@ -1,4 +1,3 @@
-/* eslint-disable camelcase */
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -14,11 +13,18 @@
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
- * specific language governing permissions and limitationsxw
+ * specific language governing permissions and limitations
* under the License.
*/
-export { getMetricOffsetsMap } from './getMetricOffsetsMap';
-export { isTimeComparison } from './isTimeComparison';
-export { isDerivedSeries } from './isDerivedSeries';
-export { extractExtraMetrics } from './extractExtraMetrics';
-export { TIME_COMPARISON_SEPARATOR } from './constants';
+import { getOriginalSeries } from '@superset-ui/chart-controls';
+
+test('returns the series name when time compare is empty', () => {
+ const seriesName = 'sum';
+ expect(getOriginalSeries(seriesName, [])).toEqual(seriesName);
+});
+
+test('returns the original series name', () => {
+ const seriesName = 'sum__1_month_ago';
+ const timeCompare = ['1_month_ago'];
+ expect(getOriginalSeries(seriesName, timeCompare)).toEqual('sum');
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
index d47b0047c9..a14904b05c 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -17,6 +17,7 @@
* under the License.
*/
/* eslint-disable camelcase */
+import { invert } from 'lodash';
import {
AnnotationLayer,
CategoricalColorNamespace,
@@ -32,7 +33,9 @@ import {
getXAxisLabel,
isPhysicalColumn,
isDefined,
+ ensureIsArray,
} from '@superset-ui/core';
+import { getOriginalSeries } from '@superset-ui/chart-controls';
import { EChartsCoreOption, SeriesOption } from 'echarts';
import {
DEFAULT_FORM_DATA,
@@ -296,55 +299,76 @@ export default function transformProps(
formatter: formatterSecondary,
});
+ const array = ensureIsArray(chartProps.rawFormData?.time_compare);
+ const inverted = invert(verboseMap);
+
rawSeriesA.forEach(entry => {
- const transformedSeries = transformSeries(entry, colorScale, {
- area,
- markerEnabled,
- markerSize,
- areaOpacity: opacity,
- seriesType,
- showValue,
- stack: Boolean(stack),
- yAxisIndex,
- filterState,
- seriesKey: entry.name,
- sliceId,
- queryIndex: 0,
- formatter:
- seriesType === EchartsTimeseriesSeriesType.Bar
- ? maxLabelFormatter
- : formatter,
- showValueIndexes: showValueIndexesA,
- totalStackedValues,
- thresholdValues,
- });
+ const entryName = String(entry.name || '');
+ const seriesName = inverted[entryName] || entryName;
+ const colorScaleKey = getOriginalSeries(seriesName, array);
+
+ const transformedSeries = transformSeries(
+ entry,
+ colorScale,
+ colorScaleKey,
+ {
+ area,
+ markerEnabled,
+ markerSize,
+ areaOpacity: opacity,
+ seriesType,
+ showValue,
+ stack: Boolean(stack),
+ yAxisIndex,
+ filterState,
+ seriesKey: entry.name,
+ sliceId,
+ queryIndex: 0,
+ formatter:
+ seriesType === EchartsTimeseriesSeriesType.Bar
+ ? maxLabelFormatter
+ : formatter,
+ showValueIndexes: showValueIndexesA,
+ totalStackedValues,
+ thresholdValues,
+ },
+ );
if (transformedSeries) series.push(transformedSeries);
});
rawSeriesB.forEach(entry => {
- const transformedSeries = transformSeries(entry, colorScale, {
- area: areaB,
- markerEnabled: markerEnabledB,
- markerSize: markerSizeB,
- areaOpacity: opacityB,
- seriesType: seriesTypeB,
- showValue: showValueB,
- stack: Boolean(stackB),
- yAxisIndex: yAxisIndexB,
- filterState,
- seriesKey: primarySeries.has(entry.name as string)
- ? `${entry.name} (1)`
- : entry.name,
- sliceId,
- queryIndex: 1,
- formatter:
- seriesTypeB === EchartsTimeseriesSeriesType.Bar
- ? maxLabelFormatterSecondary
- : formatterSecondary,
- showValueIndexes: showValueIndexesB,
- totalStackedValues: totalStackedValuesB,
- thresholdValues: thresholdValuesB,
- });
+ const entryName = String(entry.name || '');
+ const seriesName = `${inverted[entryName] || entryName} (1)`;
+ const colorScaleKey = getOriginalSeries(seriesName, array);
+
+ const transformedSeries = transformSeries(
+ entry,
+ colorScale,
+ colorScaleKey,
+ {
+ area: areaB,
+ markerEnabled: markerEnabledB,
+ markerSize: markerSizeB,
+ areaOpacity: opacityB,
+ seriesType: seriesTypeB,
+ showValue: showValueB,
+ stack: Boolean(stackB),
+ yAxisIndex: yAxisIndexB,
+ filterState,
+ seriesKey: primarySeries.has(entry.name as string)
+ ? `${entry.name} (1)`
+ : entry.name,
+ sliceId,
+ queryIndex: 1,
+ formatter:
+ seriesTypeB === EchartsTimeseriesSeriesType.Bar
+ ? maxLabelFormatterSecondary
+ : formatterSecondary,
+ showValueIndexes: showValueIndexesB,
+ totalStackedValues: totalStackedValuesB,
+ thresholdValues: thresholdValuesB,
+ },
+ );
if (transformedSeries) series.push(transformedSeries);
});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index ac096a0697..40ce5323e7 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -17,10 +17,12 @@
* under the License.
*/
/* eslint-disable camelcase */
+import { invert } from 'lodash';
import {
AnnotationLayer,
AxisType,
CategoricalColorNamespace,
+ ensureIsArray,
GenericDataType,
getMetricLabel,
getNumberFormatter,
@@ -36,6 +38,7 @@ import {
} from '@superset-ui/core';
import {
extractExtraMetrics,
+ getOriginalSeries,
isDerivedSeries,
} from '@superset-ui/chart-controls';
import { EChartsCoreOption, SeriesOption } from 'echarts';
@@ -227,30 +230,43 @@ export default function transformProps(
contributionMode || isAreaExpand ? ',.0%' : yAxisFormat,
);
+ const array = ensureIsArray(chartProps.rawFormData?.time_compare);
+ const inverted = invert(verboseMap);
+
rawSeries.forEach(entry => {
const lineStyle = isDerivedSeries(entry, chartProps.rawFormData)
? { type: 'dashed' as ZRLineType }
: {};
- const transformedSeries = transformSeries(entry, colorScale, {
- area,
- filterState,
- seriesContexts,
- markerEnabled,
- markerSize,
- areaOpacity: opacity,
- seriesType,
- stack,
- formatter,
- showValue,
- onlyTotal,
- totalStackedValues: sortedTotalValues,
- showValueIndexes,
- thresholdValues,
- richTooltip,
- sliceId,
- isHorizontal,
- lineStyle,
- });
+
+ const entryName = String(entry.name || '');
+ const seriesName = inverted[entryName] || entryName;
+ const colorScaleKey = getOriginalSeries(seriesName, array);
+
+ const transformedSeries = transformSeries(
+ entry,
+ colorScale,
+ colorScaleKey,
+ {
+ area,
+ filterState,
+ seriesContexts,
+ markerEnabled,
+ markerSize,
+ areaOpacity: opacity,
+ seriesType,
+ stack,
+ formatter,
+ showValue,
+ onlyTotal,
+ totalStackedValues: sortedTotalValues,
+ showValueIndexes,
+ thresholdValues,
+ richTooltip,
+ sliceId,
+ isHorizontal,
+ lineStyle,
+ },
+ );
if (transformedSeries) {
if (stack === StackControlsValue.Stream) {
// bug in Echarts - `stackStrategy: 'all'` doesn't work with nulls, so we cast them to 0
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
index 953037c3d5..37e3eb9fce 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@@ -51,7 +51,6 @@ import {
MarkArea2DDataItemOption,
} from 'echarts/types/src/component/marker/MarkAreaModel';
import { MarkLine1DDataItemOption } from 'echarts/types/src/component/marker/MarkLineModel';
-
import { extractForecastSeriesContext } from '../utils/forecast';
import {
EchartsTimeseriesSeriesType,
@@ -144,6 +143,7 @@ export const getBaselineSeriesForStream = (
export function transformSeries(
series: SeriesOption,
colorScale: CategoricalColorScale,
+ colorScaleKey: string,
opts: {
area?: boolean;
filterState?: FilterState;
@@ -186,7 +186,6 @@ export function transformSeries(
showValueIndexes = [],
thresholdValues = [],
richTooltip,
- seriesKey,
sliceId,
isHorizontal = false,
queryIndex = 0,
@@ -236,7 +235,7 @@ export function transformSeries(
}
// forcing the colorScale to return a different color for same metrics across different queries
const itemStyle = {
- color: colorScale(seriesKey || forecastSeries.name, sliceId),
+ color: colorScale(colorScaleKey, sliceId),
opacity,
};
let emphasis = {};