Process NameTuple and fix Union types using | by matejsp · Pull Request #13788 · python/mypy

matejsp · 2022-10-01T20:44:56Z

SubdivisionEntity = typing.NamedTuple(
    'SubdivisionEntity',
    [
        ('subdivision', str),
        ('entity', str),
    ],
)

from

SubdivisionEntity = Incomplete

to

class SubdivisionAffiliation(NamedTuple):
    subdivision: str
    entity: str

and small fix for: #12929 fixing missing Union import

[case testImportUnion]
def func(a: str | int) -> str | int:
  pass

[out]
from typing import Union

def func(a: Union[str, int]) -> Union[str, int]: ...

sobolevn · 2022-10-01T21:29:15Z

test-data/unit/stubgen.test


+[case testNamedtupleWithTypes]
+import collections, x
+X = typing.NamedTuple('X', [('a', str), ('b', str)])


Can you please also test a case like X = typing.NamedTuple('X', [('@', int)])?

@sobolevn, won't that fail at runtime?

I will fail at runtime:
E ValueError: Type names and field names must be valid identifiers: '@'

I have added isidentifier validation.

JelleZijlstra · 2022-10-01T23:53:48Z

mypy/stubgen.py

-            isinstance(callee, MemberExpr) and callee.name == "namedtuple"
+        return (
+            isinstance(callee, NameExpr)
+            and (callee.name.endswith("namedtuple") or callee.name.endswith("NamedTuple"))


Would this match something like notanamedtuple?

Yes I don't know what mypy support for "namedtuple" is, but I wanted the same behaviour for Namedtuple (NameExpr).

Original code was:

return (isinstance(callee, NameExpr) and callee.name.endswith("namedtuple")) or ( isinstance(callee, MemberExpr) and callee.name == "namedtuple"

I just added NamedTuple :)

collections.namedtuple vs typing.NamedTuple.

It seems that NameExpr can be qualified name:

self.name = name # Name referred to (may be qualified)

Both NameExpr and MemberExpr have .fullname attribute. Maybe it is better to use it instead?

Hm I tried ... but fullname is resolved only when running stubgen from cmd line but not when running unit tests (it is None) and all namedtuple tests failing. Any idea?

So for fullname I need _semanal tests otherwise parseonly is enabled for unit tests. I can switch to semanal for namedtuple tests but it seems excessive. Thoughts?

sobolevn · 2022-10-02T06:48:32Z

mypy/stubgen.py

-            isinstance(callee, MemberExpr) and callee.name == "namedtuple"
+        return (
+            isinstance(callee, NameExpr)
+            and (callee.name.endswith("namedtuple") or callee.name.endswith("NamedTuple"))


Both NameExpr and MemberExpr have .fullname attribute. Maybe it is better to use it instead?

sobolevn · 2022-10-02T06:50:46Z

test-data/unit/stubgen.test


+[case testNamedtupleUsingInvalidIdent]
+import collections, x
+X = collections.namedtuple('X', ['@'])


I don't think that it is correct. It just drops a field that is present in the original code.

I think a proper way to fix is to use X = collections.namedtuple('X', ['@']) in stubs.

There are several typeshed examples where we actually do this.

Generating X = collections.namedtuple('X', ['@', 'a', 'b', 'c']) in the stubs would create invalid stub code.
Well imho I prefer partial correctness (and correct code), since we could resolve other fields (a, b, c in my case) and skip incorrect one.

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>

sobolevn reviewed Oct 1, 2022

View reviewed changes

JelleZijlstra reviewed Oct 1, 2022

View reviewed changes

matejsp force-pushed the fix-stubgen branch from 3ba4ffd to e4c35ce Compare October 2, 2022 05:47

sobolevn reviewed Oct 2, 2022

View reviewed changes

Process NameTuple and fix Union types using |

d07c310

matejsp force-pushed the fix-stubgen branch from e4c35ce to d07c310 Compare October 3, 2022 23:05

Support for NamedTuple as parent of class

1852ff6

hamdanal mentioned this pull request Feb 11, 2023

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

Merged

JelleZijlstra closed this in #14680 May 6, 2023

JelleZijlstra closed this in 171e6f8 May 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Process NameTuple and fix Union types using |#13788

Process NameTuple and fix Union types using |#13788
matejsp wants to merge 2 commits intopython:masterfrom
matejsp:fix-stubgen

matejsp commented Oct 1, 2022

Uh oh!

sobolevn Oct 1, 2022

Uh oh!

AlexWaygood Oct 2, 2022

Uh oh!

matejsp Oct 2, 2022

Uh oh!

JelleZijlstra Oct 1, 2022

Uh oh!

matejsp Oct 2, 2022 •

edited

Loading

Uh oh!

sobolevn Oct 2, 2022

Uh oh!

matejsp Oct 2, 2022

Uh oh!

matejsp Oct 2, 2022

Uh oh!

sobolevn Oct 2, 2022

Uh oh!

sobolevn Oct 2, 2022

Uh oh!

matejsp Oct 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

matejsp commented Oct 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matejsp Oct 2, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matejsp Oct 2, 2022 •

edited

Loading