bito-code-review[bot] commented on code in PR #43693:
URL: https://github.com/apache/superset/pull/43693#discussion_r4068226412
##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -190,6 +190,44 @@ 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), datetime
+ dtypes (converting to string representation with fill_value for NaT), and
+ nullable extension dtypes (casting to object so a string fill value is
+ valid) to prevent dtype errors and preserve NULL/NaN/NaT keys through
+ pivot_table().
+ """
+ s = df[col]
+ if isinstance(s.dtype, pd.CategoricalDtype):
+ if s.isna().any():
+ # Only add the category when there is a NULL to fill: adding it
+ # unconditionally leaves pivot_table()'s default observed=False
+ # to materialize an unobserved "<NULL>" group/column even for
+ # dimensions that never had a missing value.
+ if fill_value not in s.cat.categories:
+ s = s.cat.add_categories([fill_value])
+ df[col] = s.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).where(~s.isna(), other=fill_value)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Datetime type loss</b></div>
<div id="fix">
`s.astype(str)` degrades the entire datetime column to strings whenever a
single NaT exists, so non-null values lose their `datetime64` type in the pivot
index/columns (e.g. `Timestamp` becomes `'2019-01-01'`). `astype(object)`
preserves `Timestamp` for non-null values while still allowing the string fill,
keeping the NULL key fix without the type regression.
</div>
</div>
<small><i>Code Review Run #13e699</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
