bpo-46571: improve `typing.no_type_check` to skip foreign objects by sobolevn · Pull Request #31042 · python/cpython
There are several changes:
- We now don't explicitly check for any base / sub types, because new name check covers it
- I've also checked that
no_type_checkdo not modify foreign functions. It was the same as withtypes - I've also covered
except TypeErrorinno_type_checkwith a simple test case, it was not covered at all - I also felt like adding
lambdatest is a good idea: becauselambdais a bit of both in class bodies: a function and an assignment
Are there any other cases we want to cover?
Here's a slightly evil corner case that this patch doesn't account for:
If I have a module typing_test.py, like so:
# typing_test.py class A: class AA: foo: int
Then, observe the following behaviour (with your patch applied):
>>> from typing import get_type_hints, no_type_check >>> import typing_test >>> get_type_hints(typing_test.A.AA) {'foo': <class 'int'>} >>> @no_type_check ... class A: ... AA = typing_test.A.AA ... ... >>> get_type_hints(typing_test.A.AA) {}
We might be able to get around this by looking at the __module__ attribute.
@AlexWaygood good one! Very evil!
I've addressed this.
@JelleZijlstra I agree that we should also fix classmethod / staticmethod problem. Because we declare to support all methods, not just instance methods.
The problem is that types.MethodType does not support attribute assignment:
>>> class Some: ... @staticmethod ... def st(x: int) -> int: ... ... @classmethod ... def cl(cls, y: int) -> int: ... >>> Some.st.__no_type_check__ = True >>> Some.cl.__no_type_check__ = True AttributeError: 'method' object has no attribute '__no_type_check__'
Ideas?
Moreover, do we need some special @property support?
@AlexWaygood good one! Very evil!
I've addressed this.
Doesn't look like you've pushed anything -- did you mean to? :)
Doesn't look like you've pushed anything -- did you mean to? :)
Not yet, I am still fighting classmethod 🙂
Probably
if isinstance(obj, types.MethodType): obj.__func__.__no_type_check__ = True
is the way to go 🤔
Will write some more tests to be sure.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Do you think this should be backported? I'm leaning no as it's a behavior change and nobody directly complained about the old behavior (looks like you found it while reviewing typing test coverage).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, a backport doesn't seem asked for.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters