bpo-46571: improve `typing.no_type_check` to skip foreign objects by sobolevn · Pull Request #31042 · python/cpython

@sobolevn

There are several changes:

  1. We now don't explicitly check for any base / sub types, because new name check covers it
  2. I've also checked that no_type_check do not modify foreign functions. It was the same as with types
  3. I've also covered except TypeError in no_type_check with a simple test case, it was not covered at all
  4. I also felt like adding lambda test is a good idea: because lambda is a bit of both in class bodies: a function and an assignment

Are there any other cases we want to cover?

https://bugs.python.org/issue46571

@sobolevn

@AlexWaygood

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

JelleZijlstra

@sobolevn

@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

@AlexWaygood good one! Very evil!

I've addressed this.

Doesn't look like you've pushed anything -- did you mean to? :)

@sobolevn

Doesn't look like you've pushed anything -- did you mean to? :)

Not yet, I am still fighting classmethod 🙂

@sobolevn

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.

@sobolevn

@sobolevn

JelleZijlstra

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).

gvanrossum

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.