EnxDev commented on code in PR #45018:
URL: https://github.com/apache/superset/pull/45018#discussion_r4210884333


##########
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:
   Looked into where the duplication came from. Before #19116 this was a plain 
`assign()`, which overwrote or appended column by column and never duplicated 
anything. The `all()` + `concat` version came in with the MultiIndex support, 
so this PR is really putting the old behavior back.
   
   That makes the `{"y": "z"}` follow-up a restore too, since `assign` 
overwrote `z` as well. And I don't see anything relying on the duplicates: 
callers in the repo all pass identity maps, and the JSON serializer already 
drops duplicate labels.



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