Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Fix] Returning timeseries_limit_metric in table viz get_data #9196

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 20 additions & 5 deletions superset/viz.py
Expand Up @@ -575,15 +575,30 @@ def get_data(self, df: pd.DataFrame) -> VizData:
the percent metrics have yet to be transformed.
"""

if not self.should_be_timeseries() and DTTM_ALIAS in df:
del df[DTTM_ALIAS]

non_percent_metric_columns = []
# Transform the data frame to adhere to the UI ordering of the columns and
# metrics whilst simultaneously computing the percentages (via normalization)
# for the percent metrics.
non_percent_metric_columns = (

if DTTM_ALIAS in df:
if self.should_be_timeseries():
non_percent_metric_columns.append(DTTM_ALIAS)
else:
del df[DTTM_ALIAS]

non_percent_metric_columns.extend(
self.form_data.get("all_columns") or self.form_data.get("groupby") or []
) + utils.get_metric_names(self.form_data.get("metrics") or [])
)

non_percent_metric_columns.extend(
utils.get_metric_names(self.form_data.get("metrics") or [])
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are lines 588 - 594 different to what was previously on lines 584 - 586?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just extending them separately because it feels a bit more readable, rather than extending list + list.


timeseries_limit_metric = utils.get_metric_name(
self.form_data.get("timeseries_limit_metric")
)
if timeseries_limit_metric:
non_percent_metric_columns.append(timeseries_limit_metric)

percent_metric_columns = utils.get_metric_names(
self.form_data.get("percent_metrics") or []
Expand Down
26 changes: 26 additions & 0 deletions tests/viz_tests.py
Expand Up @@ -407,6 +407,32 @@ def test_should_be_timeseries_raises_when_no_granularity(self):
with self.assertRaises(Exception):
test_viz.should_be_timeseries()

def test_adhoc_metric_with_sortby(self):
metrics = [
{
"expressionType": "SIMPLE",
"aggregate": "SUM",
"label": "sum_value",
"column": {"column_name": "value1", "type": "DOUBLE"},
}
]
form_data = {
"metrics": metrics,
"timeseries_limit_metric": {
"expressionType": "SIMPLE",
"aggregate": "SUM",
"label": "SUM(value1)",
"column": {"column_name": "value1", "type": "DOUBLE"},
},
"order_desc": False,
}

df = pd.DataFrame({"SUM(value1)": [15], "sum_value": [15],})
datasource = self.get_datasource_mock()
test_viz = viz.TableViz(datasource, form_data)
data = test_viz.get_data(df)
self.assertEqual(["sum_value", "SUM(value1)"], data["columns"])


class DistBarVizTestCase(SupersetTestCase):
def test_groupby_nulls(self):
Expand Down