chore(typing): enable reportIncompatibleMethodOverride in pyright (#3096)

* chore: enable reportIncompatibleMethodOverride in pyright

* chore(suggestion): Try turning ibis issue into a positive?

Part 1 of (#3096 (comment))

Isolates the problematic typing and then reuses it in other places it causes issues

* chore(typing): Ignore with context

Seems to appear at random and vanish

Error message is nonsense

* refactor: Move common `not_implemented` up

* refactor(typing): Do this ignore instead actually

* be reasonable

#3096 (comment)

---------

Co-authored-by: dangotbanned <[email protected]>

Author: MarcoGorelli

narwhals/_arrow/series.py

@@ -373,12 +373,12 @@ def shift(self, n: int) -> Self:
             return self._with_native(self.native)
         return self._with_native(pa.concat_arrays(arrays))
 
-    def std(self, ddof: int, *, _return_py_scalar: bool = True) -> float:
+    def std(self, *, ddof: int, _return_py_scalar: bool = True) -> float:
         return maybe_extract_py_scalar(
             pc.stddev(self.native, ddof=ddof), _return_py_scalar
         )
 
-    def var(self, ddof: int, *, _return_py_scalar: bool = True) -> float:
+    def var(self, *, ddof: int, _return_py_scalar: bool = True) -> float:
         return maybe_extract_py_scalar(
             pc.variance(self.native, ddof=ddof), _return_py_scalar
         )

narwhals/_compliant/expr.py

@@ -28,7 +28,12 @@
     LazyExprT,
     NativeExprT,
 )
-from narwhals._utils import _StoresCompliant, qualified_type_name, zip_strict
+from narwhals._utils import (
+    _StoresCompliant,
+    not_implemented,
+    qualified_type_name,
+    zip_strict,
+)
 from narwhals.dependencies import is_numpy_array, is_numpy_scalar
 
 if TYPE_CHECKING:
@@ -168,8 +173,14 @@ class DepthTrackingExpr(
     _depth: int
     _function_name: str
 
+    # NOTE: pyright bug?
+    # Method "from_column_names" overrides class "CompliantExpr" in an incompatible manner
+    # Parameter 2 type mismatch: base parameter is type "EvalNames[CompliantFrameT@DepthTrackingExpr]", override parameter is type "EvalNames[CompliantFrameT@DepthTrackingExpr]"
+    #   Type "EvalNames[CompliantFrameT@DepthTrackingExpr]" is not assignable to type "EvalNames[CompliantFrameT@DepthTrackingExpr]"
+    #     Parameter 1: type "CompliantFrameT@DepthTrackingExpr" is incompatible with type "CompliantFrameT@DepthTrackingExpr"
+    #       Type "CompliantFrameT@DepthTrackingExpr" is not assignable to type "CompliantFrameT@DepthTrackingExpr"
     @classmethod
-    def from_column_names(
+    def from_column_names(  # pyright: ignore[reportIncompatibleMethodOverride]
         cls: type[Self],
         evaluate_column_names: EvalNames[CompliantFrameT],
         /,
@@ -899,6 +910,12 @@ def fn(names: Sequence[str]) -> Sequence[str]:
     def name(self) -> LazyExprNameNamespace[Self]:
         return LazyExprNameNamespace(self)
 
+    ewm_mean = not_implemented()  # type: ignore[misc]
+    map_batches = not_implemented()  # type: ignore[misc]
+    replace_strict = not_implemented()  # type: ignore[misc]
+
+    cat: not_implemented = not_implemented()  # type: ignore[assignment]
+
 
 class _ExprNamespace(  # type: ignore[misc]
     _StoresCompliant[CompliantExprT_co], Protocol[CompliantExprT_co]

narwhals/_dask/expr.py

@@ -298,14 +298,14 @@ def min(self) -> Self:
     def max(self) -> Self:
         return self._with_callable(lambda expr: expr.max().to_series(), "max")
 
-    def std(self, ddof: int) -> Self:
+    def std(self, *, ddof: int) -> Self:
         return self._with_callable(
             lambda expr: expr.std(ddof=ddof).to_series(),
             "std",
             scalar_kwargs={"ddof": ddof},
         )
 
-    def var(self, ddof: int) -> Self:
+    def var(self, *, ddof: int) -> Self:
         return self._with_callable(
             lambda expr: expr.var(ddof=ddof).to_series(),
             "var",
@@ -682,20 +682,8 @@ def str(self) -> DaskExprStringNamespace:
     def dt(self) -> DaskExprDateTimeNamespace:
         return DaskExprDateTimeNamespace(self)
 
-    arg_max: not_implemented = not_implemented()
-    arg_min: not_implemented = not_implemented()
-    arg_true: not_implemented = not_implemented()
-    ewm_mean: not_implemented = not_implemented()
-    gather_every: not_implemented = not_implemented()
-    head: not_implemented = not_implemented()
-    map_batches: not_implemented = not_implemented()
-    sample: not_implemented = not_implemented()
-    rank: not_implemented = not_implemented()
-    replace_strict: not_implemented = not_implemented()
-    sort: not_implemented = not_implemented()
-    tail: not_implemented = not_implemented()
+    rank = not_implemented()
 
     # namespaces
     list: not_implemented = not_implemented()  # type: ignore[assignment]
-    cat: not_implemented = not_implemented()  # type: ignore[assignment]
     struct: not_implemented = not_implemented()  # type: ignore[assignment]

narwhals/_duckdb/expr.py

@@ -188,7 +188,7 @@ def func(expr: Expression) -> Expression:
     def len(self) -> Self:
         return self._with_callable(lambda _expr: F("count"))
 
-    def std(self, ddof: int) -> Self:
+    def std(self, *, ddof: int) -> Self:
         if ddof == 0:
             return self._with_callable(lambda expr: F("stddev_pop", expr))
         if ddof == 1:
@@ -204,7 +204,7 @@ def _std(expr: Expression) -> Expression:
 
         return self._with_callable(_std)
 
-    def var(self, ddof: int) -> Self:
+    def var(self, *, ddof: int) -> Self:
         if ddof == 0:
             return self._with_callable(lambda expr: F("var_pop", expr))
         if ddof == 1:

narwhals/_ibis/dataframe.py

@@ -336,29 +336,17 @@ def unique(
         return self._with_native(self.native.distinct(on=subset))
 
     def sort(self, *by: str, descending: bool | Sequence[bool], nulls_last: bool) -> Self:
-        if isinstance(descending, bool):
-            descending = [descending for _ in range(len(by))]
+        from narwhals._ibis.expr import IbisExpr
 
-        sort_cols: list[Any] = []
-
-        for i in range(len(by)):
-            direction_fn = ibis.desc if descending[i] else ibis.asc
-            col = direction_fn(by[i], nulls_first=not nulls_last)
-            sort_cols.append(col)
-
-        return self._with_native(self.native.order_by(*sort_cols))
+        cols = IbisExpr._sort(*by, descending=descending, nulls_last=nulls_last)
+        return self._with_native(self.native.order_by(*cols))
 
     def top_k(self, k: int, *, by: Iterable[str], reverse: bool | Sequence[bool]) -> Self:
-        if isinstance(reverse, bool):
-            reverse = [reverse] * len(list(by))
-        sort_cols = []
-
-        for is_reverse, by_col in zip_strict(reverse, by):
-            direction_fn = ibis.asc if is_reverse else ibis.desc
-            col = direction_fn(by_col, nulls_first=False)
-            sort_cols.append(cast("ir.Column", col))
+        from narwhals._ibis.expr import IbisExpr
 
-        return self._with_native(self.native.order_by(*sort_cols).head(k))
+        desc = not reverse if isinstance(reverse, bool) else [not el for el in reverse]
+        cols = IbisExpr._sort(*by, descending=desc, nulls_last=True)
+        return self._with_native(self.native.order_by(*cols).head(k))
 
     def drop_nulls(self, subset: Sequence[str] | None) -> Self:
         subset_ = subset if subset is not None else self.columns

narwhals/_ibis/expr.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import operator
-from functools import partial
 from typing import TYPE_CHECKING, Any, Callable, Literal, TypeVar, cast
 
 import ibis
@@ -10,7 +9,17 @@
 from narwhals._ibis.expr_list import IbisExprListNamespace
 from narwhals._ibis.expr_str import IbisExprStringNamespace
 from narwhals._ibis.expr_struct import IbisExprStructNamespace
-from narwhals._ibis.utils import is_floating, lit, narwhals_to_native_dtype
+from narwhals._ibis.utils import (
+    IntoColumn,
+    asc_nulls_first,
+    asc_nulls_last,
+    desc_nulls_first,
+    desc_nulls_last,
+    extend_bool,
+    is_floating,
+    lit,
+    narwhals_to_native_dtype,
+)
 from narwhals._sql.expr import SQLExpr
 from narwhals._utils import Implementation, Version, not_implemented, zip_strict
 
@@ -79,7 +88,7 @@ def _window_expression(
         self,
         expr: ir.Value,
         partition_by: Sequence[str | ir.Value] = (),
-        order_by: Sequence[str | ir.Column] = (),
+        order_by: Sequence[IntoColumn] = (),
         rows_start: int | None = None,
         rows_end: int | None = None,
         *,
@@ -94,9 +103,11 @@ def _window_expression(
             rows_between = {"preceding": -rows_start}
         else:
             rows_between = {}
+        desc = descending or False
+        last = nulls_last or False
         window = ibis.window(
             group_by=partition_by,
-            order_by=self._sort(*order_by, descending=descending, nulls_last=nulls_last),
+            order_by=self._sort(*order_by, descending=desc, nulls_last=last),
             **rows_between,
         )
         return expr.over(window)
@@ -110,24 +121,23 @@ def broadcast(self, kind: Literal[ExprKind.AGGREGATION, ExprKind.LITERAL]) -> Se
         # Ibis does its own broadcasting.
         return self
 
+    @staticmethod
     def _sort(
-        self,
-        *cols: ir.Column | str,
-        descending: Sequence[bool] | None = None,
-        nulls_last: Sequence[bool] | None = None,
+        *cols: IntoColumn,
+        descending: Sequence[bool] | bool = False,
+        nulls_last: Sequence[bool] | bool = False,
     ) -> Iterator[ir.Column]:
-        descending = descending or [False] * len(cols)
-        nulls_last = nulls_last or [False] * len(cols)
+        n = len(cols)
+        descending = extend_bool(descending, n)
+        nulls_last = extend_bool(nulls_last, n)
         mapping = {
-            (False, False): partial(ibis.asc, nulls_first=True),
-            (False, True): partial(ibis.asc, nulls_first=False),
-            (True, False): partial(ibis.desc, nulls_first=True),
-            (True, True): partial(ibis.desc, nulls_first=False),
+            (False, False): asc_nulls_first,
+            (False, True): asc_nulls_last,
+            (True, False): desc_nulls_first,
+            (True, True): desc_nulls_last,
         }
-        yield from (
-            cast("ir.Column", mapping[(_desc, _nulls_last)](col))
-            for col, _desc, _nulls_last in zip_strict(cols, descending, nulls_last)
-        )
+        for col, _desc, _nulls_last in zip_strict(cols, descending, nulls_last):
+            yield mapping[(_desc, _nulls_last)](col)
 
     @classmethod
     def from_column_names(
@@ -217,7 +227,7 @@ def func(df: IbisLazyFrame) -> Sequence[ir.IntegerScalar]:
             version=self._version,
         )
 
-    def std(self, ddof: int) -> Self:
+    def std(self, *, ddof: int) -> Self:
         def _std(expr: ir.NumericColumn, ddof: int) -> ir.Value:
             if ddof == 0:
                 return expr.std(how="pop")
@@ -230,7 +240,7 @@ def _std(expr: ir.NumericColumn, ddof: int) -> ir.Value:
 
         return self._with_callable(lambda expr: _std(expr, ddof))
 
-    def var(self, ddof: int) -> Self:
+    def var(self, *, ddof: int) -> Self:
         def _var(expr: ir.NumericColumn, ddof: int) -> ir.Value:
             if ddof == 0:
                 return expr.var(how="pop")
@@ -289,7 +299,7 @@ def is_unique(self) -> Self:
 
     def rank(self, method: RankMethod, *, descending: bool) -> Self:
         def _rank(expr: ir.Column) -> ir.Value:
-            order_by = next(self._sort(expr, descending=[descending], nulls_last=[True]))
+            order_by = next(self._sort(expr, descending=descending, nulls_last=True))
             window = ibis.window(order_by=order_by)
 
             if method == "dense":

narwhals/_ibis/expr_dt.py

@@ -58,8 +58,8 @@ def truncate(self, every: str) -> IbisExpr:
             fn = self._truncate(UNITS_DICT_TRUNCATE[unit])
         return self.compliant._with_callable(fn)
 
-    def offset_by(self, every: str) -> IbisExpr:
-        interval = Interval.parse_no_constraints(every)
+    def offset_by(self, by: str) -> IbisExpr:
+        interval = Interval.parse_no_constraints(by)
         unit = interval.unit
         if unit in {"y", "q", "mo", "d", "ns"}:
             msg = f"Offsetting by {unit} is not yet supported for ibis."

narwhals/_ibis/utils.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from functools import lru_cache
+from functools import lru_cache, partial
 from typing import TYPE_CHECKING, Any, Literal, cast, overload
 
 import ibis
@@ -9,7 +9,7 @@
 from narwhals._utils import Version, isinstance_or_issubclass
 
 if TYPE_CHECKING:
-    from collections.abc import Mapping
+    from collections.abc import Callable, Iterable, Mapping, Sequence
     from datetime import timedelta
 
     import ibis.expr.types as ir
@@ -23,6 +23,8 @@
     from narwhals.dtypes import DType
     from narwhals.typing import IntoDType, PythonLiteral
 
+IntoColumn: TypeAlias = "str | ir.Value | ir.Column"
+SortFn: TypeAlias = "Callable[[IntoColumn], ir.Column]"
 Incomplete: TypeAlias = Any
 """Marker for upstream issues."""
 
@@ -45,6 +47,23 @@ def lit(value: Any, dtype: Any | None = None) -> Incomplete:
     return literal(value, dtype)
 
 
+asc_nulls_first = cast("SortFn", partial(ibis.asc, nulls_first=True))
+asc_nulls_last = cast("SortFn", partial(ibis.asc, nulls_first=False))
+desc_nulls_first = cast("SortFn", partial(ibis.desc, nulls_first=True))
+desc_nulls_last = cast("SortFn", partial(ibis.desc, nulls_first=False))
+
+
+def extend_bool(
+    value: bool | Iterable[bool],  # noqa: FBT001
+    n_match: int,
+) -> Sequence[bool]:
+    """Ensure the given bool or sequence of bools is the correct length.
+
+    Stolen from https://github.com/pola-rs/polars/blob/b8bfb07a4a37a8d449d6d1841e345817431142df/py-polars/polars/_utils/various.py#L580-L594
+    """
+    return [value] * n_match if isinstance(value, bool) else list(value)
+
+
 BucketUnit: TypeAlias = Literal[
     "years",
     "quarters",

narwhals/_polars/dataframe.py

@@ -181,7 +181,7 @@ def schema(self) -> dict[str, DType]:
 
     def join(
         self,
-        other: Self,
+        other: PolarsBaseFrame[NativePolarsFrame],
         *,
         how: JoinStrategy,
         left_on: Sequence[str] | None,
@@ -568,7 +568,7 @@ def to_polars(self) -> pl.DataFrame:
 
     def join(
         self,
-        other: Self,
+        other: PolarsBaseFrame[pl.DataFrame],
         *,
         how: JoinStrategy,
         left_on: Sequence[str] | None,

narwhals/_spark_like/dataframe.py

@@ -385,6 +385,7 @@ def unique(
     def join(
         self,
         other: Self,
+        *,
         how: JoinStrategy,
         left_on: Sequence[str] | None,
         right_on: Sequence[str] | None,