EnxDev commented on code in PR #45083:
URL: https://github.com/apache/superset/pull/45083#discussion_r4222468387
##########
superset/utils/pandas_postprocessing/utils.py:
##########
@@ -248,14 +258,24 @@ def _append_columns(
# 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}
+ appended = {
+ key: value
+ for key, value in columns.items()
+ if value != key and value not in base_df.columns
+ }
+ overwritten = {key: value for key, value in columns.items() if key not in
appended}
_base_df = base_df
if overwritten:
# make sure to return a new DataFrame instead of changing the
`base_df`.
_base_df = base_df.copy()
- _base_df.loc[:, overwritten.keys()] = append_df
+ # Select before renaming, as below, and because once the source name
may
+ # differ from the target, it is the target that says which column to
+ # write.
+ overwritten_df = append_df.loc[:, overwritten.keys()].rename(
+ columns=overwritten
+ )
+ _base_df.loc[:, overwritten_df.columns] = overwritten_df
Review Comment:
Non-blocking: if two sources point at the same existing target, like `{"y":
"z", "z": "z"}`, both land in `overwritten` and this line raises `ValueError:
Setting with non-unique columns is not allowed`. That isn't an
`InvalidPostProcessingError`, so the chart data request comes back as a 500
rather than a 400. On master it just returned a duplicate `z`.
The same mapping with a new target (`{"y": "w", "z": "w"}`) still goes
through `concat` and duplicates `w`. Could we reject duplicate targets up front
with an `InvalidPostProcessingError`? That covers both cases. Fine as a
follow-up too.
--
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]
