Skip to content

Commit

Permalink
feat(plugin-chart-echarts): support non-timeseries x-axis (#17917)
Browse files Browse the repository at this point in the history
* feat(plugin-chart-echarts): support non-timeseries x-axis

* fix tests

* change formula return type from Date to number

* add x_axis test coverage

* rename func and improve coverage

* add x-axis control to bar chart

* remove redundant console.log

* fix description

* make x-axis control mandatory

* 🙃

* fix x-axis formatter

* fix showValues

* fix implicit rDTTM_ALIAS references in postProcessing

* replace TIME_COLUMN with DTTM_ALIAS

* fix remaining implicit indexes

* fix: Disable filtering on wide result sets (#18021)

* fix: handle null values in time-series table (#18039)

* cleanup column_type_mappings (#17569)

Signed-off-by: Đặng Minh Dũng <dungdm93@live.com>

* important change to MakeFile (#18037)

* add missing is_timeseries to pivot op

Co-authored-by: Erik Ritter <erik.ritter@airbnb.com>
Co-authored-by: Grace Guo <grace.guo@airbnb.com>
Co-authored-by: Đặng Minh Dũng <dungdm93@live.com>
Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
  • Loading branch information
5 people committed Jan 21, 2022
1 parent b083b34 commit e9651ea
Show file tree
Hide file tree
Showing 42 changed files with 487 additions and 199 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,30 @@
* under the License.
*/
import {
DTTM_ALIAS,
ensureIsArray,
getColumnLabel,
getMetricLabel,
PostProcessingPivot,
} from '@superset-ui/core';
import { PostProcessingFactory } from './types';
import { TIME_COLUMN, isValidTimeCompare } from './utils';
import { isValidTimeCompare } from './utils';
import { timeComparePivotOperator } from './timeComparePivotOperator';

export const pivotOperator: PostProcessingFactory<
PostProcessingPivot | undefined
> = (formData, queryObject) => {
const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel);
if (queryObject.is_timeseries && metricLabels.length) {
const { x_axis: xAxis } = formData;
if ((xAxis || queryObject.is_timeseries) && metricLabels.length) {
if (isValidTimeCompare(formData, queryObject)) {
return timeComparePivotOperator(formData, queryObject);
}

return {
operation: 'pivot',
options: {
index: [TIME_COLUMN],
index: [xAxis || DTTM_ALIAS],
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitationsxw
* under the License.
*/
import { PostProcessingProphet } from '@superset-ui/core';
import { DTTM_ALIAS, PostProcessingProphet } from '@superset-ui/core';
import { PostProcessingFactory } from './types';

export const prophetOperator: PostProcessingFactory<
Expand All @@ -32,6 +32,7 @@ export const prophetOperator: PostProcessingFactory<
yearly_seasonality: formData.forecastSeasonalityYearly,
weekly_seasonality: formData.forecastSeasonalityWeekly,
daily_seasonality: formData.forecastSeasonalityDaily,
index: formData.x_axis || DTTM_ALIAS,
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
* under the License.
*/
import {
DTTM_ALIAS,
ensureIsArray,
isPhysicalColumn,
PostProcessingResample,
} from '@superset-ui/core';
import { PostProcessingFactory } from './types';
import { TIME_COLUMN } from './utils';

export const resampleOperator: PostProcessingFactory<
PostProcessingResample | undefined
Expand All @@ -45,7 +45,7 @@ export const resampleOperator: PostProcessingFactory<
method: resampleMethod,
rule: resampleRule,
fill_value: resampleZeroFill ? 0 : null,
time_column: TIME_COLUMN,
time_column: formData.x_axis || DTTM_ALIAS,
groupby_columns,
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
* under the License.
*/
import {
ensureIsInt,
ComparisionType,
ensureIsArray,
RollingType,
PostProcessingRolling,
ensureIsInt,
PostProcessingCum,
ComparisionType,
PostProcessingRolling,
RollingType,
} from '@superset-ui/core';
import {
getMetricOffsetsMap,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,23 @@
* specific language governing permissions and limitationsxw
* under the License.
*/
import { PostProcessingSort, RollingType } from '@superset-ui/core';
import { DTTM_ALIAS, PostProcessingSort, RollingType } from '@superset-ui/core';
import { PostProcessingFactory } from './types';
import { TIME_COLUMN } from './utils';

export const sortOperator: PostProcessingFactory<
PostProcessingSort | undefined
> = (formData, queryObject) => {
const { x_axis: xAxis } = formData;
if (
queryObject.is_timeseries &&
(xAxis || queryObject.is_timeseries) &&
Object.values(RollingType).includes(formData.rolling_type)
) {
const index = xAxis || DTTM_ALIAS;
return {
operation: 'sort',
options: {
columns: {
[TIME_COLUMN]: true,
[index]: true,
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@
*/
import {
ComparisionType,
PostProcessingPivot,
NumpyFunction,
DTTM_ALIAS,
ensureIsArray,
getColumnLabel,
NumpyFunction,
PostProcessingPivot,
} from '@superset-ui/core';
import {
getMetricOffsetsMap,
Expand Down Expand Up @@ -57,7 +58,7 @@ export const timeComparePivotOperator: PostProcessingFactory<
return {
operation: 'pivot',
options: {
index: ['__timestamp'],
index: [formData.x_axis || DTTM_ALIAS],
columns: ensureIsArray(queryObject.columns).map(getColumnLabel),
aggregates:
comparisonType === ComparisionType.Values ? valuesAgg : changeAgg,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,3 @@
* under the License.
*/
export const TIME_COMPARISON_SEPARATOR = '__';
export const TIME_COLUMN = '__timestamp';
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@
*/
export { getMetricOffsetsMap } from './getMetricOffsetsMap';
export { isValidTimeCompare } from './isValidTimeCompare';
export { TIME_COMPARISON_SEPARATOR, TIME_COLUMN } from './constants';
export { TIME_COMPARISON_SEPARATOR } from './constants';
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,32 @@ test('pivot by __timestamp with groupby', () => {
});
});

test('pivot by x_axis with groupby', () => {
expect(
pivotOperator(
{
...formData,
x_axis: 'baz',
},
{
...queryObject,
columns: ['foo', 'bar'],
},
),
).toEqual({
operation: 'pivot',
options: {
index: ['baz'],
columns: ['foo', 'bar'],
aggregates: {
'count(*)': { operator: 'mean' },
'sum(val)': { operator: 'mean' },
},
drop_missing_columns: false,
},
});
});

test('timecompare in formdata', () => {
expect(
pivotOperator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { QueryObject, SqlaFormData } from '@superset-ui/core';
import { DTTM_ALIAS, QueryObject, SqlaFormData } from '@superset-ui/core';
import { prophetOperator } from '../../../src';

const formData: SqlaFormData = {
Expand All @@ -42,7 +42,7 @@ test('should skip prophetOperator', () => {
expect(prophetOperator(formData, queryObject)).toEqual(undefined);
});

test('should do prophetOperator', () => {
test('should do prophetOperator with default index', () => {
expect(
prophetOperator(
{
Expand All @@ -65,6 +65,36 @@ test('should do prophetOperator', () => {
yearly_seasonality: true,
weekly_seasonality: false,
daily_seasonality: false,
index: DTTM_ALIAS,
},
});
});

test('should do prophetOperator over named column', () => {
expect(
prophetOperator(
{
...formData,
x_axis: 'ds',
forecastEnabled: true,
forecastPeriods: '3',
forecastInterval: '5',
forecastSeasonalityYearly: true,
forecastSeasonalityWeekly: false,
forecastSeasonalityDaily: false,
},
queryObject,
),
).toEqual({
operation: 'prophet',
options: {
time_grain: 'P1Y',
periods: 3.0,
confidence_interval: 5.0,
yearly_seasonality: true,
weekly_seasonality: false,
daily_seasonality: false,
index: 'ds',
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ test('should skip resampleOperator', () => {
).toEqual(undefined);
});

test('should do resample', () => {
test('should do resample on implicit time column', () => {
expect(
resampleOperator(
{ ...formData, resample_method: 'ffill', resample_rule: '1D' },
Expand All @@ -80,6 +80,29 @@ test('should do resample', () => {
});
});

test('should do resample on x-axis', () => {
expect(
resampleOperator(
{
...formData,
x_axis: 'ds',
resample_method: 'ffill',
resample_rule: '1D',
},
queryObject,
),
).toEqual({
operation: 'resample',
options: {
fill_value: null,
groupby_columns: [],
method: 'ffill',
rule: '1D',
time_column: 'ds',
},
});
});

test('should do zerofill resample', () => {
expect(
resampleOperator(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,19 @@ test('sort by __timestamp', () => {
},
});
});

test('sort by named x-axis', () => {
expect(
sortOperator(
{ ...formData, x_axis: 'ds', rolling_type: 'cumsum' },
{ ...queryObject },
),
).toEqual({
operation: 'sort',
options: {
columns: {
ds: true,
},
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,29 @@ test('time compare pivot: difference/percentage/ratio', () => {
});
});
});

test('time compare pivot on x-axis', () => {
expect(
timeComparePivotOperator(
{
...formData,
comparison_type: 'values',
time_compare: ['1 year ago', '1 year later'],
x_axis: 'ds',
},
queryObject,
),
).toEqual({
operation: 'pivot',
options: {
aggregates: {
'count(*)': { operator: 'mean' },
'count(*)__1 year ago': { operator: 'mean' },
'count(*)__1 year later': { operator: 'mean' },
},
drop_missing_columns: false,
columns: [],
index: ['ds'],
},
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,16 @@ export interface PostProcessingContribution {
export interface PostProcessingPivot {
operation: 'pivot';
options: {
index: string[];
columns: string[];
aggregates: Aggregates;
column_fill_value?: string;
columns: string[];
combine_value_with_metric?: boolean;
drop_missing_columns?: boolean;
flatten_columns?: boolean;
index: string[];
marginal_distribution_name?: string;
marginal_distributions?: boolean;
metric_fill_value?: any;
reset_index?: boolean;
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export enum FeatureFlag {
ALERTS_ATTACH_REPORTS = 'ALERTS_ATTACH_REPORTS',
ALLOW_FULL_CSV_EXPORT = 'ALLOW_FULL_CSV_EXPORT',
UX_BETA = 'UX_BETA',
GENERIC_CHART_AXES = 'GENERIC_CHART_AXES',
}
export type ScheduleQueriesProps = {
JSONSCHEMA: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ import {
PostProcessingResample,
QueryFormData,
} from '@superset-ui/core';
import {
rollingWindowOperator,
TIME_COLUMN,
} from '@superset-ui/chart-controls';
import { rollingWindowOperator } from '@superset-ui/chart-controls';

const TIME_GRAIN_MAP: Record<string, string> = {
PT1S: 'S',
Expand Down Expand Up @@ -65,7 +62,7 @@ export default function buildQuery(formData: QueryFormData) {
method: 'asfreq',
rule,
fill_value: null,
time_column: TIME_COLUMN,
time_column: DTTM_ALIAS,
},
};
}
Expand Down
Loading

0 comments on commit e9651ea

Please sign in to comment.