Skip to content

chore(typing): enable reportIncompatibleMethodOverride in pyright #3096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MarcoGorelli merged 7 commits into from
Sep 7, 2025
Merged

chore(typing): enable reportIncompatibleMethodOverride in pyright #3096

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f87ec46
chore: enable reportIncompatibleMethodOverride in pyright
MarcoGorelli Sep 6, 2025
1a8e7d6
chore(suggestion): Try turning ibis issue into a positive?
dangotbanned Sep 7, 2025
3e16aa4
Merge branch 'main' into reportincompatiblemethodoverride
dangotbanned Sep 7, 2025
9cda7ba
chore(typing): Ignore with context
dangotbanned Sep 7, 2025
2174359
refactor: Move common `not_implemented` up
dangotbanned Sep 7, 2025
dc4a596
refactor(typing): Do this ignore instead actually
dangotbanned Sep 7, 2025
bc59340
be reasonable
dangotbanned Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions narwhals/_arrow/series.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
20 changes: 6 additions & 14 deletions narwhals/_dask/expr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -682,18 +682,10 @@ 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()
ewm_mean = not_implemented()
map_batches = not_implemented()
rank = not_implemented()
replace_strict = not_implemented()

# namespaces
list: not_implemented = not_implemented() # type: ignore[assignment]
Expand Down
4 changes: 2 additions & 2 deletions narwhals/_duckdb/expr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down
12 changes: 8 additions & 4 deletions narwhals/_ibis/expr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def _window_expression(
self,
expr: ir.Value,
partition_by: Sequence[str | ir.Value] = (),
order_by: Sequence[str | ir.Column] = (),
order_by: Sequence[str | ir.Value] = (),
rows_start: int | None = None,
rows_end: int | None = None,
*,
Expand All @@ -96,7 +96,11 @@ def _window_expression(
rows_between = {}
window = ibis.window(
group_by=partition_by,
order_by=self._sort(*order_by, descending=descending, nulls_last=nulls_last),
order_by=self._sort(
*cast("Sequence[ir.Column]", order_by),
descending=descending,
nulls_last=nulls_last,
),
**rows_between,
)
return expr.over(window)
Expand Down Expand Up @@ -217,7 +221,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")
Expand All @@ -230,7 +234,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")
Expand Down
4 changes: 2 additions & 2 deletions narwhals/_ibis/expr_dt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
4 changes: 2 additions & 2 deletions narwhals/_polars/dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions narwhals/_spark_like/dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ def unique(
def join(
self,
other: Self,
*,
how: JoinStrategy,
left_on: Sequence[str] | None,
right_on: Sequence[str] | None,
Expand Down
8 changes: 4 additions & 4 deletions narwhals/_spark_like/expr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def _null_count(expr: Column) -> Column:

return self._with_callable(_null_count)

def std(self, ddof: int) -> Self:
def std(self, *, ddof: int) -> Self:
F = self._F
if ddof == 0:
return self._with_callable(F.stddev_pop)
Expand All @@ -277,7 +277,7 @@ def func(expr: Column) -> Column:

return self._with_callable(func)

def var(self, ddof: int) -> Self:
def var(self, *, ddof: int) -> Self:
F = self._F
if ddof == 0:
return self._with_callable(F.var_pop)
Expand Down Expand Up @@ -305,9 +305,9 @@ def _is_finite(expr: Column) -> Column:

return self._with_elementwise(_is_finite)

def is_in(self, values: Sequence[Any]) -> Self:
def is_in(self, other: Sequence[Any]) -> Self:
def _is_in(expr: Column) -> Column:
return expr.isin(values) if values else self._F.lit(False)
return expr.isin(other) if other else self._F.lit(False)

return self._with_elementwise(_is_in)

Expand Down
21 changes: 6 additions & 15 deletions narwhals/_sql/expr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,18 +784,9 @@ def str(self) -> SQLExprStringNamespace[Self]: ...
def dt(self) -> SQLExprDateTimeNamesSpace[Self]: ...

# Not implemented

arg_max: not_implemented = not_implemented()
arg_min: not_implemented = not_implemented()
arg_true: not_implemented = not_implemented()
cat: not_implemented = not_implemented() # type: ignore[assignment]
drop_nulls: 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()
replace_strict: not_implemented = not_implemented()
sort: not_implemented = not_implemented()
tail: not_implemented = not_implemented()
sample: not_implemented = not_implemented()
unique: not_implemented = not_implemented()
cat: Any = not_implemented()
drop_nulls: Any = not_implemented()
ewm_mean: Any = not_implemented()
map_batches: Any = not_implemented()
replace_strict: Any = not_implemented()
unique: Any = not_implemented()
14 changes: 7 additions & 7 deletions narwhals/dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ def top_k(

def join(
self,
other: Self,
other: BaseFrame[_FrameT],
on: str | list[str] | None,
how: JoinStrategy,
*,
Expand Down Expand Up @@ -335,7 +335,7 @@ def gather_every(self, n: int, offset: int = 0) -> Self:

def join_asof(
self,
other: Self,
other: BaseFrame[_FrameT],
*,
left_on: str | None,
right_on: str | None,
Expand Down Expand Up @@ -1111,7 +1111,7 @@ def to_dict(self, *, as_series: Literal[True] = ...) -> dict[str, Series[Any]]:
def to_dict(self, *, as_series: Literal[False]) -> dict[str, list[Any]]: ...
@overload
def to_dict(
self, *, as_series: bool
self, *, as_series: bool = True
) -> dict[str, Series[Any]] | dict[str, list[Any]]: ...
def to_dict(
self, *, as_series: bool = True
Expand Down Expand Up @@ -1800,7 +1800,7 @@ def top_k(

def join(
self,
other: Self,
other: BaseFrame[DataFrameT],

@dangotbanned dangotbanned Sep 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really liking how this one leaks out into the docs

Show docs

image

But more importantly it breaks the existing warnings here:

import polars as pl

import narwhals as nw

data = {"a": ["A", "B", "A"], "b": [1, 2, 3]}

df = nw.from_native(pl.DataFrame(data))

okay = df.join(df, on="a")

# Previously reported an error
# Argument of type "LazyFrame[Any]" cannot be assigned to parameter "other" of type "narwhals.dataframe.DataFrame[polars.dataframe.frame.DataFrame]" in function "join"
#  "LazyFrame[Any]" is not assignable to "narwhals.dataframe.DataFrame[polars.dataframe.frame.DataFrame]"

bad = df.join(df.lazy(), on="a")

LazyFrame[Any] can be assigned to BaseFrame[DataFrameT]

@dangotbanned dangotbanned Sep 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably wouldn't have picked this up were it not for this recent PR 😄

@dangotbanned dangotbanned Sep 6, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion

Just for this one, since it solves the two issues in https://github.com/narwhals-dev/narwhals/pull/3096/files#r2328197883

PolarsBaseFrame is fine as it's internal and the subclasses fill the constrained TypeVar anyway 🥳

diff --git a/narwhals/dataframe.py b/narwhals/dataframe.py
index 9cc5382c7..5b2752e02 100644
--- a/narwhals/dataframe.py
+++ b/narwhals/dataframe.py
@@ -91,6 +91,7 @@ if TYPE_CHECKING:
     )
 
     PS = ParamSpec("PS")
+    Incomplete: TypeAlias = Any
 
 _FrameT = TypeVar("_FrameT", bound="IntoFrame")
 LazyFrameT = TypeVar("LazyFrameT", bound="IntoLazyFrame")
@@ -284,7 +285,7 @@ class BaseFrame(Generic[_FrameT]):
 
     def join(
         self,
-        other: BaseFrame[_FrameT],
+        other: Incomplete,
         on: str | list[str] | None,
         how: JoinStrategy,
         *,
@@ -335,7 +336,7 @@ class BaseFrame(Generic[_FrameT]):
 
     def join_asof(
         self,
-        other: BaseFrame[_FrameT],
+        other: Incomplete,
         *,
         left_on: str | None,
         right_on: str | None,
@@ -1800,7 +1801,7 @@ class DataFrame(BaseFrame[DataFrameT]):
 
     def join(
         self,
-        other: BaseFrame[DataFrameT],
+        other: Self,
         on: str | list[str] | None = None,
         how: JoinStrategy = "inner",
         *,
@@ -1846,7 +1847,7 @@ class DataFrame(BaseFrame[DataFrameT]):
 
     def join_asof(
         self,
-        other: BaseFrame[DataFrameT],
+        other: Self,
         *,
         left_on: str | None = None,
         right_on: str | None = None,
@@ -3052,7 +3053,7 @@ class LazyFrame(BaseFrame[LazyFrameT]):
 
     def join(
         self,
-        other: BaseFrame[LazyFrameT],
+        other: Self,
         on: str | list[str] | None = None,
         how: JoinStrategy = "inner",
         *,
@@ -3107,7 +3108,7 @@ class LazyFrame(BaseFrame[LazyFrameT]):
 
     def join_asof(
         self,
-        other: BaseFrame[LazyFrameT],
+        other: Self,
         *,
         left_on: str | None = None,
         right_on: str | None = None,

@MarcoGorelli MarcoGorelli Sep 7, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems reasonable

@dangotbanned dangotbanned Sep 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in (be reasonable) 😉

on: str | list[str] | None = None,
how: JoinStrategy = "inner",
*,
Expand Down Expand Up @@ -1846,7 +1846,7 @@ def join(

def join_asof(
self,
other: Self,
other: BaseFrame[DataFrameT],
*,
left_on: str | None = None,
right_on: str | None = None,
Expand Down Expand Up @@ -3052,7 +3052,7 @@ def top_k(

def join(
self,
other: Self,
other: BaseFrame[LazyFrameT],
on: str | list[str] | None = None,
how: JoinStrategy = "inner",
*,
Expand Down Expand Up @@ -3107,7 +3107,7 @@ def join(

def join_asof(
self,
other: Self,
other: BaseFrame[LazyFrameT],
*,
left_on: str | None = None,
right_on: str | None = None,
Expand Down
4 changes: 2 additions & 2 deletions narwhals/stable/v1/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ def to_dict(self, *, as_series: Literal[True] = ...) -> dict[str, Series[Any]]:
def to_dict(self, *, as_series: Literal[False]) -> dict[str, list[Any]]: ...
@overload
def to_dict(
self, *, as_series: bool
self, *, as_series: bool = True
) -> dict[str, Series[Any]] | dict[str, list[Any]]: ...
def to_dict(
def to_dict( # pyright: ignore[reportIncompatibleMethodOverride]
self, *, as_series: bool = True
) -> dict[str, Series[Any]] | dict[str, list[Any]]:
# Type checkers complain that `nw.Series` is not assignable to `nw.v1.stable.Series`.
Expand Down
4 changes: 2 additions & 2 deletions narwhals/stable/v2/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@ def to_dict(self, *, as_series: Literal[True] = ...) -> dict[str, Series[Any]]:
def to_dict(self, *, as_series: Literal[False]) -> dict[str, list[Any]]: ...
@overload
def to_dict(
self, *, as_series: bool
self, *, as_series: bool = True
) -> dict[str, Series[Any]] | dict[str, list[Any]]: ...
def to_dict(
def to_dict( # pyright: ignore[reportIncompatibleMethodOverride]
self, *, as_series: bool = True
) -> dict[str, Series[Any]] | dict[str, list[Any]]:
# Type checkers complain that `nw.Series` is not assignable to `nw.v2.stable.Series`.
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ pythonPlatform = "All"
# NOTE (`pyarrow-stubs` do unsafe `TypeAlias` and `TypeVar` imports)
# pythonVersion = "3.9"
reportMissingTypeArgument = "error"
reportIncompatibleMethodOverride = "error"
reportMissingImports = "none"
reportMissingModuleSource = "none"
reportPrivateImportUsage = "none"
Expand Down
Loading