Skip to content

tie IntoFrameT to NativeFrame #3496

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

Open
MarcoGorelli wants to merge 27 commits into
base: main
Choose a base branch
from
Open

tie IntoFrameT to NativeFrame #3496

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a11807a
tie `IntoFrameT` to `NativeFrame`
MarcoGorelli Mar 3, 2026
108c8c0
Merge branch 'main' into another-typing-readme
MarcoGorelli Mar 5, 2026
97999f3
some extra overloads
MarcoGorelli Mar 10, 2026
9d85f33
wip: add extra test
MarcoGorelli Mar 12, 2026
890fb17
add more overloads + test
MarcoGorelli Mar 12, 2026
649077a
Merge branch 'another-typing-readme' of github.com:MarcoGorelli/narwh…
MarcoGorelli Mar 12, 2026
6bc4dc1
Merge branch 'main' into another-typing-readme
MarcoGorelli Mar 12, 2026
272bba0
compat
MarcoGorelli Mar 12, 2026
a0cd526
Merge branch 'another-typing-readme' of github.com:MarcoGorelli/narwh…
MarcoGorelli Mar 12, 2026
988e533
cvg
MarcoGorelli Mar 12, 2026
8706378
deps
MarcoGorelli Mar 12, 2026
4e330a8
cvg
MarcoGorelli Mar 12, 2026
d513368
Merge remote-tracking branch 'upstream/main' into another-typing-readme
MarcoGorelli Mar 27, 2026
273960e
remove unnecessary `eager_only=True`
MarcoGorelli Mar 27, 2026
e6f3bb5
Merge branch 'main' into another-typing-readme
MarcoGorelli Mar 27, 2026
0fa4ce1
update v2 from_native too
MarcoGorelli Apr 2, 2026
3a07d2c
Merge remote-tracking branch 'upstream/main' into another-typing-readme
MarcoGorelli Apr 2, 2026
22520ac
test some incompatible apis
MarcoGorelli Apr 3, 2026
ca8e0ac
revert LazyFrame.lazy -> LazyFrame[Any]
MarcoGorelli Apr 4, 2026
cc089ed
split out into_frame_t tests
MarcoGorelli Apr 6, 2026
30cadf4
add more assert_types to `test_readme_example`
MarcoGorelli Apr 6, 2026
98d3e44
Merge remote-tracking branch 'upstream/main' into another-typing-readme
MarcoGorelli Apr 6, 2026
287be0a
compat
MarcoGorelli Apr 6, 2026
78516e1
remove unnecessary var-annotated ignore
MarcoGorelli Apr 6, 2026
6c20fc4
Merge remote-tracking branch 'upstream/main' into another-typing-readme
MarcoGorelli Apr 10, 2026
44add0d
broaden union and return type in non-overloaded definition
MarcoGorelli Apr 10, 2026
a32fb68
v2 too
MarcoGorelli Apr 10, 2026
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
27 changes: 11 additions & 16 deletions narwhals/_compliant/dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
CompliantSeriesT,
EagerExprT,
EagerSeriesT,
NativeDataFrameT,
NativeLazyFrameT,
NativeFrameT,
NativeSeriesT,
)
from narwhals._translate import (
Expand Down Expand Up @@ -179,8 +178,8 @@ class CompliantDataFrame(
DictConvertible["_ToDict[CompliantSeriesT]", Mapping[str, Any]],
ArrowConvertible["pa.Table", "IntoArrowTable"],
Sized,
CompliantFrame[CompliantExprT_contra, NativeDataFrameT, ToNarwhalsT_co],
Protocol[CompliantSeriesT, CompliantExprT_contra, NativeDataFrameT, ToNarwhalsT_co],
CompliantFrame[CompliantExprT_contra, NativeFrameT, ToNarwhalsT_co],
Protocol[CompliantSeriesT, CompliantExprT_contra, NativeFrameT, ToNarwhalsT_co],
):
def __narwhals_dataframe__(self) -> Self: ...
@classmethod
Expand Down Expand Up @@ -292,8 +291,8 @@ def write_parquet(self, file: str | Path | BytesIO) -> None: ...


class CompliantLazyFrame(
CompliantFrame[CompliantExprT_contra, NativeLazyFrameT, ToNarwhalsT_co],
Protocol[CompliantExprT_contra, NativeLazyFrameT, ToNarwhalsT_co],
CompliantFrame[CompliantExprT_contra, NativeFrameT, ToNarwhalsT_co],
Protocol[CompliantExprT_contra, NativeFrameT, ToNarwhalsT_co],
):
def __narwhals_lazyframe__(self) -> Self: ...
# `LazySelectorNamespace._iter_columns` depends
Expand All @@ -312,24 +311,20 @@ def sink_parquet(self, file: str | Path | BytesIO) -> None: ...


class EagerDataFrame(
CompliantDataFrame[
EagerSeriesT, EagerExprT, NativeDataFrameT, "DataFrame[NativeDataFrameT]"
],
CompliantLazyFrame[EagerExprT, "Incomplete", "DataFrame[NativeDataFrameT]"],
CompliantDataFrame[EagerSeriesT, EagerExprT, NativeFrameT, "DataFrame[NativeFrameT]"],
CompliantLazyFrame[EagerExprT, "Incomplete", "DataFrame[NativeFrameT]"],
ValidateBackendVersion,
Protocol[EagerSeriesT, EagerExprT, NativeDataFrameT, NativeSeriesT],
Protocol[EagerSeriesT, EagerExprT, NativeFrameT, NativeSeriesT],
):
@property
def _backend_version(self) -> tuple[int, ...]:
return self._implementation._backend_version()

def __narwhals_namespace__(
self,
) -> EagerNamespace[
Self, EagerSeriesT, EagerExprT, NativeDataFrameT, NativeSeriesT
]: ...
) -> EagerNamespace[Self, EagerSeriesT, EagerExprT, NativeFrameT, NativeSeriesT]: ...

def to_narwhals(self) -> DataFrame[NativeDataFrameT]:
def to_narwhals(self) -> DataFrame[NativeFrameT]:
return self._version.dataframe(self, level="full")

def aggregate(self, *exprs: EagerExprT) -> Self: # pyright: ignore[reportIncompatibleMethodOverride]
Expand All @@ -343,7 +338,7 @@ def aggregate(self, *exprs: EagerExprT) -> Self: # pyright: ignore[reportIncomp
return self.select(*exprs) # pyright: ignore[reportArgumentType]

def _with_native(
self, df: NativeDataFrameT, *, validate_column_names: bool = True
self, df: NativeFrameT, *, validate_column_names: bool = True
) -> Self: ...

def _check_columns_exist(self, subset: Sequence[str]) -> ColumnNotFoundError | None:
Expand Down
4 changes: 2 additions & 2 deletions narwhals/_native.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ def dropDuplicatesWithinWatermark(self, *arg: Any, **kwargs: Any) -> Any: ... #
NativePySparkConnect: TypeAlias = _PySparkDataFrame
NativeSparkLike: TypeAlias = "NativeSQLFrame | NativePySpark | NativePySparkConnect"
NativeKnown: TypeAlias = "NativePolars | NativeArrow | NativePandasLike | NativeSparkLike | NativeDuckDB | NativeDask | NativeIbis"
NativeUnknown: TypeAlias = "NativeDataFrame | NativeSeries | NativeLazyFrame"
NativeUnknown: TypeAlias = "NativeFrame | NativeSeries"
NativeAny: TypeAlias = "NativeKnown | NativeUnknown"

IntoDataFrame: TypeAlias = NativeDataFrame
Expand All @@ -304,7 +304,7 @@ def dropDuplicatesWithinWatermark(self, *arg: Any, **kwargs: Any) -> Any: ... #
"""

IntoLazyFrame: TypeAlias = Union[NativeLazyFrame, NativeIbis]
IntoFrame: TypeAlias = Union[IntoDataFrame, IntoLazyFrame]
IntoFrame: TypeAlias = NativeFrame
"""Anything which can be converted to a Narwhals DataFrame or LazyFrame.

Use this if your function can accept an object which can be converted to either
Expand Down
20 changes: 10 additions & 10 deletions narwhals/dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
PS = ParamSpec("PS")
Incomplete: TypeAlias = Any

_FrameT = TypeVar("_FrameT", bound="IntoFrame")
FrameT = TypeVar("FrameT", bound="IntoFrame")
LazyFrameT = TypeVar("LazyFrameT", bound="IntoLazyFrame")
DataFrameT = TypeVar("DataFrameT", bound="IntoDataFrame")
R = TypeVar("R")
Expand All @@ -104,7 +104,7 @@
MultiIndexSelector: TypeAlias = "_MultiIndexSelector[Series[Any]]"


class BaseFrame(Generic[_FrameT]):
class BaseFrame(Generic[FrameT]):
_compliant_frame: Any
_level: Literal["full", "lazy", "interchange"]

Expand Down Expand Up @@ -447,7 +447,7 @@ def explode(self, columns: str | Sequence[str], *more_columns: str) -> Self:
return self._with_compliant(self._compliant_frame.explode(columns=to_explode))


class DataFrame(BaseFrame[DataFrameT]):
class DataFrame(BaseFrame[FrameT]):
"""Narwhals DataFrame, backed by a native eager dataframe.

Warning:
Expand Down Expand Up @@ -475,7 +475,7 @@ class DataFrame(BaseFrame[DataFrameT]):
_version: ClassVar[Version] = Version.MAIN

@property
def _compliant(self) -> CompliantDataFrame[Any, Any, DataFrameT, Self]:
def _compliant(self) -> CompliantDataFrame[Any, Any, FrameT, Self]:
return self._compliant_frame

@property
Expand All @@ -492,7 +492,7 @@ def _validate_metadata(self, metadata: ExprMetadata) -> None:

def __init__(self, df: Any, *, level: Literal["full", "lazy", "interchange"]) -> None:
self._level: Literal["full", "lazy", "interchange"] = level
self._compliant_frame: CompliantDataFrame[Any, Any, DataFrameT, Self]
self._compliant_frame: CompliantDataFrame[Any, Any, FrameT, Self]
if is_compliant_dataframe(df):
self._compliant_frame = df.__narwhals_dataframe__()
else: # pragma: no cover
Expand Down Expand Up @@ -871,7 +871,7 @@ def lazy(
msg = f"Not-supported backend.\n\nExpected one of {get_args(_LazyAllowedImpl)} or `None`, got {lazy_backend}"
raise ValueError(msg)

def to_native(self) -> DataFrameT:
def to_native(self) -> FrameT:
"""Convert Narwhals DataFrame to native one.

Examples:
Expand Down Expand Up @@ -2348,7 +2348,7 @@ def explode(self, columns: str | Sequence[str], *more_columns: str) -> Self:
return super().explode(columns, *more_columns)


class LazyFrame(BaseFrame[LazyFrameT]):
class LazyFrame(BaseFrame[FrameT]):
"""Narwhals LazyFrame, backed by a native lazyframe.

Warning:
Expand All @@ -2362,7 +2362,7 @@ class LazyFrame(BaseFrame[LazyFrameT]):
"""

@property
def _compliant(self) -> CompliantLazyFrame[Any, LazyFrameT, Self]:
def _compliant(self) -> CompliantLazyFrame[Any, FrameT, Self]:
return self._compliant_frame

@property
Expand Down Expand Up @@ -2395,7 +2395,7 @@ def _validate_metadata(self, metadata: ExprMetadata) -> None:

def __init__(self, df: Any, *, level: Literal["full", "lazy", "interchange"]) -> None:
self._level = level
self._compliant_frame: CompliantLazyFrame[Any, LazyFrameT, Self]
self._compliant_frame: CompliantLazyFrame[Any, FrameT, Self]
if is_compliant_lazyframe(df):
self._compliant_frame = df.__narwhals_lazyframe__()
else: # pragma: no cover
Expand Down Expand Up @@ -2478,7 +2478,7 @@ def collect(
msg = f"Unsupported `backend` value.\nExpected one of {get_args(_LazyFrameCollectImpl)} or None, got: {eager_backend}."
raise ValueError(msg)

def to_native(self) -> LazyFrameT:
def to_native(self) -> FrameT:
"""Convert Narwhals LazyFrame to native one.

Examples:
Expand Down
21 changes: 12 additions & 9 deletions narwhals/translate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
Frame,
IntoDataFrameT,
IntoFrame,
IntoFrameT,
IntoLazyFrameT,
IntoSeries,
IntoSeriesT,
Expand All @@ -70,12 +71,12 @@

@overload
def to_native(
narwhals_object: DataFrame[IntoDataFrameT], *, pass_through: Literal[False] = ...
) -> IntoDataFrameT: ...
narwhals_object: DataFrame[IntoFrameT], *, pass_through: Literal[False] = ...
) -> IntoFrameT: ...
@overload
def to_native(
narwhals_object: LazyFrame[IntoLazyFrameT], *, pass_through: Literal[False] = ...
) -> IntoLazyFrameT: ...
narwhals_object: LazyFrame[IntoFrameT], *, pass_through: Literal[False] = ...
) -> IntoFrameT: ...
@overload
def to_native(
narwhals_object: Series[IntoSeriesT], *, pass_through: Literal[False] = ...
Expand All @@ -85,12 +86,10 @@ def to_native(narwhals_object: Any, *, pass_through: bool) -> Any: ...


def to_native(
narwhals_object: DataFrame[IntoDataFrameT]
| LazyFrame[IntoLazyFrameT]
| Series[IntoSeriesT],
narwhals_object: DataFrame[IntoFrameT] | LazyFrame[IntoFrameT] | Series[IntoSeriesT],
*,
pass_through: bool = False,
) -> IntoDataFrameT | IntoLazyFrameT | IntoSeriesT | Any:
) -> IntoFrameT | IntoSeriesT | Any:
"""Convert Narwhals object to native one.

Arguments:
Expand Down Expand Up @@ -145,6 +144,10 @@ def from_native(
native_object: IntoLazyFrameT, **kwds: Unpack[AllowLazy]
) -> LazyFrame[IntoLazyFrameT]: ...
@overload
def from_native( # type: ignore[overload-overlap]
Copy link
Copy Markdown
Member

@dangotbanned dangotbanned Mar 9, 2026

Choose a reason for hiding this comment

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

?

Copy link
Copy Markdown
Member Author

@MarcoGorelli MarcoGorelli Mar 10, 2026

Choose a reason for hiding this comment

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

🤷 pyright's fine with it but mypy outputs

narwhals/translate.py:147: error: Overloaded function signatures 9 and 10 overlap with incompatible return types  [overload-overlap]
    def from_native(
    ^
narwhals/translate.py:147: error: Overloaded function signatures 9 and 11 overlap with incompatible return types  [overload-overlap]
    def from_native(

Copy link
Copy Markdown
Member

@dangotbanned dangotbanned Apr 4, 2026

Choose a reason for hiding this comment

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

I'm not feeling convinced by this or (#3496 (comment))
AFAICT, both are the same issue reported by two different type checkers.

If you remove all the ignore comments, does pyrefly not report these as an issue as well?
For ty, we'd need (astral-sh/ty#103) to get it reported - so probably nothing there yet

native_object: IntoFrameT, **kwds: Unpack[AllowLazy]
) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
Comment on lines +158 to +161
Copy link
Copy Markdown
Member

@dangotbanned dangotbanned Apr 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know for sure if it'll work, but one way to make the overload defs non-overlapping might be:

@overload
def from_native(native_object: IntoFrameT) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...

Since AllowLazy is using total=False, that might not be enough though 🤔

@overload
def from_native(
native_object: IntoDataFrameT | IntoSeriesT, **kwds: Unpack[AllowSeries]
) -> DataFrame[IntoDataFrameT] | Series[IntoSeriesT]: ...
Expand All @@ -164,7 +167,7 @@ def from_native(
series_only: bool,
allow_series: bool | None,
) -> Any: ...
def from_native(
def from_native( # pyright: ignore[reportInconsistentOverload]
Copy link
Copy Markdown
Member

@dangotbanned dangotbanned Mar 9, 2026

Choose a reason for hiding this comment

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

?

Copy link
Copy Markdown
Member Author

@MarcoGorelli MarcoGorelli Mar 10, 2026

Choose a reason for hiding this comment

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

pyright gives

  /home/marcogorelli/polars-api-compat-dev/narwhals/translate.py:170:5 - error: Overloaded implementation is not consistent with signature of overload 9
    Function return type "DataFrame[IntoFrameT@from_native] | LazyFrame[IntoFrameT@from_native]" is incompatible with type "LazyFrame[IntoLazyFrameT@from_native] | DataFrame[IntoDataFrameT@from_native] | Series[IntoSeriesT@from_native] | IntoFrameT@from_native"
      Type "DataFrame[IntoFrameT@from_native] | LazyFrame[IntoFrameT@from_native]" is not assignable to type "LazyFrame[IntoLazyFrameT@from_native] | DataFrame[IntoDataFrameT@from_native] | Series[IntoSeriesT@from_native] | IntoFrameT@from_native"
        Type "DataFrame[IntoFrameT@from_native]" is not assignable to type "LazyFrame[IntoLazyFrameT@from_native] | DataFrame[IntoDataFrameT@from_native] | Series[IntoSeriesT@from_native] | IntoFrameT@from_native"
          Type "DataFrame[IntoFrameT@from_native]" is not assignable to type "IntoFrameT@from_native"
          "DataFrame[IntoFrameT@from_native]" is not assignable to "DataFrame[IntoDataFrameT@from_native]"
            Type parameter "FrameT@DataFrame" is invariant, but "IntoFrameT@from_native" is not the same as "IntoDataFrameT@from_native"
          "DataFrame[IntoFrameT@from_native]" is not assignable to "LazyFrame[IntoLazyFrameT@from_native]"
          "DataFrame[IntoFrameT@from_native]" is not assignable to "Series[IntoSeriesT@from_native]" (reportInconsistentOverload)
1 error, 0 warnings, 0 informations
make: *** [Makefile:25: typing] Error 1

given that this part isn't user-facing (the overloads are used) i didn't it was too big of a deal

Copy link
Copy Markdown
Member

@dangotbanned dangotbanned Apr 4, 2026

Choose a reason for hiding this comment

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

given that this part isn't user-facing (the overloads are used) i didn't it was too big of a deal

How each type checker deals will overlapping overloads is a bit hairy.
The typing spec says this about solving overloads at the callsite

If the return types are not equivalent, overload matching is ambiguous.
In this case, assume a return type of Any and stop.

AFAICT, there isn't a spec for overlapping definitions 😢
So the conformance tests focus on just the usage part.

This error

Both mypy and pyright are trying to tell you that:

As a parameter, IntoFrameT matches:

IntoDataFrameT | IntoLazyFrameT
IntoFrameT

But the return types do not match:

DataFrame[IntoDataFrameT] | LazyFrame[IntoLazyFrameT]
DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]

  • I think mypy does the Any thing?
  • I'm more sure that pyright would synthesize a signature with a * in it for inference
  • (I'd assume) ty uses intersections

Important

The user facing problems I've experienced is that:

  1. it makes type checkers run much slower, since they need to track 2 or more overloads that matched
  2. this can make a language server produce flaky errors, since each run might take a different route

I left a screen capture in (zen-xu/pyarrow-stubs#208 (comment)) with an extreme example or how slow it can get 😓

native_object: IntoLazyFrameT
| IntoDataFrameT
| IntoSeriesT
Expand Down
Loading