SEPURI-SAI-KRISHNA commented on code in PR #45018:
URL: https://github.com/apache/superset/pull/45018#discussion_r4202763766
##########
superset/utils/pandas_postprocessing/utils.py:
##########
@@ -238,13 +243,25 @@ def _append_columns(
in `base_df` unchanged.
:return: new DataFrame with combined data from `base_df` and `append_df`
"""
- if all(key == value for key, value in columns.items()):
+ if not columns:
+ # Nothing to combine, but still hand back a new DataFrame so that a
+ # caller which mutates the result does not reach `base_df`.
+ return base_df.copy()
+
+ overwritten = {key: value for key, value in columns.items() if key ==
value}
+ appended = {key: value for key, value in columns.items() if key != value}
Review Comment:
Confirmed, `['y', 'z', 'z']` with `columns.duplicated().any()` true.
Recorded on #45017 with the repro, and I added a "Not covered" line to the
PR body so the summary does not read as covering it.
On the fix itself: keying the split on `target in base_df.columns` rather
than on `source == target` handles your case and subsumes the identity case,
since an identity target is present by definition. That makes it a change to
what the helper promises rather than another bug in the same lines, which is
why I would rather do it as the follow-up than widen this PR.
--
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]
