python / cpython

The Python programming language
https://www.python.org
Other
62.35k stars 29.94k forks source link

TypedDict subtypes ignore any other metaclasses in 3.9+ #89082

Closed sobolevn closed 2 years ago

sobolevn commented 3 years ago
BPO 44919
Nosy @ambv, @serhiy-storchaka, @sobolevn, @uriyyo
PRs
  • python/cpython#28057
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.9', '3.10', '3.11'] title = 'TypedDict subtypes ignore any other metaclasses in 3.9+' updated_at = user = 'https://github.com/sobolevn' ``` bugs.python.org fields: ```python activity = actor = 'lukasz.langa' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'sobolevn' dependencies = [] files = [] hgrepos = [] issue_num = 44919 keywords = ['patch'] message_count = 2.0 messages = ['399615', '402781'] nosy_count = 4.0 nosy_names = ['lukasz.langa', 'serhiy.storchaka', 'sobolevn', 'uriyyo'] pr_nums = ['28057'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue44919' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

    sobolevn commented 3 years ago

    Some context. I have a User class defined as a TypedDict:

    from typing import TypedDict
    
    class User(TypedDict):
        name: str
        registered: bool

    Now, I want to check if some dict is an instance of User like so: isinstance(my_dict, User). But, I can't. Because it raises TypeError('TypedDict does not support instance and class checks').

    Ok. Let's change __instancecheck__ method then. We can only do this in a metaclass:

    from typing import _TypedDictMeta
    
    class UserDictMeta(_TypedDictMeta):
        def __instancecheck__(cls, arg: object) -> bool:
            return (
                isinstance(arg, dict) and
                isinstance(arg.get('name'), str) and
                isinstance(arg.get('registered'), bool)
            )
    
    class UserDict(User, metaclass=UserDictMeta):
        ...

    It looks like it should work. It used to work like this in Python3.7 and Python3.8.

    But since Python3.9 it does not work. Let's try to debug what happens:

    print(type(UserDict))  # <class 'typing._TypedDictMeta'>
    print(UserDict.__instancecheck__(UserDict, {}))  # TypeError: TypedDict does not support instance and class checks

    It looks like my custom UserDictMeta is completely ignored. And I cannot change how __instancecheck__ behaves.

    I suspect that the reason is in these 2 lines: https://github.com/python/cpython/blob/ad0a8a9c629a7a0fa306fbdf019be63c701a8028/Lib/typing.py#L2384-L2385

    What's the most unclear in this behavior is that it does not match regular Python subclass patterns. Simplified example of the same behavior, using only primite types:

    class FirstMeta(type):
        def __instancecheck__(cls, arg: object) -> bool:
            raise TypeError('You cannot use this type in isinstance() call')
    
    class First(object, metaclass=FirstMeta):
        ...
    
    # User space:
    
    class MyClass(First): # this looks like a user-define TypedDict subclass
        ...
    
    class MySubClassMeta(FirstMeta):
        def __instancecheck__(cls, arg: object) -> bool:
            return True  # just an override example
    
    class MySubClass(MyClass, metaclass=MySubClassMeta):
        ...
    
    print(isinstance(1, MySubClass))  # True
    print(isinstance(1, MyClass))  # TypeError

    As you can see our MySubClassMeta works perfectly fine this way.

    I suppose that this is a bug in TypedDict, not a desired behavior. Am I correct?

    ambv commented 2 years ago

    The change to disallow this in Python 3.9 was deliberate, see BPO-40187 and its description.

    Your attempt to make isinstance() work with TypedDict subclasses goes directly against this design. In fact, PEP-589 explicitly says that:

    I'm -1 to allow this but I'll also wait for Serhiy to voice his opinion.

    JelleZijlstra commented 2 years ago

    Agree with @ambv, let's close this as rejected.