aminghadersohi commented on code in PR #40812:
URL: https://github.com/apache/superset/pull/40812#discussion_r3370926467
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -123,6 +123,14 @@ def pivot( # pylint: disable=too-many-arguments
_("Pivot operation must include at least one aggregate")
)
+ # An empty list is semantically equivalent to no groupby columns. Normalise
+ # it to None so that all downstream branches (column_fill_value,
pivot_table,
+ # _restore_dropped_metric_columns) behave identically to the no-columns
case.
+ # This matters for categorical bar charts whose form_data carries
groupby=[]
+ # (the control panel default), causing series_columns=[] to be sent from
the
+ # frontend and an empty columns list to arrive here.
+ columns = columns or None
Review Comment:
Good question — I dug into this before responding.
In Python, an empty list is already falsy, so all the existing `if columns:`
guards in `pivot()` already handle `columns=[]` identically to `columns=None`:
- `if columns and column_fill_value:` → False in both cases
- `if not drop_missing_columns and columns:` → False in both cases
- `orig_columns_df = df[columns] if columns else None` → None in both cases
The only place where they could differ is `df.pivot_table(...,
columns=columns, ...)`. I tested that directly in pandas 2.1.4 — both
`columns=[]` and `columns=None` produce an identical flat result. I also
searched the git log for any prior revert or issue related to this
normalization and found nothing.
So the `columns = columns or None` line is purely defensive: it makes the
contract explicit at the pandas API boundary rather than relying on the
implicit falsy behaviour of an empty list. It can't break anything that wasn't
already broken — if you can recall what the past issue was, happy to adjust.
--
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]
