SEPURI-SAI-KRISHNA commented on PR #45043:
URL: https://github.com/apache/superset/pull/45043#issuecomment-6036083076

   Good catch, and it is a regression I introduced. Confirmed:
   
   ```python
   df = pd.DataFrame({"a": [1], "b": [2], "c": [3]})
   list(df.rename(columns={"a": "b", "b": "d"}).columns)
   # ['b', 'd', 'c'], unique
   ```
   
   `b` is vacated by the same mapping, so `any` rejects a rename that master 
accepts and that produces a perfectly good index.
   
   The real problem is that the guard tests whether a target label is present 
rather than whether the rename introduces a duplicate. Flipping `all` to `any` 
keeps the wrong test and only moves which cases it gets wrong. Running that 
down turned up two more:
   
   - `{"a": "z", "b": "z"}` gives `['z', 'z', 'c']`. Master misses it and `any` 
misses it too, since neither `z` exists beforehand for a presence test to find.
   - A frame that arrives with duplicate labels, which earlier operations can 
produce, must stay renameable. Asking whether the result holds any duplicate 
would make every later rename on such a frame fail, so the guard compares 
duplicate counts and only rejects a rename that adds one.
   
   Pushed that guard: rename an empty slice, `df.iloc[:0]`, with the same 
arguments the real call uses, then compare counts. No rows are copied, so it 
costs 0.5 ms on a 1000 column frame. Three more tests, covering your case and 
the two above. The full behaviour matrix is in the PR body.
   
   One relaxation worth a maintainer's eye: a straight swap, `{"a": "b", "b": 
"a"}`, used to raise and now succeeds with `['b', 'a', 'c']`. That is a unique 
index pandas handles fine, so I treated it as part of the same correction 
rather than preserving an accidental rejection. I can keep swaps rejected if 
that is preferred.
   


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