Repository: narwhals-dev/narwhals
Pull request #2443 (2025-04-28 10:37:11)

enh: Support `fill_null(strategy=...).over(order_by=...)` for pandas-like
<!--
# Thanks for contributing a pull request! 
## Please make sure you see our contribution guidelines: https://github.com/narwhals-dev/narwhals/blob/main/CONTRIBUTING.md
-->

## What type of PR is this? (check all applicable)

- [ ] 💾 Refactor
- [ ] ✨ Feature
- [ ] 🐛 Bug Fix
- [ ] 🔧 Optimization
- [ ] 📝 Documentation
- [ ] ✅ Test
- [ ] 🐳 Other

## Related issues

- Related issue #\<issue number\>
- Closes #\<issue number\>

## Checklist

- [ ] Code follows style guide (ruff)
- [ ] Tests added
- [ ] Documented the changes

## If you have comments or can explain your changes, please do so below


MarcoGorelli: merging as the natural successor to https://github.com/narwhals-dev/narwhals/pull/2413, i think it's good to release them together

Patch:
diff --git a/narwhals/_dask/expr.py b/narwhals/_dask/expr.py
index 3f2af221..0653fe5b 100644
--- a/narwhals/_dask/expr.py
+++ b/narwhals/_dask/expr.py
@@ -616,6 +616,10 @@ class DaskExpr(
                 "https://narwhals-dev.github.io/narwhals/pandas_like_concepts/improve_group_by_operation/"
             )
             raise NotImplementedError(msg)
+        elif order_by:
+            # Wrong results https://github.com/dask/dask/issues/11806.
+            msg = "`over` with `order_by` is not yet supported in Dask."
+            raise NotImplementedError(msg)
         else:
             function_name = PandasLikeGroupBy._leaf_name(self)
             try:
diff --git a/narwhals/_pandas_like/expr.py b/narwhals/_pandas_like/expr.py
index 6e8f8412..16a57e58 100644
--- a/narwhals/_pandas_like/expr.py
+++ b/narwhals/_pandas_like/expr.py
@@ -20,6 +20,8 @@ if TYPE_CHECKING:
     from narwhals._expression_parsing import ExprMetadata
     from narwhals._pandas_like.dataframe import PandasLikeDataFrame
     from narwhals._pandas_like.namespace import PandasLikeNamespace
+    from narwhals.typing import FillNullStrategy
+    from narwhals.typing import NonNestedLiteral
     from narwhals.typing import RankMethod
     from narwhals.utils import Implementation
     from narwhals.utils import Version
@@ -41,6 +43,7 @@ WINDOW_FUNCTIONS_TO_PANDAS_EQUIVALENT = {
     "shift": "shift",
     "rank": "rank",
     "diff": "diff",
+    "fill_null": "fillna",
 }
 
 
@@ -275,6 +278,17 @@ class PandasLikeExpr(EagerExpr["PandasLikeDataFrame", PandasLikeSeries]):
                         )
                     else:
                         res_native = getattr(rolling, pandas_function_name)()
+                elif function_name == "fill_null":
+                    df_grouped = grouped[list(output_names)]
+                    if self._call_kwargs["strategy"] == "forward":
+                        res_native = df_grouped.ffill(limit=self._call_kwargs["limit"])
+                    elif self._call_kwargs["strategy"] == "backward":
+                        res_native = df_grouped.bfill(limit=self._call_kwargs["limit"])
+                    else:  # pragma: no cover
+                        # This is deprecated in pandas. Indeed, `nw.col('a').fill_null(3).over('b')`
+                        # does not seem very useful, and DuckDB doesn't support it either.
+                        msg = "`fill_null` with `over` without `strategy` specified is not supported."
+                        raise NotImplementedError(msg)
                 elif function_name == "len":
                     if len(output_names) != 1:  # pragma: no cover
                         msg = "Safety check failed, please report a bug."
@@ -319,6 +333,16 @@ class PandasLikeExpr(EagerExpr["PandasLikeDataFrame", PandasLikeSeries]):
     def cum_prod(self, *, reverse: bool) -> Self:
         return self._reuse_series("cum_prod", call_kwargs={"reverse": reverse})
 
+    def fill_null(
+        self,
+        value: Self | NonNestedLiteral,
+        strategy: FillNullStrategy | None,
+        limit: int | None,
+    ) -> Self:
+        return self._reuse_series(
+            "fill_null", call_kwargs={"strategy": strategy, "limit": limit}, value=value
+        )
+
     def rolling_sum(self, window_size: int, *, min_samples: int, center: bool) -> Self:
         return self._reuse_series(
             "rolling_sum",


Test patch:
diff --git a/tests/expr_and_series/fill_null_test.py b/tests/expr_and_series/fill_null_test.py
index 26ca35e2..173c35ce 100644
--- a/tests/expr_and_series/fill_null_test.py
+++ b/tests/expr_and_series/fill_null_test.py
@@ -371,13 +371,16 @@ def test_fill_null_series_exceptions(constructor_eager: ConstructorEager) -> Non
 def test_fill_null_strategies_with_partition_by(
     constructor: Constructor, request: pytest.FixtureRequest
 ) -> None:
-    if any(x in str(constructor) for x in ("pandas", "modin", "cudf", "pyarrow", "dask")):
+    if any(x in str(constructor) for x in ("pyarrow_table", "dask")):
         request.applymarker(pytest.mark.xfail)
 
     if ("duckdb" in str(constructor) and DUCKDB_VERSION < (1, 3)) or (
         "polars" in str(constructor) and POLARS_VERSION < (1, 10)
     ):
         pytest.skip()
+    if "modin" in str(constructor):
+        # unreliable
+        pytest.skip()
 
     data = {
         "partition": ["A", "B", "C", "B", "A", "B", "A", "C", "C"],