sadpandajoe commented on code in PR #43693:
URL: https://github.com/apache/superset/pull/43693#discussion_r3890675089
##########
tests/unit_tests/pandas_postprocessing/test_pivot.py:
##########
@@ -871,3 +872,268 @@ def test_pivot_show_values_as_preserves_structural_nan()
-> None:
assert pd.isna(result.loc["r1", ("v", "c2")])
# r1's row-total is just c1 (10.0), so c1 is 100%.
assert result.loc["r1", ("v", "c1")] == pytest.approx(1.0)
+
+
+# --- NULL index preservation tests (#43547) ----------------------------------
+#
+# pandas pivot_table() silently drops rows whose index columns contain NaN,
+# regardless of the dropna= setting (which only governs the column axis).
+# The fix fills index columns with NULL_STRING before calling pivot_table(),
+# mirroring the existing treatment of the columns= parameter.
+
+
+def test_pivot_preserves_null_index_value() -> None:
+ """A NULL value in a flat (no columns groupby) index column must appear
+ as a '<NULL>' row in the result rather than being silently dropped.
+
+ Regression for #43547: pivot_table() drops NaN index rows regardless of
+ dropna=; filling the index with NULL_STRING before the call preserves them.
+ """
+ df = DataFrame(
+ {
+ "row": ["r1", None, "r2"], # middle row has a NULL index value
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["row"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ # The NULL group must survive as a real index label, not be dropped.
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ # The metric value for the NULL group must be correct.
+ assert result.loc[NULL_STRING, "v"] == 99
+
+
+def test_pivot_preserves_null_index_value_with_columns() -> None:
+ """A NULL value in an index column must survive as '<NULL>' even when a
+ columns= groupby is also active (MultiIndex column case).
+
+ Regression for #43547: the fix must work for both the flat pivot and the
+ MultiIndex pivot so that NULL row groups are never silently dropped.
+ """
+ df = DataFrame(
+ {
+ "row": ["r1", None, "r2"], # middle row has a NULL index value
+ "col": ["c1", "c1", "c1"],
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["row"],
+ columns=["col"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ # The NULL group must survive as a real index label in the MultiIndex
pivot.
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ # The metric value for the NULL-indexed group must be correct.
+ assert result.loc[NULL_STRING, ("v", "c1")] == 99
+
+
+def test_pivot_preserves_null_index_value_categorical() -> None:
+ """A categorical index column with a NULL value must have NULL_STRING added
+ as a valid category first and be preserved as '<NULL>' in the pivot output.
+
+ Regression for #43547: ensures fillna() on CategoricalDtype does not raise
+ and preserves the NULL index group.
+ """
+ df = DataFrame(
+ {
+ "row": pd.Categorical(["r1", None, "r2"]),
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["row"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ assert result.loc[NULL_STRING, "v"] == 99
+
+
+def test_pivot_preserves_null_index_value_categorical_with_columns() -> None:
+ """Both index and column dimensions as categorical dtypes with NULL values
+ must properly add NULL_STRING to categories and preserve '<NULL>' rows and
+ columns in the MultiIndex output.
+ """
+ df = DataFrame(
+ {
+ "row": pd.Categorical(["r1", None, "r2"]),
+ "col": pd.Categorical(["c1", None, "c2"]),
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["row"],
+ columns=["col"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ assert result.loc[NULL_STRING, ("v", NULL_STRING)] == 99
+
+
+def test_pivot_preserves_null_index_value_datetime() -> None:
+ """A datetime index column containing NaT/NULL must not raise a TypeError
+ when filled and must be preserved as '<NULL>' in the pivot output.
+
+ Regression for #43547: datetime64 columns cannot store strings directly;
+ converting NaT-containing datetime columns to string ensures NaT keys
+ survive pivot_table() without dtype/sort errors.
+ """
+ df = DataFrame(
+ {
+ "dttm": to_datetime(["2019-01-01", None, "2019-01-03"]),
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["dttm"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ assert result.loc[NULL_STRING, "v"] == 99
+
+
+def test_pivot_preserves_null_index_value_datetime_with_columns() -> None:
+ """A datetime index column containing NaT/NULL with a columns groupby
+ must preserve '<NULL>' in the MultiIndex output without type errors.
+ """
+ df = DataFrame(
+ {
+ "dttm": to_datetime(["2019-01-01", None, "2019-01-03"]),
+ "col": ["c1", "c1", "c2"],
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["dttm"],
+ columns=["col"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ assert result.loc[NULL_STRING, ("v", "c1")] == 99
+
+
+def test_pivot_preserves_null_index_value_datetime_timezone_aware() -> None:
+ """A timezone-aware datetime index containing NaT must preserve '<NULL>'
+ without dtype or timezone conversion errors.
+ """
+ df = DataFrame(
+ {
+ "dttm": to_datetime(["2019-01-01", None, "2019-01-03"], utc=True),
+ "v": [10, 99, 30],
+ }
+ )
+ result = pivot(
+ df=df,
+ index=["dttm"],
+ aggregates={"v": {"operator": "sum"}},
+ )
+ assert NULL_STRING in result.index, (
+ f"Expected '{NULL_STRING}' in pivot index; got {result.index.tolist()}"
+ )
+ assert result.loc[NULL_STRING, "v"] == 99
+
+
+def test_pivot_preserves_null_index_value_categorical_already_in_categories()
-> None:
+ """A categorical index that already has NULL_STRING in its categories
+ must not fail or attempt duplicate category insertion and must fill NULLs.
+ """
+ df = DataFrame(
+ {
+ "row": pd.Categorical(["r1", None, "r2"], categories=["r1", "r2",
NULL_STRING]),
+ "v": [10, 99, 30],
+ }
Review Comment:
Agreed—the current head fails required pre-commit because Ruff reformats the
changed files and the end-of-file hook changes this test file. Could you apply
and commit the formatter output?
--
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]
