rusackas commented on code in PR #43694:
URL: https://github.com/apache/superset/pull/43694#discussion_r4135194742


##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -299,8 +299,26 @@ def pivot(  # pylint: disable=too-many-arguments  # noqa: 
C901
         percent_mode = show_values_as
 
     if columns and column_fill_value:
+        for col in columns:
+            if (
+                isinstance(df[col].dtype, pd.CategoricalDtype)
+                and column_fill_value not in df[col].cat.categories
+            ):
+                df[col] = df[col].cat.add_categories([column_fill_value])
         df[columns] = df[columns].fillna(value=column_fill_value)
 
+    # Fill NULL/NaN values in the index columns with NULL_STRING so that
+    # NULL grouping keys survive as a real "<NULL>" row in the pivot output.
+    # Mirrors the column fill above; pivot_table() drops NaN index rows
+    # regardless of the dropna= setting (dropna only governs the column axis).
+    for col in index:
+        if (
+            isinstance(df[col].dtype, pd.CategoricalDtype)
+            and NULL_STRING not in df[col].cat.categories
+        ):
+            df[col] = df[col].cat.add_categories([NULL_STRING])
+    df[index] = df[index].fillna(value=NULL_STRING)

Review Comment:
   Datetime, timedelta64 and the nullable extension dtypes all get cast to 
`object` before filling now, so the sentinel goes in without a dtype error 
(f2c9974147).



##########
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:
   Fill's skipped entirely now when nothing's actually missing, so no more 
zero-value group on a clean categorical dimension.



##########
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:
   Only the missing entries become the sentinel now, valid values stay real 
`Timestamp` objects. Added a test asserting the type so this doesn't regress.



##########
superset/utils/pandas_postprocessing/pivot.py:
##########
@@ -190,6 +190,33 @@ 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:
   `Int64`/`Float64`/boolean get the same object-cast treatment as 
datetime/timedelta now, with new regression tests for `Int64` and boolean.



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