geido commented on code in PR #43718:
URL: https://github.com/apache/superset/pull/43718#discussion_r3901588024
##########
superset/charts/client_processing.py:
##########
@@ -75,6 +83,361 @@ def get_column_key(label: tuple[str, ...], metrics:
list[str]) -> tuple[Any, ...
return tuple(parts)
+# How a metric's rollup total is derived from its cells, mirroring
+# `additiveReducerFor` in the pivot plugin's `plugin/utilities.ts`: SUM and
+# COUNT add up, MIN takes the lowest, MAX the highest. Everything else (saved
+# metrics, adhoc SQL, AVG, ...) is non-additive and has no correct answer at
+# this layer, so it falls back to summing the cells.
+_ROLLUP_REDUCERS: dict[str, str] = {"MIN": "min", "MAX": "max"}
+DEFAULT_ROLLUP_REDUCER = "sum"
+
+
+def split_grouping_sets_levels(
+ df: pd.DataFrame,
+) -> tuple[pd.DataFrame, dict[frozenset[str], pd.DataFrame]]:
+ """
+ Separate a GROUPING SETS result into its leaf frame and rollup levels.
+
+ A pivot chart with non-additive metrics asks for every rollup level in one
+ frame, tagging each row with a ``GROUPING()`` marker per groupby column
+ (see ``common/grouping_sets.py``): ``0`` where the column is grouped at
that
+ row's level, ``1`` where it has been rolled up. The rollup rows must not be
+ pivoted as ordinary rows -- their collapsed dimensions are NULL, so they
+ would add phantom rows and columns and inflate every denominator.
+
+ They are not discardable either: for a non-additive metric the database
+ rollup is the only correct total, and re-deriving one from the leaf cells
+ gives a different number (the mean of means, say, rather than the mean).
+ The chart divides by these values, so the export has to as well.
+
+ :return: the leaf frame, and each rollup level keyed by its grouped columns
+ """
+ markers = [
+ column
+ for column in df.columns
+ if isinstance(column, str) and column.endswith(GROUPING_MARKER_SUFFIX)
+ ]
+ if not markers:
+ return df, {}
+
+ grouped_of = {marker: marker[: -len(GROUPING_MARKER_SUFFIX)] for marker in
markers}
+ levels: dict[frozenset[str], pd.DataFrame] = {}
+ leaf = df
+ for keys, rows_at_level in df.groupby(markers, sort=False):
+ # `groupby` yields a scalar key for a single column and a tuple beyond.
+ marker_values = keys if isinstance(keys, tuple) else (keys,)
+ grouped = frozenset(
+ grouped_of[marker]
+ for marker, rolled_up in zip(markers, marker_values, strict=True)
+ if not rolled_up
+ )
+ level = rows_at_level.drop(columns=markers).reset_index(drop=True)
+ levels[grouped] = level
+ if len(grouped) == len(markers):
+ leaf = level
+ return leaf, levels
+
+
+def get_metric_rollup_reducers(
+ metrics: list[Any], verbose_map: Optional[dict[str, Any]] = None
+) -> dict[str, str]:
+ """Map each metric's label to the reducer that rolls its cells up."""
+ reducers: dict[str, str] = {}
+ for metric in metrics:
+ reducer = DEFAULT_ROLLUP_REDUCER
+ if isinstance(metric, dict) and metric.get("expressionType") ==
"SIMPLE":
+ reducer = _ROLLUP_REDUCERS.get(
+ metric.get("aggregate") or "", DEFAULT_ROLLUP_REDUCER
+ )
+ reducers[get_metric_name(metric, verbose_map)] = reducer
+ return reducers
+
+
+def _collapsed_metric(present: list[Any], metrics: list[str]) -> Any:
+ """
+ The metric a rollup spanning `present` stands for.
+
+ A total that collapses the metric axis is undefined in the renderer, which
+ resolves it to the last metric pushed into the shared slot (see the
+ ``metricAxis`` handling in ``react-pivottable/utilities.ts``). Mirror that
+ by taking the last metric in the configured order, so exported percentages
+ match the chart rather than summing metrics that share no unit.
+ """
+ distinct = set(present)
+ if len(distinct) == 1:
+ return distinct.pop()
+ for metric in reversed(metrics):
+ if metric in distinct:
+ return metric
+ return None
+
+
+def _metric_of_column(column: Any, metric_level: int) -> Any:
+ """The metric a pivoted column belongs to."""
+ return column[metric_level] if isinstance(column, tuple) else column
+
+
+def _reduce(
+ data: Union[pd.DataFrame, pd.Series],
+ reducer: str,
+ axis: Optional[int] = None,
+) -> Any:
+ """Apply a rollup reducer (``sum``/``min``/``max``), skipping empty
cells."""
+ method = getattr(data, reducer)
+ return method(axis=axis) if axis is not None else method()
+
+
+def _keyed_rollup(
+ frame: pd.DataFrame, dimensions: list[str]
+) -> dict[tuple[str, ...], dict[str, Any]]:
+ """Index a rollup level by its grouped dimension values, as strings."""
+ filled = frame.fillna("SUPERSET_PANDAS_NAN")
+ return {
+ tuple(str(record[dimension]) for dimension in dimensions): record
+ for record in filled.to_dict("records")
+ }
+
+
+def _apply_rollup_totals( # pylint: disable=too-many-arguments,too-many-locals
+ df: pd.DataFrame,
+ rows: list[str],
+ columns: list[str],
+ metrics: list[str],
+ rollup_levels: dict[frozenset[str], pd.DataFrame],
+ metric_level: int,
+ full_total_rows: list[Any],
+ full_total_columns: list[Any],
+) -> pd.DataFrame:
+ """
+ Replace whole-axis totals with the values the database computed.
+
+ The totals inserted above are reductions of the leaf cells, which is exact
+ only for an additive metric. Where the chart requested a rollup level, use
+ it, so that a total matches the chart and still divides by itself -- the
+ denominators come from the same levels.
+
+ Subtotals over a prefix of an axis are left as leaf reductions: the chart
+ gates those on their own toggles, so the matching level is not always
+ requested.
+ """
+ by_row = rollup_levels.get(frozenset(rows))
+ by_column = rollup_levels.get(frozenset(columns))
+ grand = rollup_levels.get(frozenset())
+ if grand is None:
+ return df
+
+ metric_names = set(metrics)
+
+ def metric_of(column: Any) -> Any:
+ name = _metric_of_column(column, metric_level)
+ return (
+ name if name in metric_names else _collapsed_metric(list(metrics),
metrics)
+ )
+
+ grand_record = next(iter(_keyed_rollup(grand, []).values()), {})
+
+ if by_row is not None and full_total_columns:
+ keyed = _keyed_rollup(by_row, rows)
+ for column in full_total_columns:
+ metric = metric_of(column)
+ df[column] = [
+ (keyed.get(tuple(str(part) for part in row)) or {}).get(metric)
+ for row in df.index
+ ]
Review Comment:
`full_total_columns` is the right set to write, but the row iteration isn't
scoped — it runs over the whole of `df.index`, including the prefix subtotal
rows that the `show_columns_total` block inserted above. A subtotal row's key
is `("EU", "Subtotal")`, which isn't in the by-row rollup, so `keyed.get(...)`
misses, `(... or {}).get(metric)` yields `None`, and the cell lands as NaN.
Instrumenting the call on the fixture in my summary comment:
```
--- BEFORE _apply_rollup_totals ---
AVG(num) Total (Sum)
gender boy girl Subtotal
EU UK 10 30 40 40
Subtotal 10 30 40 40
NA US 40 20 60 60
Subtotal 40 20 60 60
Total (Sum) 50 50 100 100
full_total_columns: [('Total (Sum)', ''), ('AVG(num)', 'Subtotal')]
--- AFTER _apply_rollup_totals ---
EU UK 10 30 18.0 18.0 <- database
rollup, correct
Subtotal 10 30 NaN NaN <- was 40, now
blank
NA US 40 20 25.0 25.0
Subtotal 40 20 NaN NaN <- was 60, now
blank
Total (Sum) 26 24 21.0 21.0
```
The docstring just above says subtotals over a prefix of an axis stay as
leaf reductions, which is what I'd expect — they just need to be excluded from
this write rather than looked up and missed. Restricting the comprehension to
rows absent from `inserted_rows` (keeping their existing value) would do it;
`pivot_df` already tracks that list.
##########
superset/charts/client_processing.py:
##########
@@ -75,6 +83,361 @@ def get_column_key(label: tuple[str, ...], metrics:
list[str]) -> tuple[Any, ...
return tuple(parts)
+# How a metric's rollup total is derived from its cells, mirroring
+# `additiveReducerFor` in the pivot plugin's `plugin/utilities.ts`: SUM and
+# COUNT add up, MIN takes the lowest, MAX the highest. Everything else (saved
+# metrics, adhoc SQL, AVG, ...) is non-additive and has no correct answer at
+# this layer, so it falls back to summing the cells.
+_ROLLUP_REDUCERS: dict[str, str] = {"MIN": "min", "MAX": "max"}
+DEFAULT_ROLLUP_REDUCER = "sum"
+
+
+def split_grouping_sets_levels(
+ df: pd.DataFrame,
+) -> tuple[pd.DataFrame, dict[frozenset[str], pd.DataFrame]]:
+ """
+ Separate a GROUPING SETS result into its leaf frame and rollup levels.
+
+ A pivot chart with non-additive metrics asks for every rollup level in one
+ frame, tagging each row with a ``GROUPING()`` marker per groupby column
+ (see ``common/grouping_sets.py``): ``0`` where the column is grouped at
that
+ row's level, ``1`` where it has been rolled up. The rollup rows must not be
+ pivoted as ordinary rows -- their collapsed dimensions are NULL, so they
+ would add phantom rows and columns and inflate every denominator.
+
+ They are not discardable either: for a non-additive metric the database
+ rollup is the only correct total, and re-deriving one from the leaf cells
+ gives a different number (the mean of means, say, rather than the mean).
+ The chart divides by these values, so the export has to as well.
+
+ :return: the leaf frame, and each rollup level keyed by its grouped columns
+ """
+ markers = [
+ column
+ for column in df.columns
+ if isinstance(column, str) and column.endswith(GROUPING_MARKER_SUFFIX)
+ ]
+ if not markers:
+ return df, {}
+
+ grouped_of = {marker: marker[: -len(GROUPING_MARKER_SUFFIX)] for marker in
markers}
+ levels: dict[frozenset[str], pd.DataFrame] = {}
+ leaf = df
+ for keys, rows_at_level in df.groupby(markers, sort=False):
+ # `groupby` yields a scalar key for a single column and a tuple beyond.
+ marker_values = keys if isinstance(keys, tuple) else (keys,)
+ grouped = frozenset(
+ grouped_of[marker]
+ for marker, rolled_up in zip(markers, marker_values, strict=True)
+ if not rolled_up
+ )
+ level = rows_at_level.drop(columns=markers).reset_index(drop=True)
+ levels[grouped] = level
+ if len(grouped) == len(markers):
+ leaf = level
+ return leaf, levels
+
+
+def get_metric_rollup_reducers(
+ metrics: list[Any], verbose_map: Optional[dict[str, Any]] = None
+) -> dict[str, str]:
+ """Map each metric's label to the reducer that rolls its cells up."""
+ reducers: dict[str, str] = {}
+ for metric in metrics:
+ reducer = DEFAULT_ROLLUP_REDUCER
+ if isinstance(metric, dict) and metric.get("expressionType") ==
"SIMPLE":
+ reducer = _ROLLUP_REDUCERS.get(
+ metric.get("aggregate") or "", DEFAULT_ROLLUP_REDUCER
+ )
+ reducers[get_metric_name(metric, verbose_map)] = reducer
+ return reducers
+
+
+def _collapsed_metric(present: list[Any], metrics: list[str]) -> Any:
+ """
+ The metric a rollup spanning `present` stands for.
+
+ A total that collapses the metric axis is undefined in the renderer, which
+ resolves it to the last metric pushed into the shared slot (see the
+ ``metricAxis`` handling in ``react-pivottable/utilities.ts``). Mirror that
+ by taking the last metric in the configured order, so exported percentages
+ match the chart rather than summing metrics that share no unit.
+ """
+ distinct = set(present)
+ if len(distinct) == 1:
+ return distinct.pop()
+ for metric in reversed(metrics):
+ if metric in distinct:
+ return metric
+ return None
+
+
+def _metric_of_column(column: Any, metric_level: int) -> Any:
+ """The metric a pivoted column belongs to."""
+ return column[metric_level] if isinstance(column, tuple) else column
+
+
+def _reduce(
+ data: Union[pd.DataFrame, pd.Series],
+ reducer: str,
+ axis: Optional[int] = None,
+) -> Any:
+ """Apply a rollup reducer (``sum``/``min``/``max``), skipping empty
cells."""
+ method = getattr(data, reducer)
+ return method(axis=axis) if axis is not None else method()
+
+
+def _keyed_rollup(
+ frame: pd.DataFrame, dimensions: list[str]
+) -> dict[tuple[str, ...], dict[str, Any]]:
+ """Index a rollup level by its grouped dimension values, as strings."""
+ filled = frame.fillna("SUPERSET_PANDAS_NAN")
+ return {
+ tuple(str(record[dimension]) for dimension in dimensions): record
+ for record in filled.to_dict("records")
+ }
+
+
+def _apply_rollup_totals( # pylint: disable=too-many-arguments,too-many-locals
+ df: pd.DataFrame,
+ rows: list[str],
+ columns: list[str],
+ metrics: list[str],
+ rollup_levels: dict[frozenset[str], pd.DataFrame],
+ metric_level: int,
+ full_total_rows: list[Any],
+ full_total_columns: list[Any],
+) -> pd.DataFrame:
+ """
+ Replace whole-axis totals with the values the database computed.
+
+ The totals inserted above are reductions of the leaf cells, which is exact
+ only for an additive metric. Where the chart requested a rollup level, use
+ it, so that a total matches the chart and still divides by itself -- the
+ denominators come from the same levels.
+
+ Subtotals over a prefix of an axis are left as leaf reductions: the chart
+ gates those on their own toggles, so the matching level is not always
+ requested.
+ """
+ by_row = rollup_levels.get(frozenset(rows))
+ by_column = rollup_levels.get(frozenset(columns))
+ grand = rollup_levels.get(frozenset())
+ if grand is None:
+ return df
+
+ metric_names = set(metrics)
+
+ def metric_of(column: Any) -> Any:
+ name = _metric_of_column(column, metric_level)
+ return (
+ name if name in metric_names else _collapsed_metric(list(metrics),
metrics)
+ )
+
+ grand_record = next(iter(_keyed_rollup(grand, []).values()), {})
+
+ if by_row is not None and full_total_columns:
+ keyed = _keyed_rollup(by_row, rows)
+ for column in full_total_columns:
+ metric = metric_of(column)
+ df[column] = [
+ (keyed.get(tuple(str(part) for part in row)) or {}).get(metric)
+ for row in df.index
+ ]
+
+ if by_column is not None and full_total_rows:
+ keyed = _keyed_rollup(by_column, columns)
+ for row in full_total_rows:
+ values = []
+ for column in df.columns:
+ metric = metric_of(column)
+ if column in full_total_columns:
+ values.append(grand_record.get(metric))
+ continue
+ key = tuple(
+ str(part)
+ for index, part in enumerate(column)
+ if index != (metric_level % len(column))
+ )
+ values.append((keyed.get(key) or {}).get(metric))
+ df.loc[row] = values
+
+ return df
+
+
+def _rollup_denominators( # pylint: disable=too-many-arguments,too-many-locals
+ df: pd.DataFrame,
+ mode: str,
+ rows: list[str],
+ columns: list[str],
+ metrics: list[str],
+ rollup_levels: dict[frozenset[str], pd.DataFrame],
+ metric_level: int,
+ inserted_rows: list[Any],
+ inserted_columns: list[Any],
+) -> Optional[pd.DataFrame]:
+ """
+ Each cell's denominator, taken from the database-computed rollup levels.
+
+ A percent mode makes the chart request the rollup level its denominator
+ needs -- all rows with the columns collapsed for "% of row", the reverse
for
+ "% of column", both collapsed for "% of total" (see
+ ``buildGroupbyCombinations``). Those values are the totals the database
+ computed, so for a non-additive metric they are the only correct ones.
+
+ Returns ``None`` when the level is absent, leaving the caller to derive the
+ denominators from the leaf cells instead: additive metrics never request
+ rollup levels in the first place, and re-deriving is exact for them.
+ """
+ grouped_for_mode = {
+ ShowValuesAs.PERCENT_OF_ROW: rows,
+ ShowValuesAs.PERCENT_OF_COLUMN: columns,
+ ShowValuesAs.PERCENT_OF_TOTAL: [],
+ }[ShowValuesAs(mode)]
+ level = rollup_levels.get(frozenset(grouped_for_mode))
+ grand = rollup_levels.get(frozenset())
+ if level is None or grand is None:
+ return None
+
+ by_key = _keyed_rollup(level, grouped_for_mode)
+ grand_record = next(iter(_keyed_rollup(grand, []).values()), {})
+
+ def metric_of(column: Any) -> Any:
+ name = _metric_of_column(column, metric_level)
+ return (
+ name if name in set(metrics) else _collapsed_metric(list(metrics),
metrics)
+ )
+
+ def denominator(row: Any, column: Any) -> Any:
+ metric = metric_of(column)
+ # A total collapses its axis entirely, so it divides by the grand
total.
+ collapsed = (
+ row in inserted_rows
+ if mode == ShowValuesAs.PERCENT_OF_ROW
+ else column in inserted_columns
+ )
+ if mode == ShowValuesAs.PERCENT_OF_TOTAL or collapsed:
+ return grand_record.get(metric)
Review Comment:
This is the second half of the subtotal problem. `collapsed` tests
membership in `inserted_rows` / `inserted_columns`, which hold every inserted
level, not just the whole-axis one. So a prefix subtotal row is treated as
collapsing its entire axis and gets the grand-total denominator.
The denominator frame this builds, on the fixture from my summary comment:
```
AVG(num) Total (Sum)
gender boy girl Subtotal
EU UK 18.0 18.0 18.0 18.0 <- its own
rollup, correct
Subtotal 21.0 21.0 21.0 21.0 <- grand total;
EU's own rollup is 17
NA US 25.0 25.0 25.0 25.0
Subtotal 21.0 21.0 21.0 21.0 <- grand total;
NA's own rollup is 24
Total (Sum) 21.0 21.0 21.0 21.0 <- correct, this
one really is whole-axis
```
So `EU / Subtotal / boy` reads `10/21 = 47.6%` where the chart divides by
the `{region}` rollup and shows `10/17 = 58.8%`.
Worth noting the correct value is already in hand:
`buildGroupbyCombinations` emits the `{region}` level whenever `rowSubTotals`
is on, so `rollup_levels[frozenset(["region"])]` is sitting right there with 17
and 24 in it — it's just never consulted. `full_total_rows` /
`full_total_columns` look like the sets you want for `collapsed`, with prefix
subtotals falling through to a lookup against their own prefix level.
##########
tests/unit_tests/charts/test_client_processing.py:
##########
@@ -1804,6 +1805,371 @@ def test_pivot_df_complex_null_values():
)
+# --- `showValuesAs` percent modes (#42809) -----------------------------------
+#
+# Exports and scheduled reports render server-side, so they have to reproduce
+# the client's `fractionOf` aggregator: each cell over its row, column, or
grand
+# total, computed per metric, with totals dividing by their own rollup rather
+# than summing the fractions around them.
+
+SHOW_VALUES_AS_OPTIONS: dict[str, Any] = {
+ "rows": ["nation"],
+ "columns": ["gender"],
+ "metrics": ["SUM(num)"],
+ "aggfunc": "Sum",
+ "transpose_pivot": False,
+ "combine_metrics": False,
+ "show_rows_total": True,
+ "show_columns_total": True,
+ "apply_metrics_on_rows": False,
+}
+
+
+def show_values_as_df() -> pd.DataFrame:
+ """A 2x2 pivot: row totals 40/40, column totals 30/50, grand total 80."""
+ return pd.DataFrame(
+ {
+ "nation": ["US", "US", "UK", "UK"],
+ "gender": ["boy", "girl", "boy", "girl"],
+ "SUM(num)": [10, 30, 20, 20],
+ }
+ )
+
+
+def total_label() -> str:
+ return f"{_('Total')} (Sum)"
+
+
+def test_pivot_df_show_values_as_percent_row():
+ pivoted = pivot_df(
+ show_values_as_df(), **SHOW_VALUES_AS_OPTIONS,
show_values_as="percent_row"
+ )
+ total = total_label()
+
+ assert pivoted.loc[("US",), ("SUM(num)", "boy")] == 0.25
+ assert pivoted.loc[("US",), ("SUM(num)", "girl")] == 0.75
+ assert pivoted.loc[("UK",), ("SUM(num)", "boy")] == 0.5
+ # a row is always 100% of itself
+ assert pivoted.loc[("US",), (total, "")] == 1
+ # the totals row shows each column's share of the grand total (30/80 and
+ # 50/80), not the sum of the fractions above it
+ assert pivoted.loc[(total,), ("SUM(num)", "boy")] == 0.375
+ assert pivoted.loc[(total,), ("SUM(num)", "girl")] == 0.625
+ assert pivoted.loc[(total,), (total, "")] == 1
+
+
+def test_pivot_df_show_values_as_percent_col():
+ pivoted = pivot_df(
+ show_values_as_df(), **SHOW_VALUES_AS_OPTIONS,
show_values_as="percent_col"
+ )
+ total = total_label()
+
+ assert pivoted.loc[("US",), ("SUM(num)", "boy")] == pytest.approx(1 / 3)
+ assert pivoted.loc[("UK",), ("SUM(num)", "boy")] == pytest.approx(2 / 3)
+ assert pivoted.loc[("US",), ("SUM(num)", "girl")] == 0.6
+ # a column is always 100% of itself
+ assert pivoted.loc[(total,), ("SUM(num)", "boy")] == 1
+ # the totals column shows each row's share of the grand total
+ assert pivoted.loc[("US",), (total, "")] == 0.5
+
+
+def test_pivot_df_show_values_as_percent_total():
+ pivoted = pivot_df(
+ show_values_as_df(), **SHOW_VALUES_AS_OPTIONS,
show_values_as="percent_total"
+ )
+ total = total_label()
+
+ assert pivoted.loc[("US",), ("SUM(num)", "boy")] == 0.125
+ assert pivoted.loc[("US",), ("SUM(num)", "girl")] == 0.375
+ assert pivoted.loc[("UK",), ("SUM(num)", "boy")] == 0.25
+ assert pivoted.loc[(total,), ("SUM(num)", "boy")] == 0.375
+ assert pivoted.loc[("US",), (total, "")] == 0.5
+ assert pivoted.loc[(total,), (total, "")] == 1
+
+
+def test_pivot_df_show_values_as_keeps_metrics_separate():
+ """One metric's cells are never divided by another metric's total."""
+ df = show_values_as_df()
+ df["MAX(num)"] = [1, 3, 6, 10]
+ pivoted = pivot_df(
+ df,
+ **{**SHOW_VALUES_AS_OPTIONS, "metrics": ["SUM(num)", "MAX(num)"]},
+ show_values_as="percent_row",
+ )
+
+ assert pivoted.loc[("US",), ("SUM(num)", "boy")] == 0.25
+ assert pivoted.loc[("UK",), ("MAX(num)", "boy")] == 0.375
+ assert pivoted.loc[("UK",), ("MAX(num)", "girl")] == 0.625
+ # a total collapsing the metric axis resolves to one metric, as the
+ # renderer does, so it still divides by itself
+ assert pivoted.loc[("US",), (total_label(), "")] == 1
+
+
+def test_pivot_df_show_values_as_with_combined_metrics():
+ """`combineMetric` moves the metric to the lowest column level."""
+ df = show_values_as_df()
+ df["MAX(num)"] = [1, 3, 6, 10]
+ pivoted = pivot_df(
+ df,
+ **{
+ **SHOW_VALUES_AS_OPTIONS,
+ "metrics": ["SUM(num)", "MAX(num)"],
+ "combine_metrics": True,
+ "show_rows_total": False,
+ "show_columns_total": False,
+ },
+ show_values_as="percent_row",
+ )
+
+ assert pivoted.loc[("US",), ("boy", "SUM(num)")] == 0.25
+ assert pivoted.loc[("US",), ("girl", "SUM(num)")] == 0.75
+ # each metric keeps its own denominator across the combined layout
+ assert pivoted.loc[("UK",), ("boy", "MAX(num)")] == 0.375
+ assert pivoted.loc[("UK",), ("girl", "MAX(num)")] == 0.625
+
+
+def test_pivot_table_v2_show_values_as_uses_min_max_rollups():
+ """A MIN/MAX metric divides by the row's extreme, not its sum.
+
+ Mirrors `additiveReducerFor` in the plugin's `plugin/utilities.ts`: the
+ chart rolls a MAX metric up with max, so a row of [6, 10] reads 60%/100%.
+ """
+ df = pd.DataFrame(
+ {
+ "nation": ["US", "US", "UK", "UK"],
+ "gender": ["boy", "girl", "boy", "girl"],
+ "MAX(num)": [1, 3, 6, 10],
+ }
+ )
+ form_data = {
+ "groupbyRows": ["nation"],
+ "groupbyColumns": ["gender"],
+ "metrics": [
+ {
+ "expressionType": "SIMPLE",
+ "aggregate": "MAX",
+ "column": {"column_name": "num"},
+ "label": "MAX(num)",
+ }
+ ],
+ "showValuesAs": "percent_row",
+ "rowTotals": True,
+ }
+
+ pivoted = pivot_table_v2(df, form_data, apply_number_format=False)
+
+ assert pivoted.loc[("UK",), ("MAX(num)", "boy")] == 0.6
+ assert pivoted.loc[("UK",), ("MAX(num)", "girl")] == 1
+ # the total divides by itself whatever the reducer
+ assert pivoted.loc[("UK",), (total_label(), "")] == 1
+
+
+def grouping_sets_df() -> pd.DataFrame:
+ """A GROUPING SETS result whose rollups differ from any leaf reduction.
+
+ Leaves are 10 and 20, so a leaf-derived total would be 30 (sum) or 15
+ (mean). The database rollups are deliberately none of those: 18 down the
+ column, 11/21 across the rows, 19 overall -- as a weighted average or a
+ distinct count would be.
+ """
+ return pd.DataFrame(
+ {
+ "nation": ["US", "UK", None, "US", "UK", None],
+ "gender": ["boy", "boy", "boy", None, None, None],
+ "nation__superset_grouping": [0, 0, 1, 0, 0, 1],
+ "gender__superset_grouping": [0, 0, 0, 1, 1, 1],
+ "AVG(num)": [10, 20, 18, 11, 21, 19],
+ }
+ )
+
+
Review Comment:
Every rollup test here — `..._pivots_only_grouping_sets_leaf_rows`,
`..._divides_by_database_rollups`, `..._rollup_totals_divide_by_themselves` —
uses a single `groupbyRows` entry, so no prefix subtotal row is ever inserted
alongside a rollup frame. That's why CI is green on the two issues I flagged
above.
A `nested_grouping_sets_df` variant with two row dimensions (fixture in my
summary comment) would cover it. The assertions that currently fail:
```python
def test_row_subtotal_keeps_its_total_column_cell():
pivoted = pivot_table_v2(nested_grouping_sets_df(), form_data,
apply_number_format=False)
assert not pd.isna(pivoted.loc[("EU", "Subtotal"), (total_label(), "")])
# got nan
def test_row_subtotal_divides_by_its_own_rollup():
pivoted = pivot_table_v2(nested_grouping_sets_df(), form_data,
apply_number_format=False)
assert pivoted.loc[("EU", "Subtotal"), ("AVG(num)", "boy")] ==
pytest.approx(10 / 17)
# got 0.476190 (= 10/21, the grand total)
```
Alongside two controls that pass on the current head, and are worth keeping
as regression guards: leaf rows still divide by their database rollups
(`10/18`, `40/25`, totals at 1.0), and the marker-free additive path is
untouched.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
