Abdulrehman-PIAIC80387 commented on code in PR #42976:
URL: https://github.com/apache/superset/pull/42976#discussion_r3820266611
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -168,4 +297,7 @@ def pivot( # pylint: disable=too-many-arguments
# be silently dropped by stack's default dropna=True behavior.
df = df.stack(level=0, dropna=False).unstack()
+ if percent_mode is not None:
Review Comment:
Confirmed empirically — you're right. Fixed in 5290ebe.
**Reproduction** (2 metrics × 2 categories × 2 rows,
`show_values_as="percent_row"`, `combine_value_with_metric=True`):
Before the reshape, columns are `(metric, category)` — `[('a', 'c1'), ('a',
'c2'), ('b', 'c1'), ('b', 'c2')]`.
After `stack(level=0).unstack()`, they become `(category, metric)` —
`[('c1', 'a'), ('c1', 'b'), ('c2', 'a'), ('c2', 'b')]`.
`_apply_show_values_as` iterates `df.columns.get_level_values(0).unique()`
and treats each as a metric. When the reshape runs first, that iteration walks
categories, so `a` gets divided by `a+b` sums:
| order | metric a row sum (should be 1.0) | metric b row sum |
|---|---|---|
| reshape → percent (current) | **1.78** ❌ | **0.22** ❌ |
| percent → reshape (fix) | 1.00 ✓ | 1.00 ✓ |
**Fix:** swap the two blocks — apply the percent transform on the natural
`(metric, category)` shape first, then let the reshape run on
already-normalized values. Regression test added at
`test_pivot_show_values_as_with_combine_value_with_metric_preserves_per_metric`
which fails on the pre-fix code and passes on the fix.
--
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]
