Copilot commented on code in PR #43694:
URL: https://github.com/apache/superset/pull/43694#discussion_r3890474161
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -187,6 +187,31 @@ def _restore_dropped_metric_columns(
return df
+def _fill_dimension_column(df: DataFrame, col: str, fill_value: str) -> None:
+ """Fill missing values in a groupby dimension column before pivoting.
+
+ Handles categorical dtypes (adding fill_value to categories) and datetime
+ dtypes (converting to string representation with fill_value for NaT) to
prevent
+ dtype errors and preserve NULL/NaN/NaT keys through pivot_table().
+ """
+ s = df[col]
+ if (
+ isinstance(s.dtype, pd.CategoricalDtype)
+ and fill_value not in s.cat.categories
+ ):
+ df[col] = s.cat.add_categories([fill_value]).fillna(value=fill_value)
Review Comment:
This adds the fill value as an unobserved category even when the dimension
contains no missing values. Because `pivot_table()` is called with the pandas
2.3 default `observed=False`, additive aggregations such as `sum` materialize
that category as a zero-valued `<NULL>` row or column, so a categorical pivot
with no NULL input gains a fake NULL group. Only extend the categories when a
fill is actually needed.
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -187,6 +187,31 @@ def _restore_dropped_metric_columns(
return df
+def _fill_dimension_column(df: DataFrame, col: str, fill_value: str) -> None:
+ """Fill missing values in a groupby dimension column before pivoting.
+
+ Handles categorical dtypes (adding fill_value to categories) and datetime
+ dtypes (converting to string representation with fill_value for NaT) to
prevent
+ dtype errors and preserve NULL/NaN/NaT keys through pivot_table().
+ """
+ s = df[col]
+ if (
+ isinstance(s.dtype, pd.CategoricalDtype)
+ and fill_value not in s.cat.categories
+ ):
+ df[col] = s.cat.add_categories([fill_value]).fillna(value=fill_value)
+ elif pd.api.types.is_datetime64_any_dtype(s.dtype) or getattr(s.dtype,
"kind", None) == "M":
+ if s.isna().any():
+ df[col] = s.astype(str).replace({
+ "NaT": fill_value,
+ "<NA>": fill_value,
+ "nan": fill_value,
+ "None": fill_value,
+ })
+ else:
+ df[col] = s.fillna(value=fill_value)
Review Comment:
This temporal special case stringifies every valid timestamp when one value
is missing, so after `flatten()` those values bypass the epoch serializer used
by chart responses (`charts/data/api.py:608-611`). It also excludes
`timedelta64` (kind `m`), causing a NULL interval dimension to reach
`fillna("<NULL>")` and raise a `TypeError`. Handle both temporal dtypes as
objects so the sentinel is allowed without rewriting valid values.
--
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]