-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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 crash from NamedTuple placeholder #18351
Conversation
Fixes python#17396 I'm having trouble writing a regression test, but the following reproduces the issue nicely: ``` rm -rf repro mkdir repro mkdir repro/np echo 'from .arraysetops import UniqueAllResult' > repro/np/__init__.pyi echo ' from typing import Generic, NamedTuple, TypeVar from np import does_not_exist _SCT = TypeVar("_SCT", bound=does_not_exist) class UniqueAllResult(NamedTuple, Generic[_SCT]): values: int ' > repro/np/arraysetops.pyi touch repro/np/py.typed PYTHONPATH=repro mypy -c 'import np' ```
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit hacky, but I guess it may be a necessary evil. Two things:
- Is there the same issue with generic TypedDicts? If yes, please fix that as well (and maybe try adding a test)
- Can you verify that a pattern like this still works? (And maybe add a corresponding test if we don't have one):
T = TypeVar("T", bound="NT")
class NT(NamedTuple, Generic[T]):
parent: T
item: int
Thanks for the review!
|
See #18351 (review) Co-authored-by: ilevkivskyi
Hm, I think this requires further investigation. NamedTuples and TypedDicts are almost identical w.r.t. deferrals etc. This may indicate that there is a simpler/better fix, or there is something deeper going on. I would recommend adding some debugging prints in a repro that mirrors the NmaedTuple one, and see what types appear there (it may be that there are still some unresolved placeholders, but they don't cause a crash in case of TypedDicts for some reason). |
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
@hauntsaninja I think I see the non-hackish solution here! I don't know, however, whether this should apply to diff --git a/mypy/semanal_shared.py b/mypy/semanal_shared.py
index bdd01ef6a..9d3d4b2eb 100644
--- a/mypy/semanal_shared.py
+++ b/mypy/semanal_shared.py
@@ -364,6 +364,9 @@ class HasPlaceholders(BoolTypeQuery):
def __init__(self) -> None:
super().__init__(ANY_STRATEGY)
+ def visit_tuple_type(self, t: TupleType, /) -> bool:
+ return super().visit_tuple_type(t) or t.partial_fallback.accept(self)
+
def visit_placeholder_type(self, t: PlaceholderType) -> bool:
return True Your repro in this PR doesn't work for me, but the repro you posted on original issue still does (and passes with the patch above applied). It also heals #18582 crash and does not fail testGenericNamedTupleRecursiveBound! |
…8585) Fixes #18582. Fixes #17396. Supersedes #18351. --------- Co-authored-by: hauntsaninja <[email protected]>
…8585) Fixes #18582. Fixes #17396. Supersedes #18351. --------- Co-authored-by: hauntsaninja <[email protected]>
…8585) Fixes #18582. Fixes #17396. Supersedes #18351. --------- Co-authored-by: hauntsaninja <[email protected]>
Fixes #17396
I'm having trouble writing a regression test, but the following reproduces the issue nicely: