Panoramasx opened a new pull request, #45209: URL: https://github.com/apache/superset/pull/45209
### SUMMARY The `rank` post-processing operation treats `group_by=""` as if no grouping column was supplied. For a DataFrame with an empty-string column label, this silently returns global percentile ranks instead of the requested group-wise ranks. Check for `None` explicitly so an empty-string label is passed to pandas like any other column name. Omitted grouping and explicit `None` retain global ranking behavior. No public API, dependency or database changes are required. For group values `[A, A, B, B]` and metric values `[1, 3, 2, 4]`, the result changes from `[0.25, 0.75, 0.5, 1.0]` to `[0.5, 1.0, 0.5, 1.0]` when `group_by=""`. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable; this is a backend DataFrame operation. ### TESTING INSTRUCTIONS 1. Run `pytest tests/unit_tests/pandas_postprocessing/test_rank.py`. 2. Run `pytest tests/unit_tests/pandas_postprocessing/`. 3. Stage the changed files and run `pre-commit run`. The parameterized regression test asserts exact expected ranks for an empty-string column label, a normal label, and explicit `None`. Existing tests cover omitted grouping and a single group. Local validation on Python 3.12.14 and pandas 2.3.3: - Before the fix: the empty-string case fails; the other four ranking tests pass. - After the fix: all five ranking tests pass. - All 191 tests in `tests/unit_tests/pandas_postprocessing/` pass. - Staged-file `pre-commit run` passes, including mypy and Ruff checks. A direct Pylint run of the changed source also passes. The environment uses the locked development dependencies, excluding the optional MySQL and LDAP drivers that need system headers on macOS. Neither driver participates in these tests. No dependency files are changed. ### ADDITIONAL INFORMATION - [x] Has associated issue: Fixes #45208. - [ ] 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]
