Repository: narwhals-dev/narwhals (python)

Issue:
qst - why does `filter(nw.col('temp').max() > 60)` sometimes raise and sometimes not?
from discord:
```python
import pyarrow as pa
import narwhals as nw

t = pa.Table.from_pydict({
  "date": ["2025-01-01", "2025-02-01", "2025-03-01", "2025-04-01"],
  "temperature": [50, 60, 70, 80],
})

nw_f = nw.from_native(t)
column = "date"

print(
    nw_f
    .filter(
        nw.col("temperature").max() > 60,
    )
)
```

I think we should validate the length of the predicates in `filter`, and that this should raise consistently

If Polars allows it, then OK, we can leave that be, but for pandas and pyarrow i think we should raise

Pull request:
fix: raise on shape mismatch in filter
## What type of PR is this? (check all applicable)

- [ ] 💾 Refactor
- [ ] ✨ Feature
- [x] 🐛 Bug Fix
- [ ] 🔧 Optimization
- [ ] 📝 Documentation
- [ ] ✅ Test
- [ ] 🐳 Other

## Related issues

- Closes #1786 

## Checklist

- [x] Code follows style guide (ruff)
- [x] Tests added
- [x] Documented the changes

## If you have comments or can explain your changes, please do so below

For lazy backends:

- duckdb raises: `duckdb.duckdb.BinderException`
- pyspark raises: `pyspark.errors.exceptions.captured.AnalysisException: [INVALID_WHERE_CONDITION]`
- dask raises `pd.errros.IndexingError`

at compute time. So polars is the only one allowing filtering with a scalar boolean

Patch:
diff --git a/narwhals/_arrow/dataframe.py b/narwhals/_arrow/dataframe.py
index 96a8ef71..e44ba29a 100644
--- a/narwhals/_arrow/dataframe.py
+++ b/narwhals/_arrow/dataframe.py
@@ -312,7 +312,11 @@ class ArrowDataFrame(CompliantDataFrame, CompliantLazyFrame):
             col_name = col_value.name
 
             column = validate_dataframe_comparand(
-                length=length, other=col_value, backend_version=self._backend_version
+                length=length,
+                other=col_value,
+                backend_version=self._backend_version,
+                allow_broadcast=True,
+                method_name="with_columns",
             )
 
             native_frame = (
@@ -482,7 +486,7 @@ class ArrowDataFrame(CompliantDataFrame, CompliantLazyFrame):
             and all(isinstance(x, bool) for x in predicates[0])
             and not constraints
         ):
-            mask = predicates[0]
+            mask_native = predicates[0]
         else:
             plx = self.__narwhals_namespace__()
             expr = plx.all_horizontal(
@@ -491,8 +495,15 @@ class ArrowDataFrame(CompliantDataFrame, CompliantLazyFrame):
                 )
             )
             # `[0]` is safe as all_horizontal's expression only returns a single column
-            mask = expr._call(self)[0]._native_series
-        return self._from_native_frame(self._native_frame.filter(mask))
+            mask = expr._call(self)[0]
+            mask_native = validate_dataframe_comparand(
+                length=len(self),
+                other=mask,
+                backend_version=self._backend_version,
+                allow_broadcast=False,
+                method_name="filter",
+            )
+        return self._from_native_frame(self._native_frame.filter(mask_native))
 
     def null_count(self: Self) -> Self:
         import pyarrow as pa
diff --git a/narwhals/_arrow/utils.py b/narwhals/_arrow/utils.py
index ca485265..7f642b94 100644
--- a/narwhals/_arrow/utils.py
+++ b/narwhals/_arrow/utils.py
@@ -6,6 +6,7 @@ from typing import Any
 from typing import Sequence
 from typing import overload
 
+from narwhals.exceptions import ShapeError
 from narwhals.utils import import_dtypes_module
 from narwhals.utils import isinstance_or_issubclass
 
@@ -212,7 +213,12 @@ def broadcast_and_extract_native(
 
 
 def validate_dataframe_comparand(
-    length: int, other: Any, backend_version: tuple[int, ...]
+    length: int,
+    other: Any,
+    backend_version: tuple[int, ...],
+    *,
+    allow_broadcast: bool,
+    method_name: str,
 ) -> Any:
     """Validate RHS of binary operation.
 
@@ -222,7 +228,14 @@ def validate_dataframe_comparand(
     from narwhals._arrow.series import ArrowSeries
 
     if isinstance(other, ArrowSeries):
-        if len(other) == 1:
+        len_other = len(other)
+        if len_other == 1:
+            if length > 1 and not allow_broadcast:
+                msg = (
+                    f"{method_name}'s length: 1 differs from that of the series: {length}"
+                )
+                raise ShapeError(msg)
+
             import numpy as np  # ignore-banned-import
             import pyarrow as pa
 
@@ -230,6 +243,11 @@ def validate_dataframe_comparand(
             if backend_version < (13,) and hasattr(value, "as_py"):
                 value = value.as_py()
             return pa.array(np.full(shape=length, fill_value=value))
+
+        if length != len_other:
+            msg = f"{method_name}'s length: {len_other} differs from that of the series: {length}"
+            raise ShapeError(msg)
+
         return other._native_series
 
     from narwhals._arrow.dataframe import ArrowDataFrame  # pragma: no cover
diff --git a/narwhals/_dask/dataframe.py b/narwhals/_dask/dataframe.py
index 1a8efc44..92c1f198 100644
--- a/narwhals/_dask/dataframe.py
+++ b/narwhals/_dask/dataframe.py
@@ -96,13 +96,14 @@ class DaskLazyFrame(CompliantLazyFrame):
     def columns(self) -> list[str]:
         return self._native_frame.columns.tolist()  # type: ignore[no-any-return]
 
-    def filter(self, *predicates: DaskExpr, **constraints: Any) -> Self:
+    def filter(self: Self, *predicates: DaskExpr, **constraints: Any) -> Self:
         plx = self.__narwhals_namespace__()
         expr = plx.all_horizontal(
             *chain(predicates, (plx.col(name) == v for name, v in constraints.items()))
         )
         # `[0]` is safe as all_horizontal's expression only returns a single column
         mask = expr._call(self)[0]
+
         return self._from_native_frame(self._native_frame.loc[mask])
 
     def select(
diff --git a/narwhals/_duckdb/dataframe.py b/narwhals/_duckdb/dataframe.py
index a3c2798b..0bd2c9f0 100644
--- a/narwhals/_duckdb/dataframe.py
+++ b/narwhals/_duckdb/dataframe.py
@@ -147,7 +147,7 @@ class DuckDBLazyFrame:
             result.append(value.alias(col))
         return self._from_native_frame(self._native_frame.select(*result))
 
-    def filter(self, *predicates: DuckDBExpr, **constraints: Any) -> Self:
+    def filter(self: Self, *predicates: DuckDBExpr, **constraints: Any) -> Self:
         plx = self.__narwhals_namespace__()
         expr = plx.all_horizontal(
             *chain(predicates, (plx.col(name) == v for name, v in constraints.items()))
diff --git a/narwhals/_pandas_like/dataframe.py b/narwhals/_pandas_like/dataframe.py
index fdd53b4a..05802b6a 100644
--- a/narwhals/_pandas_like/dataframe.py
+++ b/narwhals/_pandas_like/dataframe.py
@@ -400,7 +400,7 @@ class PandasLikeDataFrame:
     def row(self, row: int) -> tuple[Any, ...]:
         return tuple(x for x in self._native_frame.iloc[row])
 
-    def filter(self, *predicates: IntoPandasLikeExpr, **constraints: Any) -> Self:
+    def filter(self: Self, *predicates: IntoPandasLikeExpr, **constraints: Any) -> Self:
         plx = self.__narwhals_namespace__()
         if (
             len(predicates) == 1
@@ -408,7 +408,7 @@ class PandasLikeDataFrame:
             and all(isinstance(x, bool) for x in predicates[0])
             and not constraints
         ):
-            _mask = predicates[0]
+            mask_native = predicates[0]
         else:
             expr = plx.all_horizontal(
                 *chain(
@@ -417,8 +417,14 @@ class PandasLikeDataFrame:
             )
             # `[0]` is safe as all_horizontal's expression only returns a single column
             mask = expr._call(self)[0]
-            _mask = validate_dataframe_comparand(self._native_frame.index, mask)
-        return self._from_native_frame(self._native_frame.loc[_mask])
+            mask_native = validate_dataframe_comparand(
+                self._native_frame.index,
+                mask,
+                allow_broadcast=False,
+                method_name="filter",
+            )
+
+        return self._from_native_frame(self._native_frame.loc[mask_native])
 
     def with_columns(
         self,
@@ -437,13 +443,21 @@ class PandasLikeDataFrame:
             if name in new_column_name_to_new_column_map:
                 to_concat.append(
                     validate_dataframe_comparand(
-                        index, new_column_name_to_new_column_map.pop(name)
+                        index,
+                        new_column_name_to_new_column_map.pop(name),
+                        allow_broadcast=True,
+                        method_name="with_columns",
                     )
                 )
             else:
                 to_concat.append(self._native_frame[name])
         to_concat.extend(
-            validate_dataframe_comparand(index, new_column_name_to_new_column_map[s])
+            validate_dataframe_comparand(
+                index,
+                new_column_name_to_new_column_map[s],
+                allow_broadcast=True,
+                method_name="with_columns",
+            )
             for s in new_column_name_to_new_column_map
         )
 
diff --git a/narwhals/_pandas_like/utils.py b/narwhals/_pandas_like/utils.py
index 08d49058..4eec710a 100644
--- a/narwhals/_pandas_like/utils.py
+++ b/narwhals/_pandas_like/utils.py
@@ -17,6 +17,7 @@ from narwhals._arrow.utils import (
     native_to_narwhals_dtype as arrow_native_to_narwhals_dtype,
 )
 from narwhals.exceptions import ColumnNotFoundError
+from narwhals.exceptions import ShapeError
 from narwhals.utils import Implementation
 from narwhals.utils import import_dtypes_module
 from narwhals.utils import isinstance_or_issubclass
@@ -152,7 +153,9 @@ def broadcast_align_and_extract_native(
     return lhs._native_series, rhs
 
 
-def validate_dataframe_comparand(index: Any, other: Any) -> Any:
+def validate_dataframe_comparand(
+    index: Any, other: Any, *, allow_broadcast: bool, method_name: str
+) -> Any:
     """Validate RHS of binary operation.
 
     If the comparison isn't supported, return `NotImplemented` so that the
@@ -164,10 +167,27 @@ def validate_dataframe_comparand(index: Any, other: Any) -> Any:
     if isinstance(other, PandasLikeDataFrame):
         return NotImplemented
     if isinstance(other, PandasLikeSeries):
-        if other.len() == 1:
+        len_index = len(index)
+        len_other = other.len()
+
+        if len_other == 1:
+            if len_index > 1 and not allow_broadcast:
+                msg = (
+                    f"{method_name}'s length: 1 differs from that of the series: "
+                    f"{len_index}"
+                )
+                raise ShapeError(msg)
             # broadcast
             s = other._native_series
             return s.__class__(s.iloc[0], index=index, dtype=s.dtype, name=s.name)
+
+        if len_index != len_other:
+            msg = (
+                f"{method_name}'s length: {len_other} differs from that of the series: "
+                f"{len_index}"
+            )
+            raise ShapeError(msg)
+
         if other._native_series.index is not index:
             return set_index(
                 other._native_series,
diff --git a/narwhals/_spark_like/dataframe.py b/narwhals/_spark_like/dataframe.py
index 34da43fd..9d302014 100644
--- a/narwhals/_spark_like/dataframe.py
+++ b/narwhals/_spark_like/dataframe.py
@@ -108,7 +108,7 @@ class SparkLikeLazyFrame:
         new_columns_list = [col.alias(col_name) for col_name, col in new_columns.items()]
         return self._from_native_frame(self._native_frame.select(*new_columns_list))
 
-    def filter(self, *predicates: SparkLikeExpr, **constraints: Any) -> Self:
+    def filter(self: Self, *predicates: SparkLikeExpr, **constraints: Any) -> Self:
         plx = self.__narwhals_namespace__()
         expr = plx.all_horizontal(
             *chain(predicates, (plx.col(name) == v for name, v in constraints.items()))


Test patch:
diff --git a/tests/frame/filter_test.py b/tests/frame/filter_test.py
index 759d175c..922af0e6 100644
--- a/tests/frame/filter_test.py
+++ b/tests/frame/filter_test.py
@@ -3,8 +3,10 @@ from __future__ import annotations
 from contextlib import nullcontext as does_not_raise
 
 import pytest
+from polars.exceptions import ShapeError as PlShapeError
 
 import narwhals.stable.v1 as nw
+from narwhals.exceptions import ShapeError
 from tests.utils import Constructor
 from tests.utils import assert_equal_data
 
@@ -33,3 +35,40 @@ def test_filter_with_boolean_list(
         result = df.filter([False, True, True])
         expected = {"a": [3, 2], "b": [4, 6], "z": [8.0, 9.0]}
         assert_equal_data(result, expected)
+
+
+def test_filter_raise_on_agg_predicate(constructor: Constructor) -> None:
+    data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8, 9]}
+    df = nw.from_native(constructor(data))
+
+    context = (
+        pytest.raises(
+            ShapeError,
+            match="filter's length: 1 differs from that of the series: 3",
+        )
+        if any(x in str(constructor) for x in ("pandas", "pyarrow", "modin"))
+        else does_not_raise()
+        if "polars" in str(constructor)
+        else pytest.raises(Exception)  # type: ignore[arg-type] # noqa: PT011
+    )
+    with context:
+        df.filter(nw.col("a").max() > 2).lazy().collect()
+
+
+def test_filter_raise_on_shape_mismatch(constructor: Constructor) -> None:
+    data = {"a": [1, 3, 2], "b": [4, 4, 6], "z": [7.0, 8, 9]}
+    df = nw.from_native(constructor(data))
+
+    context = (
+        pytest.raises(
+            ShapeError, match="filter's length: 2 differs from that of the series: 3"
+        )
+        if any(x in str(constructor) for x in ("pandas", "pyarrow", "modin"))
+        else pytest.raises(
+            PlShapeError, match="filter's length: 2 differs from that of the series: 3"
+        )
+        if "polars" in str(constructor)
+        else pytest.raises(Exception)  # type: ignore[arg-type] # noqa: PT011
+    )
+    with context:
+        df.filter(nw.col("b").unique() > 2).lazy().collect()