sadpandajoe commented on code in PR #42976:
URL: https://github.com/apache/superset/pull/42976#discussion_r3873217081
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -26,6 +27,92 @@
validate_column_args,
)
+_PERCENT_MODES = frozenset({"percent_row", "percent_col", "percent_total"})
+
+
+def _div_preserving_nan(numerator: DataFrame, denominator: Any, axis: int) ->
DataFrame:
+ """Divide ``numerator`` by ``denominator``, preserving NaN numerators.
+
+ A genuine SQL NULL numerator must stay NaN (rendered blank) rather than
+ become ``0.0`` — matches the client-side #42810 semantics guarding
+ against measured "0.0%" values for values that should stay blank.
+ """
+ result = numerator.div(denominator, axis=axis)
+ return result.mask(numerator.isna(), other=float("nan"))
+
+
+def _apply_percent_transform_to_group(g: DataFrame, mode: str) -> DataFrame:
+ """Apply a percent-of-{row,col,total} transform to a single-metric block.
+
+ Called both for the whole DataFrame when it holds one metric, and
+ per-metric-group for MultiIndex / flat-multi-metric pivots. A zero or
+ NaN denominator produces NaN cells rather than ``Infinity``/``NaN``
+ from division-by-zero, matching the client's ``if (acc === null)
+ return null`` guard in ``fractionOf``.
+ """
+ if mode == "percent_row":
+ row_totals = g.sum(axis=PandasAxis.COLUMN, skipna=True).replace(0,
float("nan"))
Review Comment:
Summing the already-aggregated cells does not produce the row rollup for
non-additive metrics. For example, category means `[5, 10]` from groups `[0,
10]` and `[10]` are normalized as 1/3 and 2/3 here, while the chart's DB rollup
uses mean `20/3`; exports will therefore disagree for AVG and similar metrics.
Can this use the requested rollup values or reject unsupported aggregations?
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -26,6 +27,92 @@
validate_column_args,
)
+_PERCENT_MODES = frozenset({"percent_row", "percent_col", "percent_total"})
+
+
+def _div_preserving_nan(numerator: DataFrame, denominator: Any, axis: int) ->
DataFrame:
+ """Divide ``numerator`` by ``denominator``, preserving NaN numerators.
+
+ A genuine SQL NULL numerator must stay NaN (rendered blank) rather than
+ become ``0.0`` — matches the client-side #42810 semantics guarding
+ against measured "0.0%" values for values that should stay blank.
+ """
+ result = numerator.div(denominator, axis=axis)
+ return result.mask(numerator.isna(), other=float("nan"))
Review Comment:
A present SQL NULL aggregated with `sum` has already become `0.0` by the
time this mask runs, so this returns a measured 0% instead of the blank the new
contract promises. Could the transform retain nullness before aggregation or
avoid claiming NULL preservation for this supported path?
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -26,6 +27,92 @@
validate_column_args,
)
+_PERCENT_MODES = frozenset({"percent_row", "percent_col", "percent_total"})
+
+
+def _div_preserving_nan(numerator: DataFrame, denominator: Any, axis: int) ->
DataFrame:
+ """Divide ``numerator`` by ``denominator``, preserving NaN numerators.
+
+ A genuine SQL NULL numerator must stay NaN (rendered blank) rather than
+ become ``0.0`` — matches the client-side #42810 semantics guarding
+ against measured "0.0%" values for values that should stay blank.
+ """
+ result = numerator.div(denominator, axis=axis)
+ return result.mask(numerator.isna(), other=float("nan"))
+
+
+def _apply_percent_transform_to_group(g: DataFrame, mode: str) -> DataFrame:
+ """Apply a percent-of-{row,col,total} transform to a single-metric block.
+
+ Called both for the whole DataFrame when it holds one metric, and
+ per-metric-group for MultiIndex / flat-multi-metric pivots. A zero or
+ NaN denominator produces NaN cells rather than ``Infinity``/``NaN``
+ from division-by-zero, matching the client's ``if (acc === null)
+ return null`` guard in ``fractionOf``.
+ """
+ if mode == "percent_row":
+ row_totals = g.sum(axis=PandasAxis.COLUMN, skipna=True).replace(0,
float("nan"))
+ return _div_preserving_nan(g, row_totals, axis=PandasAxis.ROW)
+ if mode == "percent_col":
+ col_totals = g.sum(axis=PandasAxis.ROW, skipna=True).replace(0,
float("nan"))
+ return _div_preserving_nan(g, col_totals, axis=PandasAxis.COLUMN)
+ # percent_total
+ grand = g.sum(skipna=True).sum(skipna=True)
+ if pd.isna(grand) or grand == 0:
+ return g * float("nan")
+ return _div_preserving_nan(g, grand, axis=PandasAxis.ROW)
+
+
+def _apply_show_values_as(df: DataFrame, mode: str) -> DataFrame:
+ """Divide each metric cell by the appropriate rollup total.
+
+ Mirrors the client-side ``fractionOf`` semantic in
+ ``plugin-chart-pivot-table/src/react-pivottable/utilities.ts:739``:
+
+ - ``percent_row``: cell / row-total (sum across the columns axis)
+ - ``percent_col``: cell / column-total (sum across the rows axis)
+ - ``percent_total``: cell / grand-total (sum of the metric block)
+
+ Per-metric isolation — the totals are computed *within each metric* so
+ one metric's numerator is never divided by another metric's total,
+ matching the client's ``metricAxis`` handling. This applies to both
+ shapes ``pivot_table`` can produce:
+
+ - **MultiIndex columns** (level 0 = metric): iterate the level-0
+ groups explicitly. Explicit iteration avoids the deprecated
+ ``df.groupby(level=0, axis=1)`` pattern (removed in pandas 3.x).
+ - **Flat columns with >1 column** (multi-metric pivot with no
+ ``columns`` groupby — each column IS a metric): treat each column
+ as its own single-column metric block.
+ - **Flat columns with 1 column** (single-metric pivot with no
+ ``columns`` groupby): the whole block is one metric.
+ """
+ is_multi_metric_wide = isinstance(df.columns, pd.MultiIndex)
+ is_flat_multi_metric = not is_multi_metric_wide and df.shape[1] > 1
+
+ if is_multi_metric_wide:
+ # Iterate level-0 groups explicitly (pandas-3-safe).
+ metrics = df.columns.get_level_values(0).unique()
+ parts = []
+ for metric in metrics:
+ block = df.xs(metric, axis=PandasAxis.COLUMN, level=0,
drop_level=False)
+ parts.append(_apply_percent_transform_to_group(block, mode))
+ # ``concat`` along columns preserves the MultiIndex; reorder to
+ # match the original column layout deterministically.
+ combined = pd.concat(parts, axis=PandasAxis.COLUMN)
Review Comment:
An empty result with a column grouping has an empty `MultiIndex`, leaving
`parts` empty and making `pd.concat` raise `ValueError: No objects to
concatenate`; the same pivot without this option returns an empty frame. Could
this return the empty frame unchanged (with a zero-row regression test)?
--
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]
