SulimanAbdulrazzaq commented on PR #44652: URL: https://github.com/apache/superset/pull/44652#issuecomment-5939729449
Thanks for the review, @rusackas. I looked at codeant's thread and I think it is a separate, pre-existing behavior, so I'd leave it out of this PR. Rows with a null group key never reach `histogram_df`: `df.groupby(groupby)` uses the default `dropna=True`, so they are dropped before any counting, with or without `normalize` or `cumulative`. The denominator line (`histogram_df / histogram_df.values.sum()`) is unchanged by this PR; it is the same context line as on `master`. So the normalization is over the groups that are actually shown, and this PR only changes the order of normalizing and accumulating. A quick check with plain pandas (5 points, 2 of them with a null key): - `groupby` keeps groups `a` and `b`; 3 of the 5 points are counted. - Normalized (identical on `master` and here): `a` = [0.667, 0.0], `b` = [0.0, 0.333]. - Cumulative with this PR: `a` = [0.667, 0.667], `b` = [0.0, 0.333]. The cumulative values across the shown groups end at 1. Including null-key points in the total would also change the non-cumulative normalized output, and would call for either showing a null group (`dropna=False`) or dividing by a count that has no bar. That is a product decision, so I'd rather not mix it into this fix. If you'd like it, I can open a follow-up issue or PR for it. -- 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]
