rusackas commented on code in PR #40967:
URL: https://github.com/apache/superset/pull/40967#discussion_r3400935737
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts:
##########
@@ -175,3 +175,283 @@ describe('Scatter Chart X-axis Number Formatting', () => {
},
);
});
+
+describe('Scatter Chart Orientation and Dot Size Metric', () => {
+ const baseFormData: EchartsTimeseriesFormData = {
+ ...DEFAULT_FORM_DATA,
+ colorScheme: 'supersetColors',
+ datasource: '1__table',
+ metrics: ['sum_val'],
+ x_axis: 'category_col',
+ groupby: [],
+ viz_type: 'echarts_timeseries_scatter',
+ seriesType: EchartsTimeseriesSeriesType.Scatter,
+ };
+
+ const categoricalData = [
+ {
+ data: [
+ { category_col: 'A', sum_val: 1, size_metric: 10 },
+ { category_col: 'B', sum_val: 2, size_metric: 25 },
+ { category_col: 'C', sum_val: 3, size_metric: 40 },
+ ],
+ colnames: ['category_col', 'sum_val', 'size_metric'],
+ coltypes: [
+ GenericDataType.String,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ ],
+ label_map: {
+ category_col: ['category_col'],
+ sum_val: ['sum_val'],
+ size_metric: ['size_metric'],
+ },
+ },
+ ];
+
+ const baseChartPropsConfig = {
+ width: 800,
+ height: 600,
+ theme: supersetTheme,
+ };
+
+ const getScatterSeries = (props: ReturnType<typeof transformProps>) =>
+ (props.echartOptions.series as any[]).filter(s => s.type === 'scatter');
+
+ const singleMetricData = [
+ {
+ ...categoricalData[0],
+ data: categoricalData[0].data.map(row => ({
+ category_col: row.category_col,
+ sum_val: row.sum_val,
+ })),
+ colnames: ['category_col', 'sum_val'],
+ coltypes: [GenericDataType.String, GenericDataType.Numeric],
+ },
+ ];
+
+ test('horizontal orientation swaps the dimension axis onto the y-axis', ()
=> {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: singleMetricData,
+ formData: { ...baseFormData, orientation: 'horizontal' },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const { xAxis, yAxis } = transformedProps.echartOptions as any;
+ expect(yAxis.type).toBe('category');
+ expect(xAxis.type).toBe('value');
+
+ const series = getScatterSeries(transformedProps);
+ expect(series).toHaveLength(1);
+ // data points are flipped to [metric, dimension]
+ expect(series[0].data[0]).toEqual([1, 'A']);
+ });
+
+ test('vertical orientation keeps the dimension axis on the x-axis', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: singleMetricData,
+ formData: baseFormData,
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const { xAxis, yAxis } = transformedProps.echartOptions as any;
Review Comment:
Same `TestAxis` type used here too.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts:
##########
@@ -175,3 +175,283 @@ describe('Scatter Chart X-axis Number Formatting', () => {
},
);
});
+
+describe('Scatter Chart Orientation and Dot Size Metric', () => {
+ const baseFormData: EchartsTimeseriesFormData = {
+ ...DEFAULT_FORM_DATA,
+ colorScheme: 'supersetColors',
+ datasource: '1__table',
+ metrics: ['sum_val'],
+ x_axis: 'category_col',
+ groupby: [],
+ viz_type: 'echarts_timeseries_scatter',
+ seriesType: EchartsTimeseriesSeriesType.Scatter,
+ };
+
+ const categoricalData = [
+ {
+ data: [
+ { category_col: 'A', sum_val: 1, size_metric: 10 },
+ { category_col: 'B', sum_val: 2, size_metric: 25 },
+ { category_col: 'C', sum_val: 3, size_metric: 40 },
+ ],
+ colnames: ['category_col', 'sum_val', 'size_metric'],
+ coltypes: [
+ GenericDataType.String,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ ],
+ label_map: {
+ category_col: ['category_col'],
+ sum_val: ['sum_val'],
+ size_metric: ['size_metric'],
+ },
+ },
+ ];
+
+ const baseChartPropsConfig = {
+ width: 800,
+ height: 600,
+ theme: supersetTheme,
+ };
+
+ const getScatterSeries = (props: ReturnType<typeof transformProps>) =>
+ (props.echartOptions.series as any[]).filter(s => s.type === 'scatter');
+
+ const singleMetricData = [
+ {
+ ...categoricalData[0],
+ data: categoricalData[0].data.map(row => ({
+ category_col: row.category_col,
+ sum_val: row.sum_val,
+ })),
+ colnames: ['category_col', 'sum_val'],
+ coltypes: [GenericDataType.String, GenericDataType.Numeric],
+ },
+ ];
+
+ test('horizontal orientation swaps the dimension axis onto the y-axis', ()
=> {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: singleMetricData,
+ formData: { ...baseFormData, orientation: 'horizontal' },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const { xAxis, yAxis } = transformedProps.echartOptions as any;
+ expect(yAxis.type).toBe('category');
+ expect(xAxis.type).toBe('value');
+
+ const series = getScatterSeries(transformedProps);
+ expect(series).toHaveLength(1);
+ // data points are flipped to [metric, dimension]
+ expect(series[0].data[0]).toEqual([1, 'A']);
+ });
+
+ test('vertical orientation keeps the dimension axis on the x-axis', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: singleMetricData,
+ formData: baseFormData,
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const { xAxis, yAxis } = transformedProps.echartOptions as any;
+ expect(xAxis.type).toBe('category');
+ expect(yAxis.type).toBe('value');
+
+ const series = getScatterSeries(transformedProps);
+ expect(series[0].data[0]).toEqual(['A', 1]);
+ });
+
+ test('size metric series is not rendered or shown in the legend', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: categoricalData,
+ formData: { ...baseFormData, size: 'size_metric' },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const series = getScatterSeries(transformedProps);
+ expect(series).toHaveLength(1);
+ expect(series[0].name).toBe('sum_val');
+ expect(transformedProps.legendData).toEqual(['sum_val']);
+ });
+
+ test('size metric scales marker areas between min and max dot size', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: categoricalData,
+ formData: {
+ ...baseFormData,
+ size: 'size_metric',
+ minMarkerSize: 5,
+ maxMarkerSize: 30,
+ },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const [series] = getScatterSeries(transformedProps);
+ expect(typeof series.symbolSize).toBe('function');
+ // smallest size value -> min dot size, largest -> max dot size
+ expect(series.symbolSize(['A', 1])).toBe(5);
+ expect(series.symbolSize(['C', 3])).toBe(30);
+ // midpoint value -> midpoint *area*, not midpoint diameter
+ expect(series.symbolSize(['B', 2]) ** 2).toBeCloseTo(
+ (5 ** 2 + 30 ** 2) / 2,
+ );
+ });
+
+ test('size metric lookups follow the dimension key when grouped', () => {
+ const groupedData = [
+ {
+ data: [
+ {
+ category_col: 'A',
+ 'sum_val, g1': 1,
+ 'size_metric, g1': 10,
+ 'sum_val, g2': 2,
+ 'size_metric, g2': 40,
+ },
+ ],
+ colnames: [
+ 'category_col',
+ 'sum_val, g1',
+ 'size_metric, g1',
+ 'sum_val, g2',
+ 'size_metric, g2',
+ ],
+ coltypes: [
+ GenericDataType.String,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ ],
+ label_map: {
+ category_col: ['category_col'],
+ 'sum_val, g1': ['sum_val', 'g1'],
+ 'size_metric, g1': ['size_metric', 'g1'],
+ 'sum_val, g2': ['sum_val', 'g2'],
+ 'size_metric, g2': ['size_metric', 'g2'],
+ },
+ },
+ ];
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: groupedData,
+ formData: {
+ ...baseFormData,
+ groupby: ['group_col'],
+ size: 'size_metric',
+ minMarkerSize: 5,
+ maxMarkerSize: 30,
+ },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const series = getScatterSeries(transformedProps);
+ expect(series.map((s: any) => s.name).sort()).toEqual([
Review Comment:
This one picks up the series type from the typed array now, no annotation
needed.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts:
##########
@@ -175,3 +175,283 @@ describe('Scatter Chart X-axis Number Formatting', () => {
},
);
});
+
+describe('Scatter Chart Orientation and Dot Size Metric', () => {
+ const baseFormData: EchartsTimeseriesFormData = {
+ ...DEFAULT_FORM_DATA,
+ colorScheme: 'supersetColors',
+ datasource: '1__table',
+ metrics: ['sum_val'],
+ x_axis: 'category_col',
+ groupby: [],
+ viz_type: 'echarts_timeseries_scatter',
+ seriesType: EchartsTimeseriesSeriesType.Scatter,
+ };
+
+ const categoricalData = [
+ {
+ data: [
+ { category_col: 'A', sum_val: 1, size_metric: 10 },
+ { category_col: 'B', sum_val: 2, size_metric: 25 },
+ { category_col: 'C', sum_val: 3, size_metric: 40 },
+ ],
+ colnames: ['category_col', 'sum_val', 'size_metric'],
+ coltypes: [
+ GenericDataType.String,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ ],
+ label_map: {
+ category_col: ['category_col'],
+ sum_val: ['sum_val'],
+ size_metric: ['size_metric'],
+ },
+ },
+ ];
+
+ const baseChartPropsConfig = {
+ width: 800,
+ height: 600,
+ theme: supersetTheme,
+ };
+
+ const getScatterSeries = (props: ReturnType<typeof transformProps>) =>
+ (props.echartOptions.series as any[]).filter(s => s.type === 'scatter');
+
+ const singleMetricData = [
+ {
+ ...categoricalData[0],
+ data: categoricalData[0].data.map(row => ({
+ category_col: row.category_col,
+ sum_val: row.sum_val,
+ })),
+ colnames: ['category_col', 'sum_val'],
+ coltypes: [GenericDataType.String, GenericDataType.Numeric],
+ },
+ ];
+
+ test('horizontal orientation swaps the dimension axis onto the y-axis', ()
=> {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: singleMetricData,
+ formData: { ...baseFormData, orientation: 'horizontal' },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const { xAxis, yAxis } = transformedProps.echartOptions as any;
+ expect(yAxis.type).toBe('category');
+ expect(xAxis.type).toBe('value');
+
+ const series = getScatterSeries(transformedProps);
+ expect(series).toHaveLength(1);
+ // data points are flipped to [metric, dimension]
+ expect(series[0].data[0]).toEqual([1, 'A']);
+ });
+
+ test('vertical orientation keeps the dimension axis on the x-axis', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: singleMetricData,
+ formData: baseFormData,
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const { xAxis, yAxis } = transformedProps.echartOptions as any;
+ expect(xAxis.type).toBe('category');
+ expect(yAxis.type).toBe('value');
+
+ const series = getScatterSeries(transformedProps);
+ expect(series[0].data[0]).toEqual(['A', 1]);
+ });
+
+ test('size metric series is not rendered or shown in the legend', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: categoricalData,
+ formData: { ...baseFormData, size: 'size_metric' },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const series = getScatterSeries(transformedProps);
+ expect(series).toHaveLength(1);
+ expect(series[0].name).toBe('sum_val');
+ expect(transformedProps.legendData).toEqual(['sum_val']);
+ });
+
+ test('size metric scales marker areas between min and max dot size', () => {
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: categoricalData,
+ formData: {
+ ...baseFormData,
+ size: 'size_metric',
+ minMarkerSize: 5,
+ maxMarkerSize: 30,
+ },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const [series] = getScatterSeries(transformedProps);
+ expect(typeof series.symbolSize).toBe('function');
+ // smallest size value -> min dot size, largest -> max dot size
+ expect(series.symbolSize(['A', 1])).toBe(5);
+ expect(series.symbolSize(['C', 3])).toBe(30);
+ // midpoint value -> midpoint *area*, not midpoint diameter
+ expect(series.symbolSize(['B', 2]) ** 2).toBeCloseTo(
+ (5 ** 2 + 30 ** 2) / 2,
+ );
+ });
+
+ test('size metric lookups follow the dimension key when grouped', () => {
+ const groupedData = [
+ {
+ data: [
+ {
+ category_col: 'A',
+ 'sum_val, g1': 1,
+ 'size_metric, g1': 10,
+ 'sum_val, g2': 2,
+ 'size_metric, g2': 40,
+ },
+ ],
+ colnames: [
+ 'category_col',
+ 'sum_val, g1',
+ 'size_metric, g1',
+ 'sum_val, g2',
+ 'size_metric, g2',
+ ],
+ coltypes: [
+ GenericDataType.String,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ GenericDataType.Numeric,
+ ],
+ label_map: {
+ category_col: ['category_col'],
+ 'sum_val, g1': ['sum_val', 'g1'],
+ 'size_metric, g1': ['size_metric', 'g1'],
+ 'sum_val, g2': ['sum_val', 'g2'],
+ 'size_metric, g2': ['size_metric', 'g2'],
+ },
+ },
+ ];
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ queriesData: groupedData,
+ formData: {
+ ...baseFormData,
+ groupby: ['group_col'],
+ size: 'size_metric',
+ minMarkerSize: 5,
+ maxMarkerSize: 30,
+ },
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const series = getScatterSeries(transformedProps);
+ expect(series.map((s: any) => s.name).sort()).toEqual([
+ 'sum_val, g1',
+ 'sum_val, g2',
+ ]);
+ const g1 = series.find((s: any) => s.name === 'sum_val, g1');
+ const g2 = series.find((s: any) => s.name === 'sum_val, g2');
Review Comment:
Same here... the callbacks infer from `TestScatterSeries[]` now.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -337,6 +342,89 @@ export default function transformProps(
xAxisType,
},
);
+
+ // Dot size by metric (scatter): the size metric's series are excluded from
+ // rendering and instead provide per-point values that scale each marker's
+ // area between minMarkerSize and maxMarkerSize.
+ const sizeMetricLabel =
+ seriesType === EchartsTimeseriesSeriesType.Scatter && size
+ ? getMetricLabel(size)
+ : undefined;
+ const sizeSeriesLabel = isDefined(sizeMetricLabel)
+ ? (verboseMap[sizeMetricLabel!] ?? sizeMetricLabel)
+ : undefined;
+ const valueMetricLabels = ensureIsArray(metrics)
+ .map(getMetricLabel)
+ .map(label => verboseMap[label] ?? label);
+ // When the size metric is also a value metric, the query dedupes them into a
+ // single column, so each point's own value doubles as its size value.
+ const sizeIsValueMetric = isDefined(sizeSeriesLabel)
+ ? valueMetricLabels.includes(sizeSeriesLabel!)
+ : false;
+ const isSizeSeries = (name: string) =>
+ isDefined(sizeSeriesLabel) &&
+ !sizeIsValueMetric &&
+ (name === sizeSeriesLabel || name.startsWith(`${sizeSeriesLabel}, `));
+ const rawSeries = sizeSeriesLabel
+ ? allRawSeries.filter(entry => !isSizeSeries(String(entry.name ?? '')))
+ : allRawSeries;
+ // Maps each value series' dimension key to a lookup from primary-axis value
+ // to size value.
+ let sizeLookups: Map<string, Map<DataRecordValue, number>> | undefined;
+ let sizeExtent: [number, number] | undefined;
+ if (sizeSeriesLabel) {
+ let sizeMin = Infinity;
+ let sizeMax = -Infinity;
+ if (sizeIsValueMetric) {
+ rawSeries.forEach(entry => {
+ (entry.data as DataRecordValue[][]).forEach(datum => {
+ const sizeValue = isHorizontal ? datum[0] : datum[1];
+ if (typeof sizeValue === 'number' && Number.isFinite(sizeValue)) {
+ sizeMin = Math.min(sizeMin, sizeValue);
+ sizeMax = Math.max(sizeMax, sizeValue);
+ }
+ });
+ });
+ } else {
+ sizeLookups = new Map();
+ allRawSeries
+ .filter(entry => isSizeSeries(String(entry.name ?? '')))
+ .forEach(entry => {
+ const name = String(entry.name ?? '');
+ const dimsKey =
+ name === sizeSeriesLabel
+ ? ''
+ : name.slice(sizeSeriesLabel.length + 2);
+ const lookup = new Map<DataRecordValue, number>();
+ (entry.data as DataRecordValue[][]).forEach(datum => {
+ const axisValue = isHorizontal ? datum[1] : datum[0];
+ const sizeValue = isHorizontal ? datum[0] : datum[1];
+ if (typeof sizeValue === 'number' && Number.isFinite(sizeValue)) {
+ lookup.set(axisValue, sizeValue);
+ sizeMin = Math.min(sizeMin, sizeValue);
+ sizeMax = Math.max(sizeMax, sizeValue);
+ }
+ });
+ sizeLookups!.set(dimsKey, lookup);
+ });
+ }
+ if (sizeMin <= sizeMax) {
+ sizeExtent = [sizeMin, sizeMax];
+ }
+ }
+ // Strips the metric label off a series name, leaving the dimension key used
+ // to match a value series with its size series.
+ const getSeriesDimsKey = (name: string): string => {
+ const matchedLabel = valueMetricLabels.find(
+ label => name === label || name.startsWith(`${label}, `),
+ );
+ if (matchedLabel === undefined) {
+ return name;
+ }
+ return name === matchedLabel ? '' : name.slice(matchedLabel.length + 2);
+ };
+ const markerSizeRange: [number, number] = [minMarkerSize, maxMarkerSize];
Review Comment:
Good catch. The range gets normalized in `transformProps` now, and there is
a test covering the inverted case.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
