fix: echarts timeseries groupby (apache#11103)

* fix: echarts timeseries groupby * address review comment
auxten · Nov 20, 2020 · dae1e22 · dae1e22
1 parent 6578ce0
commit dae1e22
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 19 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git 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
@@ -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
@@ -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
@@ -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):