msyavuz commented on code in PR #43718:
URL: https://github.com/apache/superset/pull/43718#discussion_r3895113353
##########
superset/charts/client_processing.py:
##########
@@ -87,8 +165,16 @@ def pivot_df( # pylint: disable=too-many-locals,
too-many-arguments, too-many-s
show_columns_total: bool = False,
apply_metrics_on_rows: bool = False,
metric_name_aggfunc: Optional[str] = None,
+ show_values_as: Optional[str] = None,
) -> pd.DataFrame:
metric_name = __("Total (%(aggfunc)s)", aggfunc=metric_name_aggfunc or
aggfunc)
+ percent_mode = (
+ show_values_as if show_values_as in SHOW_VALUES_AS_PERCENT_MODES else
None
+ )
Review Comment:
Correct, but pre-existing rather than introduced here.
`pivot_df` computes every total by re-aggregating cells
(`client_processing.py:276`)
instead of using the per-rollup-level values the chart gets from GROUPING
SETS, so
server-side pivot totals already diverge from the rendered chart for
non-additive
aggregates — with or without percentages. The percent transform inherits
that, it
doesn't create it.
Worth noting the trigger is narrow: SIP-216 removed the "Aggregate
function" control,
so `aggregateFunction` only survives as orphaned data in `params` on
charts saved
before the removal. New and re-saved charts fall back to the `Sum` default.
Closing the gap properly means splitting the GROUPING SETS levels
server-side in the
export path — the rollup rows are already in the payload, just never
separated.
That's a bigger change than this PR, tracked separately.
--
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]
