stubgen: Fix call-based namedtuple omitted from class bases #14680

hamdanal · 2023-02-11T15:08:11Z

Fix inheriting from a call-based collections.namedtuple / typing.NamedTuple definition that was omitted from the generated stub.

This automatically adds support for the call-based NamedTuple in general not only in class bases (Closes #13788).

An example before and after

Input:

import collections
import typing
from collections import namedtuple
from typing import NamedTuple

CollectionsCall = namedtuple("CollectionsCall", ["x", "y"])

class CollectionsClass(namedtuple("CollectionsClass", ["x", "y"])):
    def f(self, a):
        pass

class CollectionsDotClass(collections.namedtuple("CollectionsClass", ["x", "y"])):
    def f(self, a):
        pass

TypingCall = NamedTuple("TypingCall", [("x", int | None), ("y", int)])

class TypingClass(NamedTuple):
    x: int | None
    y: str

    def f(self, a):
        pass

class TypingClassWeird(NamedTuple("TypingClassWeird", [("x", int | None), ("y", str)])):
    z: float | None

    def f(self, a):
        pass

class TypingDotClassWeird(typing.NamedTuple("TypingClassWeird", [("x", int | None), ("y", str)])):
    def f(self, a):
        pass

Output diff (before and after):

diff --git a/before.pyi b/after.pyi
index c88530e2c..95ef843b4 100644
--- a/before.pyi
+++ b/after.pyi
@@ -1,26 +1,29 @@
+import typing
 from _typeshed import Incomplete
 from typing_extensions import NamedTuple

 class CollectionsCall(NamedTuple):
     x: Incomplete
     y: Incomplete

-class CollectionsClass:
+class CollectionsClass(NamedTuple('CollectionsClass', [('x', Incomplete), ('y', Incomplete)])):
     def f(self, a) -> None: ...

-class CollectionsDotClass:
+class CollectionsDotClass(NamedTuple('CollectionsClass', [('x', Incomplete), ('y', Incomplete)])):
     def f(self, a) -> None: ...

-TypingCall: Incomplete
+class TypingCall(NamedTuple):
+    x: int | None
+    y: int

 class TypingClass(NamedTuple):
     x: int | None
     y: str
     def f(self, a) -> None: ...

-class TypingClassWeird:
+class TypingClassWeird(NamedTuple('TypingClassWeird', [('x', int | None), ('y', str)])):
     z: float | None
     def f(self, a) -> None: ...

-class TypingDotClassWeird:
+class TypingDotClassWeird(typing.NamedTuple('TypingClassWeird', [('x', int | None), ('y', str)])):
     def f(self, a) -> None: ...

Fixes python#9901 Fixes python#13662 Fix inheriting from a call-based `collections.namedtuple` / `typing.NamedTuple` definition that was omitted from the generated stub. This automatically adds support for the call-based `NamedTuple` in general not only as a based class (Closes python#13788). <details> <summary>An example before and after</summary> Input: ```python import collections import typing from collections import namedtuple from typing import NamedTuple CollectionsCall = namedtuple("CollectionsCall", ["x", "y"]) class CollectionsClass(namedtuple("CollectionsClass", ["x", "y"])): def f(self, a): pass class CollectionsDotClass(collections.namedtuple("CollectionsClass", ["x", "y"])): def f(self, a): pass TypingCall = NamedTuple("TypingCall", [("x", int | None), ("y", int)]) class TypingClass(NamedTuple): x: int | None y: str def f(self, a): pass class TypingClassWeird(NamedTuple("TypingClassWeird", [("x", int | None), ("y", str)])): z: float | None def f(self, a): pass class TypingDotClassWeird(typing.NamedTuple("TypingClassWeird", [("x", int | None), ("y", str)])): def f(self, a): pass ``` Output diff (before and after): ```diff diff --git a/before.pyi b/after.pyi index c88530e2c..95ef843b4 100644 --- a/before.pyi +++ b/after.pyi @@ -1,26 +1,29 @@ +import typing from _typeshed import Incomplete from typing_extensions import NamedTuple class CollectionsCall(NamedTuple): x: Incomplete y: Incomplete -class CollectionsClass: +class CollectionsClass(NamedTuple('CollectionsClass', [('x', Incomplete), ('y', Incomplete)])): def f(self, a) -> None: ... -class CollectionsDotClass: +class CollectionsDotClass(NamedTuple('CollectionsClass', [('x', Incomplete), ('y', Incomplete)])): def f(self, a) -> None: ... -TypingCall: Incomplete +class TypingCall(NamedTuple): + x: int | None + y: int class TypingClass(NamedTuple): x: int | None y: str def f(self, a) -> None: ... -class TypingClassWeird: +class TypingClassWeird(NamedTuple('TypingClassWeird', [('x', int | None), ('y', str)])): z: float | None def f(self, a) -> None: ... -class TypingDotClassWeird: +class TypingDotClassWeird(typing.NamedTuple('TypingClassWeird', [('x', int | None), ('y', str)])): def f(self, a) -> None: ... ``` </details>

mypy/stubgen.py

hamdanal · 2023-03-10T21:10:09Z

I made the requested changes.
I also deleted the NamedTuplePrinter class that I added in an earlier commit and moved the logic to the existing AliasPrinter. This makes the code a bit cleaner.

ikonst · 2023-03-10T21:22:07Z

Curious how this change didn't invalidate a test

-and f"{callee.expr.name}.{callee.name}" in "collections.namedtuple"
+and f"{callee.expr.name}.{callee.name}" == "collections.namedtuple"

hamdanal · 2023-03-10T21:24:27Z

Curious how this change didn't invalidate a test

-and f"{callee.expr.name}.{callee.name}" in "collections.namedtuple"
+and f"{callee.expr.name}.{callee.name}" == "collections.namedtuple"

I think because "collections.namedtuple" in "collections.namedtuple" is True

mypy/stubgen.py

ikonst · 2023-03-10T21:43:15Z

I think because "collections.namedtuple" in "collections.namedtuple" is True

🤦 🤣

ikonst · 2023-03-10T22:24:02Z

(note that I'm not a maintainer, just a fellow contributor)

AlexWaygood · 2023-04-05T18:04:39Z

test-data/unit/stubgen.test

+from _typeshed import Incomplete
+from typing import NamedTuple
+
+class X(NamedTuple('X', [('a', Incomplete), ('b', Incomplete)])): ...


Hmm, in typeshed we prefer to avoid call-based namedtuples wherever possible, so we would usually do something like this for this kind of thing:

class _XBase(NamedTuple): a: Incomplete b: Incomplete class X(_XBase): ...

I'm guessing it might be hard to achieve that from stubgen, though?

I agree, call-based syntax isn't desirable. It is however tricky to get the conversion to a stub-only class definition right. In fact I tried briefly when I started with the PR then decided it is safer and much simpler to keep the call-based definition.

Note that even though this syntax is ugly/undesirable, it type-checks perfectly fine by both mypy and pyright and this PR still fixes the issue where stubgen generated wrong stubs without any warning. It also makes a posterior step of manual conversion to a class definition much simpler as the information is now there in the stub instead of having to grep the python source for all namedtuple bases in class definitions.

Having said that, I don't mind implementing this, whether in this PR or in a follow up one. I do like to get the opinion of a stubgen maintainer/expert before working on it though as it will require some work. If the answer is do it, I have a couple of questions regarding this step that I can ask then.

@AlexWaygood are you comfortable enough this change will be welcome so that I can work on it or should we ping someone?

I think it's fine to generate the stubs as in this PR. If you fix the merge conflict, I can review this PR.

JelleZijlstra · 2023-05-06T14:18:18Z

mypy/stubgen.py

+                        # Incomplete as field types
+                        fields_str = ", ".join(f"({f!r}, {t})" for f, t in nt_fields)
+                    else:
+                        # Invalid namedtuple() call, cannot determine fields


Looks like you generate NamedTuple(name, [Incomplete]) as the base in this case. That seems wrong, as it's an invalid type. I think we should make Incomplete the base in this case.

Also, is this code path tested?

Good call. I fixed it. The tests now cover the invalid namedtuple call code path.

hamdanal added 3 commits February 11, 2023 16:03

Fix state

593d66c

Improve named tuples detection

7578168

AlexWaygood added the topic-stubgen label Feb 12, 2023