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

Fix Unpack imported from typing #14378

Merged
merged 2 commits into from
Jan 2, 2023

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Jan 2, 2023

Add missing check for typing.Unpack to fix running with --python 3.11.

@github-actions

This comment has been minimized.

mypy/semanal.py Outdated
@@ -1881,7 +1881,7 @@ def analyze_unbound_tvar(self, t: Type) -> tuple[str, TypeVarLikeExpr] | None:
# It's bound by our type variable scope
return None
return unbound.name, sym.node
if sym and sym.fullname == "typing_extensions.Unpack":
if sym and sym.fullname in {"typing.Unpack", "typing_extensions.Unpack"}:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if sym and sym.fullname in {"typing.Unpack", "typing_extensions.Unpack"}:
if sym and sym.fullname in ("typing.Unpack", "typing_extensions.Unpack"):

Nit, but this is what most other similar checks look like and I suspect mypyc handles it better. It's likely also faster in non-compiled Python to use a tuple here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure about mypyc, but for non-compiled Python it's actually a toss-up for two items. However, starting with three, sets are usually better.

Copy link
Member

Choose a reason for hiding this comment

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

I ran some local benchmarks and set is faster even for two-element tuples when the element is the second one. Still, better to be consistent with the other mypy code.

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@JelleZijlstra JelleZijlstra self-assigned this Jan 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

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

@JelleZijlstra JelleZijlstra merged commit 5f480f3 into python:master Jan 2, 2023
@cdce8p cdce8p deleted the fix-typing-unpack-311 branch January 2, 2023 16:55
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.

2 participants