Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add object.__subclasshook__ #9755

Merged
merged 1 commit into from
Feb 22, 2023
Merged

Conversation

NeilGirdhar
Copy link
Contributor

Consider:

from dataclasses import dataclass, is_dataclass
from abc import ABC
from typing import Any

class AwesomeDataclassInstance(ABC):
    def __init_subclass__(cls) -> None:
        super().__init_subclass__()
        dataclass(cls)

    @classmethod
    def __subclasshook__(cls, sub: Any, /) -> bool:
        return isinstance(sub, type) and is_dataclass(sub) or super().__subclasshook__(sub)

It will fail because __subclasshook__ is not found.

@NeilGirdhar NeilGirdhar force-pushed the subclasshook branch 3 times, most recently from d4b8c52 to 1858b0c Compare February 18, 2023 23:13
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One the one hand, this is incorrect: neither abc.ABC nor abc.ABCMeta define __subclasshook__. The reason why the super().__subclasshook__ call works is that it's actually builtins.object that defines a __subclasshook__ method:

>>> object.__dict__["__subclasshook__"]
<method '__subclasshook__' of 'object' objects>
>>> object.__subclasshook__(int)
NotImplemented

On the other hand, calling __subclasshook__ makes no sense unless you're doing it on a class that has ABCMeta as the metaclass. So I'm not sure I'd want to add it to the stub for builtins.object. So maybe this is an acceptable "white lie".

I'm curious what other maintainers think on this. In any event, if we are going to do this, I think a comment in the stub would be warranted, since it isn't actually defined in the abc.ABC class at runtime.

@NeilGirdhar
Copy link
Contributor Author

NeilGirdhar commented Feb 22, 2023

One the one hand, this is incorrect:

I'm also curious what the experts think. In my experience, it's better to be accurate (rather than the "white lie" that's here) since you don't know what someone might do. So, if you agree, I'll move the method to object.

On the other hand, why is this on object instead of on ABC? Isn't that just polluting object? Should we suggest that Python move this method to ABC?

@srittau
Copy link
Collaborator

srittau commented Feb 22, 2023

In my opinion, we should stay as close to the implementation as possible, except if there's a really good reason. I believe we should add object.__subclasshook__.

@JelleZijlstra
Copy link
Member

Agree, we should add object.__subclasshook__ in typeshed since it exists at runtime.

Possibly it shouldn't exist, but that's a decision to make on the CPython project. I don't understand the implementation of ABCs well enough to understand why the decision was made to add this method to object, but maybe there's a good reason.

@AlexWaygood
Copy link
Member

Sounds good, let's move it to builtins.object then.

@NeilGirdhar NeilGirdhar force-pushed the subclasshook branch 2 times, most recently from 461c7a1 to c3c8ba6 Compare February 22, 2023 22:37
@NeilGirdhar
Copy link
Contributor Author

Makes sense, done!

@github-actions

This comment has been minimized.

stdlib/builtins.pyi Outdated Show resolved Hide resolved
@AlexWaygood AlexWaygood changed the title Add ABC.__subclasshook__ Add object.__subclasshook__ Feb 22, 2023
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit 06755e1 into python:main Feb 22, 2023
@NeilGirdhar NeilGirdhar deleted the subclasshook branch February 22, 2023 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants