You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yo...@apache.org on 2022/09/16 07:58:01 UTC
[superset] branch master updated: refactor: get Axis from a helper function (#21449)
This is an automated email from the ASF dual-hosted git repository.
yongjiezhao 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 2d16100dbc refactor: get Axis from a helper function (#21449)
2d16100dbc is described below
commit 2d16100dbcc1d12d4c077b109f80ade53785077e
Author: Yongjie Zhao <yo...@apache.org>
AuthorDate: Fri Sep 16 15:57:45 2022 +0800
refactor: get Axis from a helper function (#21449)
---
.../src/operators/pivotOperator.ts | 9 ++++----
.../src/operators/prophetOperator.ts | 13 +++++-------
.../src/operators/timeComparePivotOperator.ts | 9 ++++----
.../src/operators/utils/{index.ts => getAxis.ts} | 24 ++++++++++++++++------
.../src/operators/utils/index.ts | 1 +
.../test/operators/pivotOperator.test.ts | 21 ++++++++++---------
.../test/operators/prophetOperator.test.ts | 1 +
.../operators/timeComparePivotOperator.test.ts | 2 +-
.../src/query/normalizeTimeColumn.ts | 1 +
.../src/Timeseries/transformProps.ts | 8 ++++----
10 files changed, 50 insertions(+), 39 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
index 564495fcbc..efd4f50a8b 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts
@@ -17,27 +17,26 @@
* under the License.
*/
import {
- DTTM_ALIAS,
ensureIsArray,
getColumnLabel,
getMetricLabel,
PostProcessingPivot,
} from '@superset-ui/core';
import { PostProcessingFactory } from './types';
+import { getAxis } from './utils';
export const pivotOperator: PostProcessingFactory<PostProcessingPivot> = (
formData,
queryObject,
) => {
const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel);
- const { x_axis: xAxis } = formData;
+ const xAxis = getAxis(formData);
- if ((xAxis || queryObject.is_timeseries) && metricLabels.length) {
- const index = [getColumnLabel(xAxis || DTTM_ALIAS)];
+ if (xAxis && metricLabels.length) {
return {
operation: 'pivot',
options: {
- index,
+ index: [xAxis],
columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
// Create 'dummy' mean aggregates to assign cell values in pivot table
// use the 'mean' aggregates to avoid drop NaN. PR: https://github.com/apache-superset/superset-ui/pull/1231
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts
index ff0fa0fb65..5d8d7feea9 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/prophetOperator.ts
@@ -16,20 +16,17 @@
* specific language governing permissions and limitationsxw
* under the License.
*/
-import {
- DTTM_ALIAS,
- getColumnLabel,
- PostProcessingProphet,
-} from '@superset-ui/core';
+import { PostProcessingProphet } from '@superset-ui/core';
import { PostProcessingFactory } from './types';
+import { getAxis } from './utils';
/* eslint-disable @typescript-eslint/no-unused-vars */
export const prophetOperator: PostProcessingFactory<PostProcessingProphet> = (
formData,
queryObject,
) => {
- const index = getColumnLabel(formData.x_axis || DTTM_ALIAS);
- if (formData.forecastEnabled) {
+ const xAxis = getAxis(formData);
+ if (formData.forecastEnabled && xAxis) {
return {
operation: 'prophet',
options: {
@@ -39,7 +36,7 @@ export const prophetOperator: PostProcessingFactory<PostProcessingProphet> = (
yearly_seasonality: formData.forecastSeasonalityYearly,
weekly_seasonality: formData.forecastSeasonalityWeekly,
daily_seasonality: formData.forecastSeasonalityDaily,
- index,
+ index: xAxis,
},
};
}
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
index a4c0f5eea6..e1310a4e3a 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/timeComparePivotOperator.ts
@@ -18,20 +18,20 @@
* under the License.
*/
import {
- DTTM_ALIAS,
ensureIsArray,
getColumnLabel,
NumpyFunction,
PostProcessingPivot,
} from '@superset-ui/core';
-import { getMetricOffsetsMap, isTimeComparison } from './utils';
+import { getMetricOffsetsMap, isTimeComparison, getAxis } from './utils';
import { PostProcessingFactory } from './types';
export const timeComparePivotOperator: PostProcessingFactory<PostProcessingPivot> =
(formData, queryObject) => {
const metricOffsetMap = getMetricOffsetsMap(formData, queryObject);
+ const xAxis = getAxis(formData);
- if (isTimeComparison(formData, queryObject)) {
+ if (isTimeComparison(formData, queryObject) && xAxis) {
const aggregates = Object.fromEntries(
[...metricOffsetMap.values(), ...metricOffsetMap.keys()].map(metric => [
metric,
@@ -39,12 +39,11 @@ export const timeComparePivotOperator: PostProcessingFactory<PostProcessingPivot
{ operator: 'mean' as NumpyFunction },
]),
);
- const index = [getColumnLabel(formData.x_axis || DTTM_ALIAS)];
return {
operation: 'pivot',
options: {
- index,
+ index: [xAxis],
columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
drop_missing_columns: !formData?.show_empty_columns,
aggregates,
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/getAxis.ts
similarity index 59%
copy from superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts
copy to superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/getAxis.ts
index 8d65ca1e59..d47089504e 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/getAxis.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,10 +13,23 @@
* 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 { TIME_COMPARISON_SEPARATOR } from './constants';
+import {
+ DTTM_ALIAS,
+ getColumnLabel,
+ isDefined,
+ QueryFormData,
+} from '@superset-ui/core';
+
+export const getAxis = (formData: QueryFormData): string | undefined => {
+ // The formData should be "raw form_data" -- the snake_case version of formData rather than camelCase.
+ if (!(formData.granularity_sqla || formData.x_axis)) {
+ return undefined;
+ }
+
+ return isDefined(formData.x_axis)
+ ? getColumnLabel(formData.x_axis)
+ : DTTM_ALIAS;
+};
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 8d65ca1e59..809ebe06ed 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,3 +21,4 @@ export { getMetricOffsetsMap } from './getMetricOffsetsMap';
export { isTimeComparison } from './isTimeComparison';
export { isDerivedSeries } from './isDerivedSeries';
export { TIME_COMPARISON_SEPARATOR } from './constants';
+export { getAxis } from './getAxis';
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts
index 41c294b405..c6c7f905d2 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts
@@ -56,13 +56,9 @@ const queryObject: QueryObject = {
test('skip pivot', () => {
expect(pivotOperator(formData, queryObject)).toEqual(undefined);
- expect(
- pivotOperator(formData, { ...queryObject, is_timeseries: false }),
- ).toEqual(undefined);
expect(
pivotOperator(formData, {
...queryObject,
- is_timeseries: true,
metrics: [],
}),
).toEqual(undefined);
@@ -70,7 +66,10 @@ test('skip pivot', () => {
test('pivot by __timestamp without groupby', () => {
expect(
- pivotOperator(formData, { ...queryObject, is_timeseries: true }),
+ pivotOperator(
+ { ...formData, granularity_sqla: 'time_column' },
+ queryObject,
+ ),
).toEqual({
operation: 'pivot',
options: {
@@ -87,11 +86,13 @@ test('pivot by __timestamp without groupby', () => {
test('pivot by __timestamp with groupby', () => {
expect(
- pivotOperator(formData, {
- ...queryObject,
- columns: ['foo', 'bar'],
- is_timeseries: true,
- }),
+ pivotOperator(
+ { ...formData, granularity_sqla: 'time_column' },
+ {
+ ...queryObject,
+ columns: ['foo', 'bar'],
+ },
+ ),
).toEqual({
operation: 'pivot',
options: {
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts
index 824efe5c15..2f3c63ddf8 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/prophetOperator.test.ts
@@ -47,6 +47,7 @@ test('should do prophetOperator with default index', () => {
prophetOperator(
{
...formData,
+ granularity_sqla: 'time_column',
forecastEnabled: true,
forecastPeriods: '3',
forecastInterval: '5',
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts
index 304756dee7..bf46940d71 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/timeComparePivotOperator.test.ts
@@ -72,10 +72,10 @@ test('should pivot on any type of timeCompare', () => {
...formData,
comparison_type: cType,
time_compare: ['1 year ago', '1 year later'],
+ granularity_sqla: 'time_column',
},
{
...queryObject,
- is_timeseries: true,
},
),
).toEqual({
diff --git a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts
index 95a06fdf6c..9879e9fe0a 100644
--- a/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts
+++ b/superset-frontend/packages/superset-ui-core/src/query/normalizeTimeColumn.ts
@@ -32,6 +32,7 @@ export function normalizeTimeColumn(
formData: QueryFormData,
queryObject: QueryObject,
): QueryObject {
+ // The formData should be "raw form_data" -- the snake_case version of formData rather than camelCase.
if (!(isFeatureEnabled(FeatureFlag.GENERIC_CHART_AXES) && formData.x_axis)) {
return queryObject;
}
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 5a90818e9c..5d49b657e8 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -20,9 +20,7 @@
import {
AnnotationLayer,
CategoricalColorNamespace,
- DTTM_ALIAS,
GenericDataType,
- getColumnLabel,
getNumberFormatter,
isEventAnnotationLayer,
isFormulaAnnotationLayer,
@@ -30,8 +28,9 @@ import {
isTimeseriesAnnotationLayer,
TimeseriesChartDataResponseResult,
t,
+ DTTM_ALIAS,
} from '@superset-ui/core';
-import { isDerivedSeries } from '@superset-ui/chart-controls';
+import { getAxis, isDerivedSeries } from '@superset-ui/chart-controls';
import { EChartsCoreOption, SeriesOption } from 'echarts';
import { ZRLineType } from 'echarts/types/src/util/types';
import {
@@ -149,8 +148,9 @@ export default function transformProps(
const colorScale = CategoricalColorNamespace.getScale(colorScheme as string);
const rebasedData = rebaseForecastDatum(data, verboseMap);
+ // todo: if the both granularity_sqla and x_axis are `null`, should throw an error
const xAxisCol =
- verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS);
+ verboseMap[xAxisOrig] || getAxis(chartProps.rawFormData) || DTTM_ALIAS;
const isHorizontal = orientation === OrientationType.horizontal;
const { totalStackedValues, thresholdValues } = extractDataTotalValues(
rebasedData,