SEPURI-SAI-KRISHNA opened a new pull request, #44828: URL: https://github.com/apache/superset/pull/44828
### SUMMARY `cum` filled gaps with `0` before cumulating, for all four operators: ```python df_cum = df_cum.fillna(0) # applied to sum, prod, min and max operation = "cum" + operator ``` `0` is the identity for addition only. A single gap therefore zeroed the whole remainder of a `cumprod` series, and made `0` the running `cummin`/`cummax` even though `0` need not appear in the data — silently, with no error. Details and provenance in #44827. This PR cumulates first and then carries the last cumulative value across the gap: ```python df_cum = _append_columns(df, getattr(df_cum, operation)().ffill(), columns) ``` | operator | input | before | after | |---|---|---|---| | `sum` | `[1, None, 2, 3]` | `[1, 1, 3, 6]` | `[1, 1, 3, 6]` (unchanged) | | `prod` | `[2, None, 3, 4]` | `[2, 0, 0, 0]` | `[2, 2, 6, 24]` | | `min` | `[5, None, 3, 7]` | `[5, 0, 0, 0]` | `[5, 5, 3, 3]` | | `max` | `[-5, None, -3, -9]` | `[-5, 0, 0, 0]` | `[-5, -5, -3, -3]` | `cumsum` output is byte-identical, so the behaviour #26429 introduced for #21093 is preserved and `test_cum_with_gap` passes unchanged. **One intentional behaviour change:** a *leading* gap now stays empty instead of becoming `0`. There is no preceding cumulative value to carry, and reporting `0` plots a point before the series has any data. This is covered by a new test. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A: no UI change. ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/pandas_postprocessing/test_cum.py ``` Two tests are added: - `test_cum_with_gap_non_additive_operators` — pins `prod`, `min` and `max` across a gap. - `test_cum_with_leading_gap` — pins the leading-gap behaviour. Both **fail on master** and pass with this change; the five pre-existing tests in the file pass either way. The whole `tests/unit_tests/pandas_postprocessing/` suite passes (166 tests). `ruff check` and `ruff format --check` are clean on both changed files. ### ADDITIONAL INFORMATION - [x] Has associated issue: Fixes #44827 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
