From 80e395559d2403c1f1fcd43a5af718ede90f80b9 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Date: Tue, 29 Sep 2020 14:25:38 +0300 Subject: [PATCH] fix: echarts timeseries groupby (#11103) * fix: echarts timeseries groupby * address review comment --- superset-frontend/package-lock.json | 6 +++--- superset-frontend/package.json | 2 +- superset/charts/schemas.py | 15 ++++++++++--- superset/utils/pandas_postprocessing.py | 13 ++++++------ tests/pandas_postprocessing_tests.py | 28 ++++++++++++++++++++----- 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index e72abe1b85d0c..d7e6825514a79 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -15766,9 +15766,9 @@ } }, "@superset-ui/plugin-chart-echarts": { - "version": "0.15.3", - "resolved": "https://registry.npmjs.org/@superset-ui/plugin-chart-echarts/-/plugin-chart-echarts-0.15.3.tgz", - "integrity": "sha512-kzELNmzIJr1KASYhuz1LkslFJ7XA16lid4DxlHlEGEw4Gmm8h0bl8KXnFeNbSpJHh95Qdfwrf2b7bmB6qmVKHg==", + "version": "0.15.4", + "resolved": "https://registry.npmjs.org/@superset-ui/plugin-chart-echarts/-/plugin-chart-echarts-0.15.4.tgz", + "integrity": "sha512-71YDoLH/Jx7HSy4AHekQk1t9G6aHXN/mZjvEeLXVxmP2wWo3X/rG2bpjzd7WP6BllZ92kUlFaGuoWRWMcBbfHw==", "requires": { "@superset-ui/chart-controls": "0.15.3", "@superset-ui/core": "0.15.2", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index a65cd9233c008..5eec961dc5d4f 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -87,7 +87,7 @@ "@superset-ui/legacy-preset-chart-big-number": "^0.15.3", "@superset-ui/legacy-preset-chart-deckgl": "^0.3.1", "@superset-ui/legacy-preset-chart-nvd3": "^0.15.3", - "@superset-ui/plugin-chart-echarts": "^0.15.3", + "@superset-ui/plugin-chart-echarts": "^0.15.4", "@superset-ui/plugin-chart-table": "^0.15.3", "@superset-ui/plugin-chart-word-cloud": "^0.15.3", "@superset-ui/preset-chart-xy": "^0.15.3", diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py index 5500271c50119..58012a820a879 100644 --- a/superset/charts/schemas.py +++ b/superset/charts/schemas.py @@ -718,6 +718,7 @@ class ChartDataQueryObjectSchema(Schema): ) groupby = fields.List( fields.String(description="Columns by which to group the query.",), + allow_none=True, ) metrics = fields.List( fields.Raw(), @@ -781,12 +782,20 @@ class ChartDataQueryObjectSchema(Schema): order_desc = fields.Boolean( description="Reverse order. Default: `false`", required=False ) - extras = fields.Nested(ChartDataExtrasSchema, required=False) - columns = fields.List(fields.String(), description="",) + extras = fields.Nested( + ChartDataExtrasSchema, + description="Extra parameters to add to the query.", + required=False, + ) + columns = fields.List( + fields.String(), + description="Columns which to select in the query.", + allow_none=True, + ) orderby = fields.List( fields.List(fields.Raw()), description="Expects a list of lists where the first element is the column " - "name which to sort by, and the second element is a boolean ", + "name which to sort by, and the second element is a boolean.", example=[["my_col_1", False], ["my_col_2", True]], ) where = fields.String( diff --git a/superset/utils/pandas_postprocessing.py b/superset/utils/pandas_postprocessing.py index 73336ebdb7500..d73bee6a74f54 100644 --- a/superset/utils/pandas_postprocessing.py +++ b/superset/utils/pandas_postprocessing.py @@ -21,7 +21,7 @@ import numpy as np from flask_babel import gettext as _ from geopy.point import Point -from pandas import DataFrame, NamedAgg, Series +from pandas import DataFrame, NamedAgg, Series, Timestamp from superset.exceptions import QueryObjectValidationError from superset.utils.core import DTTM_ALIAS, PostProcessingContributionOrientation @@ -93,7 +93,8 @@ def _flatten_column_after_pivot( - column: Union[str, Tuple[str, ...]], aggregates: Dict[str, Dict[str, Any]] + column: Union[float, Timestamp, str, Tuple[str, ...]], + aggregates: Dict[str, Dict[str, Any]], ) -> str: """ Function for flattening column names into a single string. This step is necessary @@ -106,15 +107,13 @@ def _flatten_column_after_pivot( :param aggregates: aggregates :return: """ - if isinstance(column, str): - return column - if len(column) == 1: - return column[0] + if not isinstance(column, tuple): + column = (column,) if len(aggregates) == 1 and len(column) > 1: # drop aggregate for single aggregate pivots with multiple groupings # from column name (aggregates always come first in column name) column = column[1:] - return ", ".join(column) + return ", ".join([str(col) for col in column]) def validate_column_args(*argnames: str) -> Callable[..., Any]: diff --git a/tests/pandas_postprocessing_tests.py b/tests/pandas_postprocessing_tests.py index 479df423c6357..fabe84bc286ab 100644 --- a/tests/pandas_postprocessing_tests.py +++ b/tests/pandas_postprocessing_tests.py @@ -19,7 +19,7 @@ import math from typing import Any, List, Optional -from pandas import DataFrame, Series +from pandas import DataFrame, Series, Timestamp import pytest from superset.exceptions import QueryObjectValidationError @@ -77,6 +77,24 @@ def test_flatten_column_after_pivot(self): ), "idx_nulls", ) + self.assertEqual( + proc._flatten_column_after_pivot( + aggregates=AGGREGATES_SINGLE, column=1234, + ), + "1234", + ) + self.assertEqual( + proc._flatten_column_after_pivot( + aggregates=AGGREGATES_SINGLE, column=Timestamp("2020-09-29T00:00:00"), + ), + "2020-09-29 00:00:00", + ) + self.assertEqual( + proc._flatten_column_after_pivot( + aggregates=AGGREGATES_SINGLE, column="idx_nulls", + ), + "idx_nulls", + ) self.assertEqual( proc._flatten_column_after_pivot( aggregates=AGGREGATES_SINGLE, column=("idx_nulls", "col1"), @@ -85,9 +103,9 @@ def test_flatten_column_after_pivot(self): ) self.assertEqual( proc._flatten_column_after_pivot( - aggregates=AGGREGATES_SINGLE, column=("idx_nulls", "col1", "col2"), + aggregates=AGGREGATES_SINGLE, column=("idx_nulls", "col1", 1234), ), - "col1, col2", + "col1, 1234", ) # Multiple aggregate cases @@ -100,9 +118,9 @@ def test_flatten_column_after_pivot(self): self.assertEqual( proc._flatten_column_after_pivot( aggregates=AGGREGATES_MULTIPLE, - column=("idx_nulls", "asc_idx", "col1", "col2"), + column=("idx_nulls", "asc_idx", "col1", 1234), ), - "idx_nulls, asc_idx, col1, col2", + "idx_nulls, asc_idx, col1, 1234", ) def test_pivot_without_columns(self):