tie `IntoFrameT` to `NativeFrame` by MarcoGorelli · Pull Request #3496 · narwhals-dev/narwhals

MarcoGorelli · 2026-03-03T21:37:55Z

🤔 might this be a solution to #3493 ?

Description

What type of PR is this? (check all applicable)

Related issues

Related issue #<issue number>
Closes #<issue number>

Checklist

Code follows style guide (ruff)
Tests added
Documented the changes

MarcoGorelli · 2026-03-10T18:00:49Z

This one demonstrates the overlap issue 👎

yup, thanks for taking a look!

…als into another-typing-readme

MarcoGorelli · 2026-03-27T16:10:40Z

This one demonstrates the overlap issue 👎

Thanks! I think this is addressible with some extra specialised overloads, have pushed

What happens when we use incompatible parts of the *Frame APIs?:

Aren't these generally tested already? Any particular tests you'd like to see?

akmalsoliev · 2026-04-02T20:59:33Z

Hey folks, any update on this one? Drove me mad for 30 minutes till I realized it is pyrefly specific error

MarcoGorelli · 2026-04-03T11:49:22Z

I don't think it's pyrefly-specific, pyright flags it too

to respond to Dan's comment point by point:

collect(backend)

If you do collect, then you can't hope to have a IntoFrameT -> IntoFrameT function anyway

lazy(backend)

same

with_row_index()

This isn't an incompatible API, it returns Self, it's the same as using select or filter

join(other=<same|wrong-type|the-result-of-a-union>)

Yup, this one's problematic (as it currently is on main). It's not made worse as far as I can tell

Something I wouldn't be opposed to would be to change the LazyFrame.lazy API to be

def lazy(self) -> LazyFrame[Any]:

as, realistically, if you're calling .lazy on a lazyframe, it's because you don't know if it's lazy or eager

unique(maintain_order=True)

Also not an incompatible API, it returns Self

sink_parquet(...)

This returns None, so I'm not sure what there is to test here

MarcoGorelli · 2026-04-06T12:22:50Z

thanks @dangotbanned , have addressed most comments

Regarding the overlap, don't type checkers usually match the first valid overload?

At least, for

from typing import overload, Sequence, reveal_type

@overload
def foo(a: str) -> int: ...  # type: ignore[overload-overlap]

@overload
def foo(a: Sequence[str]) -> str: ...

def foo(a: str | Sequence[str]) -> int | str:
    if isinstance(a, str):
        return 0
    return 'zero'

reveal_type(foo('abc'))
reveal_type(foo(['abc']))

All of mypy, pyright, pyrefly, ty, and zuban, do exactly that. pandas and numpy both rely on this in their stubs, so I think this is unlikely to change?

(scratch) marcogorelli@DESKTOP-U8OKFP3:~/scratch$ mypy t.py
t.py:14: note: Revealed type is "int"
t.py:15: note: Revealed type is "str"
Success: no issues found in 1 source file
(scratch) marcogorelli@DESKTOP-U8OKFP3:~/scratch$ pyright t.py
/home/marcogorelli/scratch/t.py
  /home/marcogorelli/scratch/t.py:14:13 - information: Type of "foo('abc')" is "int"
  /home/marcogorelli/scratch/t.py:15:13 - information: Type of "foo(['abc'])" is "str"
0 errors, 0 warnings, 2 informations
(scratch) marcogorelli@DESKTOP-U8OKFP3:~/scratch$ pyrefly check t.py
 INFO revealed type: int [reveal-type]
  --> t.py:14:12
   |
14 | reveal_type(foo('abc'))
   |            ------------
   |
 INFO revealed type: str [reveal-type]
  --> t.py:15:12
   |
15 | reveal_type(foo(['abc']))
   |            --------------
   |
 INFO 0 errors
(scratch) marcogorelli@DESKTOP-U8OKFP3:~/scratch$ ty check t.py
info[revealed-type]: Revealed type
  --> t.py:14:13
   |
12 |     return 'zero'
13 |
14 | reveal_type(foo('abc'))
   |             ^^^^^^^^^^ `int`
15 | reveal_type(foo(['abc']))
   |

info[revealed-type]: Revealed type
  --> t.py:15:13
   |
14 | reveal_type(foo('abc'))
15 | reveal_type(foo(['abc']))
   |             ^^^^^^^^^^^^ `str`
   |

Found 2 diagnostics
(scratch) marcogorelli@DESKTOP-U8OKFP3:~/scratch$ zuban check t.py
t.py:14: note: Revealed type is "builtins.int"
t.py:15: note: Revealed type is "builtins.str"
Success: no issues found in 1 source file

MarcoGorelli · 2026-04-10T16:51:12Z

just checking if you still disagree with the approach of whether there can be a way forwards here?

dangotbanned · 2026-04-10T17:23:42Z

Hey Marco, I haven't got back to this yet - so this isn't a comment on the most recent changes - but appreciate you taking things on board 😊

just checking if you still disagree with the approach

In my last review I didn't mean to give off that impression, sorry for that 😔
It was more that I felt there were some unknowns and worried that not having the full picture now would mean we'd here again soon, fixing more from_native edge cases again.

That isn't to say my worries are correct, and I would much rather your changes turn out to be fine 🤞

I'll do my best to go through your changes this weekend, if that's okay?

One thing for now, re overloads (#3496 (comment))

I was thinking about this PR a couple of days ago when I saw this potential spec change:

Improve Step 5 of overload call evaluation. python/typing#2250

I think the 2nd spec change there is related - might be worth reading to see if (#3496 (comment)) would be handled differently?

MarcoGorelli · 2026-04-10T18:08:35Z

thanks! yeah no hurry at all, just checking if there was a potential way forwards or if you were firmly against it

btw, i think we're already relying on inconsistent overlapping overloads here

narwhals/narwhals/dataframe.py

Lines 1049 to 1057 in 25bdb8f

    
           # `str` overlaps with `Sequence[str]` 
        
           # We can ignore this but we must keep this overload ordering 
        
           @overload 
        
           def __getitem__(self, item: tuple[SingleIndexSelector, SingleColSelector]) -> Any: ... 
        
           @overload 
        
           def __getitem__(  # type: ignore[overload-overlap] 
        
               self, item: str | tuple[MultiIndexSelector, SingleColSelector] 
        
           ) -> Series[Any]: ...

dangotbanned · 2026-04-12T19:30:58Z

Regarding the overlap, don't type checkers usually match the first valid overload?
...
All of mypy, pyright, pyrefly, ty, and zuban, do exactly that. pandas and numpy both rely on this in their stubs, so I think this is unlikely to change?

I read through (python/typing#2250) and it looks like that would be non-conformant behavior:

It's worth noting that this is being proposed by a pyrefly maintainer.
But there looks to have been support from others in (python/typing#566 (comment)) and Spec change: ambiguous arguments in overload call evaluation

dangotbanned · 2026-04-12T19:46:59Z

+@overload
+def from_native(  # type: ignore[overload-overlap]
+    native_object: IntoFrameT, **kwds: Unpack[AllowLazy]
+) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...


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 🤔

MarcoGorelli · 2026-04-13T13:04:45Z

I read through (python/typing#2250) and it looks like that would be non-conformant behavior:

I'll ask, but my understanding is that it wouldn't be non-conformant because if you have

@overload
def foo(a: str) -> int: ...  # type: ignore[overload-overlap]

@overload
def foo(a: Sequence[str]) -> str: ...

def foo(a: str | Sequence[str]) -> int | str:
    ...

foo('foo')

then all materialisations of the a argument match the first overload, and so the remaining overloads can be eliminated

MarcoGorelli · 2026-04-13T13:39:52Z

To tie this back specifically to the spec, in step 5, it says

For all arguments, determine whether all possible materializations of the argument’s type are assignable to the corresponding parameter type for each of the remaining overloads. If so, eliminate all of the subsequent remaining overloads.

In this case, a='foo' is assignable to a: str and so the remaining overloads can be ignored

My reading of the linked issues (python/typing#566 (comment)) and Spec change: ambiguous arguments in overload call evaluation is that they're about different issues

dangotbanned · 2026-04-13T13:47:41Z

My reading of the linked issues (python/typing#566 (comment)) and Spec change: ambiguous arguments in overload call evaluation is that they're about different issues

I think this is the same issue

https://github.com/rchen152/typing/blob/137e3fcbcf112f1514e1d2eb8c236d67b78125b1/conformance/tests/overloads_evaluation.py#L442-L462

Which pyright currently infers to the first overload, and this is what I'm trying to say is something we shouldn't rely on because it is unspecified and soon to be specified as non-conformant behavior for a type checker to do

> # Step 5 eliminates the first overload because there exists a
> # materialization of `A[Any]` that is not assignable to `A[None]`. Step 6
> # picks the second overload.

Show pre-3.12 version

from typing import TypeVar, Generic, Any, assert_type, overload

T = TypeVar("T")


class A(Generic[T]):
    x: T

    def f(self) -> T:
        return self.x


@overload
def example12(x: A[None]) -> A[None]: ...


@overload
def example12(x: A[Any]) -> A[Any]: ...


def example12(x: A[Any]) -> A[Any]:
    return x


def check_example12(x: Any):
    # Step 5 eliminates the first overload because there exists a
    # materialization of `A[Any]` that is not assignable to `A[None]`. Step 6
    # picks the second overload.
    ret = example12(x)
    assert_type(ret, A[Any])  # "assert_type" mismatch: expected "A[Any]" but received "A[None] "Pylance(reportAssertTypeFailure)"

MarcoGorelli · 2026-04-13T13:52:40Z

That's dealing with a case where an argument of type Any is passed, which has a materialisation which doesn't match the first overload

In the example I gave, we're passing a string argument, in which every materialisation matches the first overload

@overload
def foo(a: str)

If you believe that the example I gave (#3496 (comment)) is non-conformant, may I please ask that you open a discussion on the typing discourse or on their github?

dangotbanned · 2026-04-13T14:40:51Z

If you believe that the example I gave #3496 (comment) is non-conformant

This PR is not about that example, although both are related to overlapping overloads.
I'm interested in specifically (#3496 (comment)), which is where the change to IntoFrameT has caused all the new issues.

By my count, IntoDataFrameT and IntoLazyFrameT (with Unpack[AllowLazy]) each can match 2 of these 3 overloads:

I believe this is why you see # type: ignore[overload-overlap] following the second two.
It's possible (#3496 (comment)) may be a path to avoiding the overlap.

At worst, it couldn't hurt to try?

I'm not sure what issue is being surpressed on this line:

https://github.com/MarcoGorelli/narwhals/blob/a32fb68cfabf99a0de3f8314a1d9bd26f1629eb2/narwhals/translate.py#L178

But I do know that it would also prevent other type checkers from showing a diagnostic there if they had one

So bringing it back to (#3496 (comment))
I appreciate that you've addressed most of my points and I've marked lots of things as resolved 🥳

But I currently feel:

That the warnings from type checkers are correct.
If we can find a solution that focuses on avoiding them, it would be best for us in the long-term
If this is working right now (which I feel is reliant on shaky grounds), it's reasonable to think it may soon not be

MarcoGorelli · 2026-04-13T16:21:48Z

This PR is not about that example

Sure, but just to check that we're on the same page - do you agree that that example is conformant or not?

By my count, IntoDataFrameT and IntoLazyFrameT (with Unpack[AllowLazy]) each can match 2 of these 3 overloads:

Yes, and all materialisations of IntoDataFrameT will match the second one (L154-L157), so the third one will be ignored (at least, that's my understanding)

It's possible (#3496 (comment)) may be a path to avoiding the overlap.
At worst, it couldn't hurt to try?

Yup, thanks 🙏 . Have tried

--git a/narwhals/translate.py b/narwhals/translate.py
index f9ef32461..5ed0bde06 100644
--- a/narwhals/translate.py
+++ b/narwhals/translate.py
@@ -148,17 +148,19 @@ def from_native(
     native_object: IntoLazyFrameT, **kwds: Unpack[AllowLazy]
 ) -> LazyFrame[IntoLazyFrameT]: ...
 @overload
-def from_native(  # type: ignore[overload-overlap]
+def from_native(
     native_object: IntoDataFrameT | IntoSeriesT, **kwds: Unpack[AllowSeries]
 ) -> DataFrame[IntoDataFrameT] | Series[IntoSeriesT]: ...
 @overload
 def from_native(
     native_object: IntoDataFrameT | IntoLazyFrameT, **kwds: Unpack[AllowLazy]
 ) -> DataFrame[IntoDataFrameT] | LazyFrame[IntoLazyFrameT]: ...
+# @overload
+# def from_native(  # type: ignore[overload-overlap]
+#     native_object: IntoFrameT, **kwds: Unpack[AllowLazy]
+# ) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
 @overload
-def from_native(  # type: ignore[overload-overlap]
-    native_object: IntoFrameT, **kwds: Unpack[AllowLazy]
-) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
+def from_native(native_object: IntoFrameT) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
 @overload
 def from_native(
     native_object: IntoDataFrameT | IntoLazyFrameT | IntoSeriesT, **kwds: Unpack[AllowAny]
@@ -175,7 +177,7 @@ def from_native(
     series_only: bool,
     allow_series: bool | None,
 ) -> Any: ...
-def from_native(  # type: ignore[misc]
+def from_native(
     native_object: IntoFrameT
     | IntoLazyFrameT
     | IntoDataFrameT

, and mypy reports

narwhals/translate.py:151: error: Overloaded function signatures 10 and 11 overlap with incompatible return types  [overload-overlap]
    def from_native(
    ^
narwhals/translate.py:180: error: Overloaded function implementation cannot produce return type of signature 12  [misc]
    def from_native(
    ^

MarcoGorelli · 2026-04-18T11:30:25Z

For reference i asked about that on the pyrefly discord and got the response

That seems correct to me! I'd expect this behavior (when the argument types are fully known, pick the first matching overload) to be pretty stable, if for no other reason than that the fallout would be pretty big if we tried to change it.

https://discord.com/channels/1319086885024567347/1494693618441519205/1494806090368679997

dangotbanned · 2026-04-18T12:40:02Z

For reference i asked about that on the pyrefly discord and got the response

Thanks for asking!

I think I'd need to join the discord to find out, but what was the question?

That seems correct to me! I'd expect this behavior (when the argument types are fully known, ...

I can't tell if this means they're talking about a case like Sequence[str] or including overlapping type variables too.

Edit:
Well I just looked and indeed this was an example for Sequence[str] again

from typing import overload, Sequence, reveal_type

@overload
def foo(a: str) -> int: ...  # type: ignore[overload-overlap]

@overload
def foo(a: Sequence[str]) -> str: ...

def foo(a: str | Sequence[str]) -> int | str:
    if isinstance(a, str):
        return 0
    return 'zero'

reveal_type(foo('abc'))  # Revealed type is "int"
reveal_type(foo(['abc']))  # Revealed type is "str"

In my head I wouldn't think a TypeVar is fully known, but IIRC there are some terms like that defined in the typing spec

Or maybe this is just saying the type doesn't contain Any or Unknown?

dangotbanned · 2026-04-18T12:46:01Z

#3496 (comment)

It's possible (#3496 (comment)) may be a path to avoiding the overlap.
At worst, it couldn't hurt to try?

Yup, thanks 🙏 . Have tried

Details

--git a/narwhals/translate.py b/narwhals/translate.py
index f9ef32461..5ed0bde06 100644
--- a/narwhals/translate.py
+++ b/narwhals/translate.py
@@ -148,17 +148,19 @@ def from_native(
     native_object: IntoLazyFrameT, **kwds: Unpack[AllowLazy]
 ) -> LazyFrame[IntoLazyFrameT]: ...
 @overload
-def from_native(  # type: ignore[overload-overlap]
+def from_native(
     native_object: IntoDataFrameT | IntoSeriesT, **kwds: Unpack[AllowSeries]
 ) -> DataFrame[IntoDataFrameT] | Series[IntoSeriesT]: ...
 @overload
 def from_native(
     native_object: IntoDataFrameT | IntoLazyFrameT, **kwds: Unpack[AllowLazy]
 ) -> DataFrame[IntoDataFrameT] | LazyFrame[IntoLazyFrameT]: ...
+# @overload
+# def from_native(  # type: ignore[overload-overlap]
+#     native_object: IntoFrameT, **kwds: Unpack[AllowLazy]
+# ) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
 @overload
-def from_native(  # type: ignore[overload-overlap]
-    native_object: IntoFrameT, **kwds: Unpack[AllowLazy]
-) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
+def from_native(native_object: IntoFrameT) -> DataFrame[IntoFrameT] | LazyFrame[IntoFrameT]: ...
 @overload
 def from_native(
     native_object: IntoDataFrameT | IntoLazyFrameT | IntoSeriesT, **kwds: Unpack[AllowAny]
@@ -175,7 +177,7 @@ def from_native(
     series_only: bool,
     allow_series: bool | None,
 ) -> Any: ...
-def from_native(  # type: ignore[misc]
+def from_native(
     native_object: IntoFrameT
     | IntoLazyFrameT
     | IntoDataFrameT

, and mypy reports

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

Thanks, really appreciate you trying that out.

narwhals/translate.py:180: error: Overloaded function implementation cannot produce return type of signature 12  [misc]
    def from_native(
    ^

Just so I understand, this error was already present - but ignored - and my suggestion didn't change that right?

MarcoGorelli · 2026-05-05T09:59:55Z

Yup, that's right!

We also have pyrefly running on the test suite now, and it's encouraging that that reports no issues with this pr

./.venv/bin/uv run --no-sync pyrefly check
 INFO Checking project configured at `/home/runner/work/narwhals/narwhals/pyproject.toml`
 INFO 0 errors (263 suppressed, 12 warnings not shown)

dangotbanned · 2026-05-18T11:51:03Z

What would this PR look like if we approached this as widening only LazyFrame[...]?

I can live with there being only one side that goes all loosey-goosey
TBH this would reflect the pyarrow and pandas situation better
The overloads would likely need to be rethought (maybe DataFrame[IntoDataFrame] gets priority?)
Might be easier for plugins, where I expect lazy-only is gonna be more common

Edit

The goal of this would not be to support IntoFrameT everywhere, but to make from_native to work for more cases than it does currently.
I think this goal is what the aim really is and IMO any changes to from_native are just gonna be balancing trade-offs

tie IntoFrameT to NativeFrame

a11807a

MarcoGorelli marked this pull request as ready for review March 4, 2026 08:07

MarcoGorelli added fix typing labels Mar 4, 2026

Merge branch 'main' into another-typing-readme

108c8c0

MarcoGorelli requested a review from dangotbanned March 8, 2026 11:08

This comment was marked as resolved.

Sign in to view

some extra overloads

97999f3

MarcoGorelli marked this pull request as draft March 10, 2026 18:00

MarcoGorelli added 9 commits March 12, 2026 12:19

wip: add extra test

9d85f33

add more overloads + test

890fb17

Merge branch 'another-typing-readme' of github.com:MarcoGorelli/narwh…

649077a

…als into another-typing-readme

Merge branch 'main' into another-typing-readme

6bc4dc1

compat

272bba0

Merge branch 'another-typing-readme' of github.com:MarcoGorelli/narwh…

a0cd526

…als into another-typing-readme

cvg

988e533

deps

8706378

cvg

4e330a8

MarcoGorelli marked this pull request as ready for review March 27, 2026 15:08

MarcoGorelli marked this pull request as draft March 27, 2026 15:09

MarcoGorelli added 3 commits March 27, 2026 15:10

Merge remote-tracking branch 'upstream/main' into another-typing-readme

d513368

remove unnecessary eager_only=True

273960e

Merge branch 'main' into another-typing-readme

e6f3bb5

MarcoGorelli marked this pull request as ready for review March 27, 2026 16:09

MarcoGorelli added 2 commits April 2, 2026 20:19

update v2 from_native too

0fa4ce1

Merge remote-tracking branch 'upstream/main' into another-typing-readme

3a07d2c

MarcoGorelli added 5 commits April 6, 2026 10:50

split out into_frame_t tests

cc089ed

add more assert_types to test_readme_example

30cadf4

Merge remote-tracking branch 'upstream/main' into another-typing-readme

98d3e44

compat

287be0a

remove unnecessary var-annotated ignore

78516e1

MarcoGorelli mentioned this pull request Apr 6, 2026

docs: use simpler readme example which avoids groupby #3536

Merged

10 tasks

MarcoGorelli added 3 commits April 10, 2026 15:01

Merge remote-tracking branch 'upstream/main' into another-typing-readme

6c20fc4

broaden union and return type in non-overloaded definition

44add0d

v2 too

a32fb68

dangotbanned reviewed Apr 12, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into another-typing-readme

1703834

MarcoGorelli mentioned this pull request May 5, 2026

typing: Narrow .lazy(...) return type when possible #3590

Open

MarcoGorelli added 2 commits May 8, 2026 14:40

Merge remote-tracking branch 'upstream/main' into another-typing-readme

8fa2519

Merge remote-tracking branch 'upstream/main' into another-typing-readme

b3eb0ee

Conversation

MarcoGorelli commented Mar 3, 2026

Description

What type of PR is this? (check all applicable)

Related issues

Checklist

Uh oh!

This comment was marked as resolved.

Uh oh!

MarcoGorelli commented Mar 10, 2026

Uh oh!

MarcoGorelli commented Mar 27, 2026

Uh oh!

akmalsoliev commented Apr 2, 2026

Uh oh!

MarcoGorelli commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarcoGorelli commented Apr 6, 2026

Uh oh!

MarcoGorelli commented Apr 10, 2026

Uh oh!

dangotbanned commented Apr 10, 2026

Uh oh!

MarcoGorelli commented Apr 10, 2026

Uh oh!

dangotbanned commented Apr 12, 2026

Uh oh!

dangotbanned Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli commented Apr 13, 2026

Uh oh!

MarcoGorelli commented Apr 13, 2026

Uh oh!

dangotbanned commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MarcoGorelli commented Apr 13, 2026

Uh oh!

dangotbanned commented Apr 13, 2026

Uh oh!

MarcoGorelli commented Apr 13, 2026

Uh oh!

MarcoGorelli commented Apr 18, 2026

Uh oh!

dangotbanned commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dangotbanned commented Apr 18, 2026

Uh oh!

MarcoGorelli commented May 5, 2026

Uh oh!

dangotbanned commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Edit

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MarcoGorelli commented Apr 3, 2026 •

edited

Loading

dangotbanned Apr 12, 2026 •

edited

Loading

dangotbanned commented Apr 13, 2026 •

edited

Loading

dangotbanned commented Apr 18, 2026 •

edited

Loading

dangotbanned commented May 18, 2026 •

edited

Loading