Fix isinstance on namedtuple type aliases#21103
Fix isinstance on namedtuple type aliases#21103paulorochaoliveira wants to merge 2 commits intopython:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
| @@ -557,6 +557,10 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) -> | |||
| and node | |||
| and isinstance(node.node, TypeAlias) | |||
| and not node.node.no_args | |||
There was a problem hiding this comment.
I feel like no_args should be True in this case? In which case this if statement is fine.
I'd rather that, unless there's a good reason for existing behavior...
There was a problem hiding this comment.
@A5rocks, I considered that, but no_args currently seems to mean more than “this alias was written without subscripting”.
In particular, it appears to assume an Instance target in a few places:
mypy/types.py(TypeAliasType._expand_once)mypy/typeanal.py(instantiate_type_alias)- the alias creation logic in
mypy/semanal.py
For aliases to classes inheriting from NamedTuple, the target is a TupleType with a namedtuple fallback rather than an Instance, so making no_args=True here would likely require broadening that invariant first.
Because of that, I went with the narrower fix in the isinstance check path. If you’d prefer, I can rework this to make no_args support namedtuple-backed aliases more generally, but that seemed like a larger behavioral change than needed for this bug.
There was a problem hiding this comment.
That's interesting! I had assumed this already worked for tuples but actually they don't work at runtime! I guess I agree that no_args is the wrong approach then. This still feels a bit hacky though so I'll think a bit more... But as a first comment, could you add a test to show that you didn't modify the following output?:
from typing import Any, TypeAlias
Alias: TypeAlias = tuple[int, int]
def is_foo(x: Any) -> bool:
return isinstance(x, Alias)(your code shouldn't have changed that, but unless that's already a test it's better to prevent people like me from modifying this without knowing the runtime behavior!)
A5rocks
left a comment
There was a problem hiding this comment.
Actually I read the context and this is already what we do for unions, so I'm happy with this.
Summary
Fix #21100
Fix a false positive where
isinstance(x, Alias)reportsParameterized generics cannot be used with class or instance checkswhen
Aliasis aTypeAliasto a class inheriting fromNamedTuple.Changes
isinstancerejection pathTypeAlias = FoowhereFoois aNamedTupleTesting
./venv/bin/pytest -n0 mypy/test/testcheck.py -k "testIsinstanceTypeAliasToNamedTuple or testIsinstanceTypeArgsAliases or testAliasNonGeneric"./venv/bin/pytest -n0 mypy/test/testcheck.py -k check-isinstance.test