Skip to content

Commit

Permalink
fix: rolling and cum operator in multiple series chart (apache#1386)
Browse files Browse the repository at this point in the history
* fix: rolling and cum operator in multiple series chart

* fix lint

* fix UT
  • Loading branch information
zhaoyongjie committed Nov 24, 2021
1 parent 9af2e4c commit 61bb6e8
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const rollingWindowOperator: PostProcessingFactory<
// time compare type: actual values
columns = [...Array.from(metricsMap.values()), ...Array.from(metricsMap.keys())];
} else {
// time compare type: absolute / percentage / ratio
// time compare type: difference / percentage / ratio
columns = Array.from(metricsMap.entries()).map(([offset, metric]) =>
[comparisonType, metric, offset].join(TIME_COMPARISON_SEPARATOR),
);
Expand All @@ -60,6 +60,7 @@ export const rollingWindowOperator: PostProcessingFactory<
options: {
operator: 'sum',
columns: columnsMap,
is_pivot_df: true,
},
};
}
Expand All @@ -72,6 +73,7 @@ export const rollingWindowOperator: PostProcessingFactory<
window: ensureIsInt(formData.rolling_periods, 1),
min_periods: ensureIsInt(formData.min_periods, 0),
columns: columnsMap,
is_pivot_df: true,
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ test('rolling_type: cumsum', () => {
'count(*)': 'count(*)',
'sum(val)': 'sum(val)',
},
is_pivot_df: true,
},
});
});
Expand All @@ -86,6 +87,7 @@ test('rolling_type: sum/mean/std', () => {
'count(*)': 'count(*)',
'sum(val)': 'sum(val)',
},
is_pivot_df: true,
},
});
});
Expand Down Expand Up @@ -114,6 +116,7 @@ test('rolling window and "actual values" in the time compare', () => {
'sum(val)__1 year ago': 'sum(val)__1 year ago',
'sum(val)__1 year later': 'sum(val)__1 year later',
},
is_pivot_df: true,
},
});
});
Expand Down Expand Up @@ -141,6 +144,7 @@ test('rolling window and "difference / percentage / ratio" in the time compare',
[`${cType}__sum(val)__sum(val)__1 year ago`]: `${cType}__sum(val)__sum(val)__1 year ago`,
[`${cType}__sum(val)__sum(val)__1 year later`]: `${cType}__sum(val)__sum(val)__1 year later`,
},
is_pivot_df: true,
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ export interface PostProcessingPivot {
index: string[];
columns: string[];
aggregates: Aggregates;
flatten_columns?: boolean;
reset_index?: boolean;
};
}

Expand Down Expand Up @@ -128,6 +130,7 @@ export interface PostProcessingRolling {
window: number;
min_periods: number;
columns: string[];
is_pivot_df?: boolean;
};
}

Expand All @@ -136,6 +139,7 @@ export interface PostProcessingCum {
options: {
columns: string[];
operator: NumpyFunction;
is_pivot_df?: boolean;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
import { buildQueryContext, QueryFormData, normalizeOrderBy } from '@superset-ui/core';
import {
buildQueryContext,
QueryFormData,
normalizeOrderBy,
RollingType,
PostProcessingPivot,
} from '@superset-ui/core';
import {
rollingWindowOperator,
timeCompareOperator,
Expand All @@ -29,22 +35,39 @@ import {
} from '@superset-ui/chart-controls';

export default function buildQuery(formData: QueryFormData) {
return buildQueryContext(formData, baseQueryObject => [
{
return buildQueryContext(formData, baseQueryObject => {
const pivotOperatorInRuntime: PostProcessingPivot | undefined = pivotOperator(formData, {
...baseQueryObject,
is_timeseries: true,
// todo: move `normalizeOrderBy to extractQueryFields`
orderby: normalizeOrderBy(baseQueryObject).orderby,
time_offsets: isValidTimeCompare(formData, baseQueryObject) ? formData.time_compare : [],
post_processing: [
resampleOperator(formData, baseQueryObject),
timeCompareOperator(formData, baseQueryObject),
sortOperator(formData, { ...baseQueryObject, is_timeseries: true }),
rollingWindowOperator(formData, baseQueryObject),
pivotOperator(formData, { ...baseQueryObject, is_timeseries: true }),
contributionOperator(formData, baseQueryObject),
prophetOperator(formData, baseQueryObject),
],
},
]);
});
if (pivotOperatorInRuntime && Object.values(RollingType).includes(formData.rolling_type)) {
pivotOperatorInRuntime.options = {
...pivotOperatorInRuntime.options,
...{
flatten_columns: false,
reset_index: false,
},
};
}

return [
{
...baseQueryObject,
is_timeseries: true,
// todo: move `normalizeOrderBy to extractQueryFields`
orderby: normalizeOrderBy(baseQueryObject).orderby,
time_offsets: isValidTimeCompare(formData, baseQueryObject) ? formData.time_compare : [],
post_processing: [
resampleOperator(formData, baseQueryObject),
timeCompareOperator(formData, baseQueryObject),
sortOperator(formData, { ...baseQueryObject, is_timeseries: true }),
// in order to be able to rolling in multiple series, must do pivot before rollingOperator
pivotOperatorInRuntime,
rollingWindowOperator(formData, baseQueryObject),
contributionOperator(formData, baseQueryObject),
prophetOperator(formData, baseQueryObject),
],
},
];
});
}

0 comments on commit 61bb6e8

Please sign in to comment.