Implement TypeIs (PEP 742) #16898
Implement TypeIs (PEP 742) #16898
Conversation
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
| return {expr: TypeGuardedType(node.callee.type_guard)}, {} | ||
| else: | ||
| assert node.callee.type_is is not None | ||
| return conditional_types_to_typemaps( |
There was a problem hiding this comment.
This is the crucial part of the change, which implements the new type narrowing behavior. It's the same as isinstance(), a little way up in the same method.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Marc Mueller <[email protected]>
|
Thanks for the review! I applied the suggestions, and will deal with the larger issues in the next few days. |
This comment has been minimized.
This comment has been minimized.
|
Missed commenting on the PR title earlier. Could you update it to mention |
JelleZijlstra
changed the title
This comment has been minimized.
This comment has been minimized.
mypy/errorcodes.py
Outdated
| @@ -281,5 +281,11 @@ def __hash__(self) -> int: | |||
| sub_code_of=MISC, | |||
| ) | |||
|
|
|||
| TYPE_NARROWER_NOT_SUBTYPE: Final[ErrorCode] = ErrorCode( | |||
| "type-is-not-subtype", | |||
There was a problem hiding this comment.
I propose type-narrower-not-subtype, right now this error code can be misleading for readers: type is not a subtype?
It would be consistent with:
TYPE_NARROWER_NOT_SUBTYPE: Final = ErrorMessage(
"Narrowed type {} is not a subtype of input type {}", codes.TYPE_NARROWER_NOT_SUBTYPE
)There was a problem hiding this comment.
Maybe narrowed-type-not-subtype? Your suggestion sounds like TypeNarrower, which was a rejected name for teh feature.
test-data/unit/check-typeis.test
Outdated
| g(reveal_type(x)) # N: Revealed type is "Union[builtins.list[builtins.str], __main__.<subclass of "list" and "A">]" | ||
| [builtins fixtures/tuple.pyi] | ||
|
|
||
| [case testTypeIsMultipleCondition-xfail] |
There was a problem hiding this comment.
Can you please add a comment on why this is marked as xfail?
There was a problem hiding this comment.
I copied this from the TypeGuard test. Seems like the xfail is just because we generate a name with an extra 1 at the end, which looks like a typo:
Expected:
main:16: note: Revealed type is "__main__.<subclass of "Foo" and "Bar">"
main:21: note: Revealed type is "__main__.<subclass of "Foo" and "Bar">" (diff)
Actual:
main:16: note: Revealed type is "__main__.<subclass of "Foo" and "Bar">"
main:21: note: Revealed type is "__main__.<subclass of "Foo" and "Bar">1" (diff)
I will remove the xfail and make it pass.
| from typing import Callable, List, TypeVar | ||
| from typing_extensions import TypeIs | ||
|
|
||
| A = Callable[[object], TypeIs[List[T]]] |
There was a problem hiding this comment.
T is not defined, why does this test pass?
There was a problem hiding this comment.
This is a general issue with test cases that I've seen in other tests as well. I'm not sure if T is coming from the builtins stubs or whatever else. If you are really interested I could quickly try and search where this has happened.
Obviously still good to point it out and change it here.
There was a problem hiding this comment.
I think the root cause of this is that a normal builtins.pyi in typeshed defined _T (which does not get exported) wheres the test fixtures (like dict.pyi) typically use stuff like T (which does get exported).
It would probably be a good idea to change all fixtures, but that might be quite a bit of work.
There was a problem hiding this comment.
Added an explicit definition of T in this test. Agree that ideally we should fix this throughout the test suite.
|
And don't forget to add a new code to https://github.com/python/mypy/blob/master/test-data/unit/check-errorcodes.test |
|
Thanks @sobolevn! I have pushed some changes to address your review. |
|
Diff from mypy_primer, showing the effect of this PR on open source code: graphql-core (https://github.com/graphql-python/graphql-core)
+ src/graphql/execution/execute.py:638: error: Unused "type: ignore" comment [unused-ignore]
|
| @@ -1018,10 +1018,22 @@ def visit_callable_type(self, template: CallableType) -> list[Constraint]: | |||
| param_spec = template.param_spec() | |||
|
|
|||
| template_ret_type, cactual_ret_type = template.ret_type, cactual.ret_type | |||
| if template.type_guard is not None: | |||
| if template.type_guard is not None and cactual.type_guard is not None: | |||
There was a problem hiding this comment.
Nice, thanks! I think this should fix a thing I ran into while testing #16939
There was a problem hiding this comment.
Thanks again, one last suggestion: we can add a test case to check-incremental to ensure that cache works correctly for type narrowing. Especially, because TypeGuard is also missing from there: https://github.com/python/mypy/blob/master/test-data/unit/check-incremental.test
|
Thanks! I'll work on a separate PR with incremental tests for both TypeGuard and TypeIs. |
Cross-ref python/peps#3649.