Abdulrehman-PIAIC80387 commented on code in PR #42976:
URL: https://github.com/apache/superset/pull/42976#discussion_r3878498131
##########
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:
Addressed in 6ad7fd5 — kept the guarantee for structurally-missing cells
only, and documented on the docstring that value-is-NULL rows collapse to `0.0`
inside `pivot_table` before this mask runs, so they render as `0%` not blank.
Full write-up in the top-level comment:
https://github.com/apache/superset/pull/42976#issuecomment-5449191803
##########
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:
Addressed in 6ad7fd5 — `show_values_as` now rejects non-additive aggregates
up front with `InvalidPostProcessingError`. Allow-list is `{sum, nansum, count,
count_nonzero}`; mean / median / min / max / distinct / callables are all
rejected before the pivot runs. Regression test
`test_pivot_show_values_as_rejects_non_additive_aggregate`. Full write-up:
https://github.com/apache/superset/pull/42976#issuecomment-5449191803
##########
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:
Addressed in 6ad7fd5 — added a `df.empty` early-return at the top of
`_apply_show_values_as`, so this `concat` is never reached with an empty list.
Zero-row regression test
`test_pivot_show_values_as_on_empty_pivot_returns_empty_frame`. Full write-up:
https://github.com/apache/superset/pull/42976#issuecomment-5449191803
--
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]
