This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Bug in isinstance(instance, cls) with cls being a protocol? (PEP 544)
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Jukka Lehtosalo, gvanrossum, kj, levkivskyi, lukasz.langa, paul-dest
Priority: normal Keywords:

Created on 2021-03-16 10:02 by paul-dest, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Messages (10)
msg388827 - (view) Author: Paul (paul-dest) Date: 2021-03-16 10:02
The section "Subtyping relationships with other types" of PEP 544 states:

"A concrete type X is a subtype of protocol P if and only if X implements all protocol members of P with compatible types. In other words, subtyping with respect to a protocol is always structural."

This requirement is violated by the current implementation of CPython (version 3.9.2):

```
from typing import Protocol


class P(Protocol):
    pm: str  # no default value, but still a protocol member


class C(P):
    # inherits P but does NOT implement pm, since P did not provide a default value
    pass

    
assert isinstance(C(), P)  # violates the PEP 544 requirement cited above

C().pm  # raises: AttributeError: 'C' object has no attribute 'pm'
```
msg388851 - (view) Author: Ken Jin (kj) * (Python committer) Date: 2021-03-16 16:01
Apologies if I misunderstood something, but doesn't PEP 544 also state in its "Rationale", "Non-goals" subsection that

"""
At runtime, protocol classes will be simple ABCs. There is no intent to provide sophisticated runtime instance and class checks against protocol classes. This would be difficult and error-prone and will contradict the logic of PEP 484.
"""

https://www.python.org/dev/peps/pep-0544/#non-goals

Which means that the Python runtime isn't supposed to check that, instead it's the static type checker's responsibility?
msg388860 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-16 17:06
Yeah, the runtime isinstance() checking is not required to give the right answer. Also, I believe one of the rules is "if you inherit from a protocol you are deemed to implement it". For methods, abstract methods can (but needn't) be used to indicate mandatory features. For attributes, there is no guarantee.
msg388908 - (view) Author: Paul (paul-dest) Date: 2021-03-17 06:23
That's the very first issue I've reported in bugs.python.org and I'm completely new to the Python dev process:

I have some further remarks at the issue (especially about consistency with the current treatment of Protocols vs. ABCs). Will they be read if placed here after the issue has been closed? Or should I (a) open a new issue or (b) change the status of this issue to "open" first?
msg388926 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-17 14:55
Yes, discussion on a close issue is still read, unless people explicitly
unsubscribe (which they rarely do unless the chatter is spam).
msg389008 - (view) Author: Paul (paul-dest) Date: 2021-03-18 10:45
Regarding "At runtime, protocol classes will be simple ABCs." (PEP 544):
Unfortunately, this is currently not the case. Actually, there is an extra metaclass for protocols, solely to provide an __instancecheck__.
https://github.com/python/cpython/blob/3.9/Lib/typing.py#L1096

```
class _ProtocolMeta(ABCMeta):
    # This metaclass is really unfortunate and exists only because of
    # the lack of __instancehook__.
    def __instancecheck__(cls, instance):
        # We need this method for situations where attributes are
        # assigned in __init__.
        if ((not getattr(cls, '_is_protocol', False) or
                _is_callable_members_only(cls)) and
                issubclass(instance.__class__, cls)):
            return True
        if cls._is_protocol:
            if all(hasattr(instance, attr) and
                    # All *methods* can be blocked by setting them to None.
                    (not callable(getattr(cls, attr, None)) or
                     getattr(instance, attr) is not None)
                    for attr in _get_protocol_attrs(cls)):
                return True
        return super().__instancecheck__(instance)
```


Regarding "There is no intent to provide sophisticated runtime instance and class checks against protocol classes." (PEP 544):
I fully understand that. But a runtime instance check that simply checks, if a protocol member is there, is not sophisticated. And as you can see in the code above, these checks are already implemented, but unfortunately they don't cover the case reported by me in the initial message.

I could provide a patch for the _ProtocolMeta to cover the case reported by me. It's just a matter of a couple of lines. Even if the runtime isinstance() checking is not required to give the right answer, I think the right answer would be nice - at least for the most basic checks as "Are the protocol members there?"

Regarding "if you inherit from a protocol you are deemed to implement it":
I couldn't find a rule with this meaning in any of the typing PEPs.

But in my point of view, the problem is a different one:
If the instance to check is of a class implemented by another developer (maybe the class is from a third-party library - Bob's library), then such a rule does not help the first developer (Alice). Alice doesn't know anything about such-a-rule-compliance of Bob's classes. She just wants to check if the instance returned by one of Bob's functions complies to the protocol.

-------------

The bottom line is:
I'd like to provide a patch if you want me to.

If you think the current implementation must not be touched, then I would appreciate if the reported case could be documented. I could deliver a draft for this, as well. Currently, the last examples in the sections "Protocol members" and "Explicitly declaring implementation" in PEP 544 contain protocol members with no default implementation in the protocol, but do not suggest the behavior reported above.
msg389019 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-03-18 14:56
To be honest, I just don’t have time to deal,with this, so I’m hoping some
other core dev has an opinion. But I can’t help you find one either. Sorry,
but that’s the reality of open source development. --
--Guido (mobile)
msg389021 - (view) Author: Paul (paul-dest) Date: 2021-03-18 15:30
The authors of PEP 544 are Ivan Levkivskyi, Jukka Lehtosalo, and Łukasz Langa. I think their opinion should count.

I can see "levkivskyi" in the noisy list, but not the other two. And don't see any possibility to add them. Who can add them?

And if added: will they read a notification of an issue in state "closed"?
msg389025 - (view) Author: Ken Jin (kj) * (Python committer) Date: 2021-03-18 16:02
@paul-dest
Anyone can add them using the bug tracker Web interface in the "Nosy List" field above the message box.

Yes they will receive a notification. Adding people sends them an email and subscribes them to this issue (even if closed).

I don't know if Jukka is on this bug tracker (at the very least I can't find them in the search). You may have better luck getting an opinion from others if you send an email to typing-sig instead https://mail.python.org/archives/list/typing-sig@python.org/. That's usually where PEP change/interpretation-related discussion goes (from my own experience).
msg389028 - (view) Author: Paul (paul-dest) Date: 2021-03-18 16:28
@kj
Thank you, Ken! I'll try it on the list as advised by you!
History
Date User Action Args
2022-04-11 14:59:42adminsetgithub: 87678
2021-03-18 16:36:04paul-destsetnosy: + lukasz.langa, Jukka Lehtosalo
2021-03-18 16:28:51paul-destsetmessages: + msg389028
2021-03-18 16:02:44kjsetmessages: + msg389025
2021-03-18 15:30:37paul-destsetmessages: + msg389021
2021-03-18 14:56:34gvanrossumsetmessages: + msg389019
2021-03-18 10:45:03paul-destsetmessages: + msg389008
2021-03-17 14:55:32gvanrossumsetmessages: + msg388926
2021-03-17 06:23:41paul-destsetmessages: + msg388908
2021-03-16 17:06:36gvanrossumsetstatus: open -> closed
resolution: not a bug
messages: + msg388860

stage: resolved
2021-03-16 16:01:31kjsetnosy: + gvanrossum, kj, levkivskyi
messages: + msg388851
2021-03-16 10:02:24paul-destcreate