Skip to content
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.

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

Enable generic NamedTuples #13396

Merged
merged 7 commits into from
Aug 15, 2022
Merged

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Aug 12, 2022

Fixes #685

This builds on top of some infra I added for recursive types (Ref #13297). Implementation is based on the idea in #13297 (comment). Generally it works well, but there are actually some problems for named tuples that are recursive. Special-casing them in maptype.py is a bit ugly, but I think this is best we can get at the moment.

An important note on runtime support for this: IIUC this should work in Python 3.11, and should work soon with typing_extensions on earlier versions. I didn't add any version checks here. If someone is interested in adding those, please do this.

@github-actions

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

OK, since I added call syntax support for TypedDicts I also add support for NamedTuples as well. One can use

NT = NamedTuple("NT", [("key", int), ("value", T)])

similarly to

NT = Tuple[int, T]

There is however one problem: I use logic from what we do for proper classes. And it looks like this allows re-using bound type variables (e.g. in nested classes). Interestingly, it is prohibited for type aliases. So they will now be an exception, but also type alias behavior IMO matches PEP 484 specification on this.

Anyway, I will not change this now to not break things, but at some point we will need to unify things on type variable binding and reuse.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Comment on lines +1273 to +1275
reveal_type(foo(nts, nti)) # N: Revealed type is "Tuple[builtins.int, builtins.object, fallback=__main__.NT[builtins.object]]"

reveal_type(foo(nti, x)) # N: Revealed type is "builtins.tuple[builtins.int, ...]"
Copy link
Member

Choose a reason for hiding this comment

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

The difference here (Tuple for fixed-length and builtins.tuple for variable-length) is unfortunate but that seems like a problem for a different PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC in error messages we format them nicely, it is only reveal_type() that reflects the internal representation.

@ilevkivskyi ilevkivskyi merged commit 8deeaf3 into python:master Aug 15, 2022
@ilevkivskyi ilevkivskyi deleted the generic-named-tuple branch August 15, 2022 09:04
@@ -1,8 +1,19 @@
from typing import Dict, List

import mypy.typeops
Copy link

Choose a reason for hiding this comment

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

This causes a circular import:

  File "venv/lib/python3.10/site-packages/mypy/report.py", line 20, in <module>
    from mypy import stats
  File "venv/lib/python3.10/site-packages/mypy/stats.py", line 13, in <module>
    from mypy.argmap import map_formals_to_actuals
  File "venv/lib/python3.10/site-packages/mypy/argmap.py", line 8, in <module>
    from mypy.maptype import map_instance_to_supertype
  File "venv/lib/python3.10/site-packages/mypy/maptype.py", line 3, in <module>
    import mypy.typeops
  File "venv/lib/python3.10/site-packages/mypy/typeops.py", line 15, in <module>
    from mypy.maptype import map_instance_to_supertype
ImportError: cannot import name 'map_instance_to_supertype' from partially initialized module 'mypy.maptype' (most likely due to a circular import) (venv/lib/python3.10/site-packages/mypy/maptype.py)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for reporting! Do you have a (preferably simple) repro for this?

Copy link

Choose a reason for hiding this comment

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

Simply importing mypy.report without mypyc enabled should trigger this issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

See #13809 Btw just curious, why do you need to import mypy.report directly?

Copy link

Choose a reason for hiding this comment

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

It's one of sanity checks in nixpkgs, to make sure mypy module is not broken.

ilevkivskyi added a commit that referenced this pull request Oct 4, 2022
This should help people who need to `import mypy.reports` directly, see
#13396 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support generic NamedTuples
3 participants