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.

Sign up for GitHub

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

fix: take form_data reference for metrics for pivot_v2 table reports #21029

Merged
merged 8 commits into from
Aug 11, 2022
Merged

fix: take form_data reference for metrics for pivot_v2 table reports #21029

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6f1d516
take form_data reference for form_data
hughhhh Aug 9, 2022
0c261f9
better
hughhhh Aug 10, 2022
8ae9c9c
remove comments
hughhhh Aug 10, 2022
756cdff
remove comments
hughhhh Aug 10, 2022
b4a9919
Update superset/charts/post_processing.py
hughhhh Aug 10, 2022
deb237d
add test
hughhhh Aug 10, 2022
f37bac5
Merge branch 'fix-pivot-reports' of https://github.com/apache/superse…
hughhhh Aug 10, 2022
26fa175
test
hughhhh Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions superset/charts/post_processing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ def apply_post_process(
elif query["result_format"] == ChartDataResultFormat.CSV:
df = pd.read_csv(StringIO(query["data"]))

# convert all columns to verbose (label) name
if datasource:
df.rename(columns=datasource.data["verbose_map"], inplace=True)

processed_df = post_processor(df, form_data, datasource)

query["colnames"] = list(processed_df.columns)
Expand Down
62 changes: 62 additions & 0 deletions tests/unit_tests/charts/test_post_processing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import pandas as pd
from numpy import True_
from pytest import raises
from sqlalchemy.orm.session import Session

from superset.charts.post_processing import apply_post_process, pivot_df, table
from superset.common.chart_data import ChartDataResultFormat
Expand Down Expand Up @@ -1960,3 +1961,64 @@ def test_apply_post_process_json_format_data_is_none():
assert apply_post_process(result, form_data) == {
"queries": [{"result_format": ChartDataResultFormat.JSON, "data": None}]
}


def test_apply_post_process_verbose_map(session: Session):
from superset.connectors.sqla.models import SqlaTable, SqlMetric
from superset.models.core import Database

engine = session.get_bind()
SqlaTable.metadata.create_all(engine) # pylint: disable=no-member
db = Database(database_name="my_database", sqlalchemy_uri="sqlite://")
sqla_table = SqlaTable(
table_name="my_sqla_table",
columns=[],
metrics=[
SqlMetric(
metric_name="count",
verbose_name="COUNT(*)",
metric_type="count",
expression="COUNT(*)",
)
],
database=db,
)

result = {
"queries": [
{
"result_format": ChartDataResultFormat.JSON,
"data": [{"count": 4725}],
}
]
}
form_data = {
"datasource": "19__table",
"viz_type": "pivot_table_v2",
"slice_id": 69,
"url_params": {},
"granularity_sqla": "time_start",
"time_grain_sqla": "P1D",
"time_range": "No filter",
"groupbyColumns": [],
"groupbyRows": [],
"metrics": ["COUNT(*)"],
"metricsLayout": "COLUMNS",
"row_limit": 10000,
"order_desc": True,
"valueFormat": "SMART_NUMBER",
"date_format": "smart_date",
"rowOrder": "key_a_to_z",
"colOrder": "key_a_to_z",
"extra_form_data": {},
"force": False,
"result_format": "json",
"result_type": "results",
}

assert (
"COUNT(*)"
in apply_post_process(result, form_data, datasource=sqla_table)["queries"][0][
"data"
].keys()
)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I would make this assertion more generic, since it might be useful to capture future regressions in other parts of the code. Something like:

assert apply_post_process(result, form_data, datasource=sqla_table) = {
    "queries": [
        {
            "data": { "COUNT(*)": ...,  ... },
        },
    ]
)