SEPURI-SAI-KRISHNA opened a new pull request, #45043:
URL: https://github.com/apache/superset/pull/45043
### SUMMARY
`rename`'s collision guard uses `all` where it needs `any`, so it only fires
when every new name collides. A mapping that renames one column onto an
existing label while renaming another onto a free one slips past it, and pandas
creates a duplicate label. Details in #45042.
```diff
- if all(new_name in _rename_level for new_name in columns.values()):
+ if any(new_name in _rename_level for new_name in columns.values()):
```
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
On a frame whose columns are `[a, b, c]`:
| mapping | before | after |
|---|---|---|
| `{"a": "b"}` | raises `Label already exists` | raises `Label already
exists` (unchanged) |
| `{"a": "x"}` | `['x', 'b', 'c']` | `['x', 'b', 'c']` (unchanged) |
| `{"a": "x", "c": "z"}` | `['x', 'b', 'z']` | `['x', 'b', 'z']` (unchanged)
|
| `{"a": "b", "c": "z"}` | `['b', 'b', 'z']` | raises `Label already exists`
|
Only the last row changes. `df["b"]` used to come back as a DataFrame where
callers expect a Series, and the duplicate label travelled on to the next
post-processing operation.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/pandas_postprocessing/
```
Two tests are added, both failing on master and passing with this change:
| test | covers |
|---|---|
| `test_should_raise_exception_partial_duplication` | a two-entry mapping
where one target collides |
| `test_should_raise_exception_partial_duplication_on_multiindex` | the same
on a MultiIndex with `level=0` |
The existing duplication tests both drive the guard with a single-entry
mapping, where `all` and `any` are the same condition, which is why the
inversion was invisible. The three tests that do pass a two-entry mapping
rename onto free names, so they never reach the guard and are unaffected.
The `tests/unit_tests/pandas_postprocessing/` suite goes from 180 to 182
passing with no existing test modified. `ruff check`, `ruff format --check` are
clean and `pylint` rates the changed source file 10.00/10. The error message is
unchanged and already extracted, so there is no `messages.pot` change.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #45042
- [ ] 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]
