msyavuz commented on PR #43718:
URL: https://github.com/apache/superset/pull/43718#issuecomment-5491224217

   All three fixed in 58d92b0.
   
   Both bugs had one root cause: I tracked which totals collapse a whole axis, 
then treated *every* inserted row and column as whole-axis. So a prefix 
subtotal was looked up under a key the by-row level doesn't hold (blank cell), 
and took the grand total as its denominator.
   
   Replaced that with prefix depth — a total grouping `i` row and `j` column 
dimensions is the level over `rows[:i] + columns[:j]`, which subsumes the 
whole-axis case as `i = j = 0` and gives prefix subtotals the level they belong 
to. Your point that `rollup_levels[frozenset(["region"])]` was already sitting 
there unconsulted was the key; it needed no new data, just the right key. 
Levels the chart didn't request now fall back to the leaf-derived total rather 
than blanking.
   
   On the fixture: `EU/Subtotal` boy is `10/17`, `NA/Subtotal` is `40/24`, the 
subtotal total-column cells are `1.0`, and the leaf controls still read `10/18` 
and `40/25`.
   
   Your third comment was the most useful one — every rollup test used a single 
`groupbyRows` entry, so no prefix subtotal row could exist alongside a rollup 
frame. Green CI on a bug the fixtures structurally couldn't reach. Added 
`nested_grouping_sets_df` with two row dimensions and every level carrying a 
value no leaf reduction would produce, plus the two failing assertions and the 
two controls.
   
   Two gaps I have not closed, both documented in the commit rather than 
silently left:
   
   - `metricsLayout: ROWS` falls back to leaf-derived totals entirely. 
`buildGroupbyCombinations` applies `transposePivot` but not `metricsLayout`, so 
the requested levels don't correspond to the display axes once `pivot_df` swaps 
them.
   - `pivot_df` inserts subtotals whenever the totals flag is on, while the 
chart gates them on separate `rowSubTotals`/`colSubTotals` toggles. When they 
disagree the level isn't in the payload and that subtotal falls back.
   
   Happy to take either here or as a follow-up — say which you'd prefer.


-- 
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]

Reply via email to