We've been having a debate inpython/cpython#102433 about howisinstance() should work at runtime when comparing instances of classes with properties against runtime-checkable protocols. There's a few interrelated issues here, so I'm interested in knowing what the wider community thinks about the best way forward.

Current behaviour

The current behaviour at runtime is that a propertyx on an objectobj will be evaluated if you callisinstance() onobj against a runtime-checkable protocol that specifiesx as part of the interface:

>>>from typingimport Protocol, runtime_checkable>>>@runtime_checkable...classHasX(Protocol):...     x:int...>>>classFoo:...@property...defx(self):...print('Called!')...return42...>>> obj= Foo()>>>isinstance(obj, HasX)Called!True

The reason for this is that callingisinstance() onobj againstHasX performs a structural check comparingobj toHasX. If all attributes and methods specified in theHasX protocol are present onobj, theisinstance() check returnsTrue. This is taken care of by_ProtocolMeta.__instancecheck__, which just callshasattr onobj for every attribute specified in theHasX protocol; callinghasattr(obj, x) will result in thex property onobj being evaluated as part of theisinstance() check.

An alternative to the current behaviour at runtime

An alternative to the current implementation would be to change thehasattr call in_ProtocolMeta.__instancecheck__ to useinspect.getattr_static (or similar):

--- a/Lib/typing.py+++ b/Lib/typing.py@@ -30,6 +30,7 @@ import sys import types import warnings+from inspect import getattr_static from types import WrapperDescriptorType, MethodWrapperType, MethodDescriptorType, GenericAlias@@ -1990,7 +1991,9 @@ def __instancecheck__(cls, instance):                 issubclass(instance.__class__, cls)):             return True         if cls._is_protocol:-            if all(hasattr(instance, attr) and+            sentinel = object()+            if all(+                    (getattr_static(instance, attr, sentinel) is not sentinel) and                     # All *methods* can be blocked by setting them to None.                     (not callable(getattr(cls, attr, None)) or                      getattr(instance, attr) is not None)

This would mean that properties would not be evaluated against instances, sincegetattr_static avoids evaluating descriptors at runtime:

>>>classFoo:...@property...defx(self):...print('Called!')...return42...>>>getattr(Foo(),'x')Called!42>>>import inspect>>> inspect.getattr_static(Foo(),'x')<property object at 0x0000015A789F5120>

Criticisms of the current behaviour at runtime

The current behaviour at runtime has been criticised due to the fact that it can result in unexpected behaviour fromisinstance() checks, which don't usually result in properties being evaluated on instances:

>>>classBar:...@property...defx(self):...raiseRuntimeError("what were you thinking?")...>>>isinstance(Bar(), HasX)Traceback (most recent call last):  File "<stdin>", line 1, in <module>  File "C:\Users\alexw\AppData\Local\Programs\Python\Python311\Lib\typing.py", line 1968, in __instancecheck__    if all(hasattr(instance, attr) and       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File "C:\Users\alexw\AppData\Local\Programs\Python\Python311\Lib\typing.py", line 1968, in <genexpr>    if all(hasattr(instance, attr) and           ^^^^^^^^^^^^^^^^^^^^^^^  File "<stdin>", line 4, in xRuntimeError: what were you thinking?>>>import time>>>classBaz:...@property...defx(self):...         time.sleep(3600)...return42...>>>isinstance(Baz(), HasX)# oh no, now we have to wait for an hour

These criticisms have been levied in the CPython repo more than once (a previous instance of this being reported waspython/cpython#89445), so this is clearly pretty surprising behaviour to some people.

Possible rebuttal to the criticisms of the current behaviour

You could say that the whole point of runtime-checkable protocols is that they do something "a little different" when you callisinstance() against them. Arguing "I didn't expect this because it's not whatisinstance() usually does" may not be the strongest case here.

When it comes to raising exceptions inside property getter methods, it may also be unwise in general to raise exceptions that aren't subclasses ofAttributeError -- doing so will generally lead to unexpected things happening withhasattr() orgetattr() calls.

Defence of the current behaviour at runtime

The current behaviour has things to recommend it. Firstly, usinghasattr() here is probably faster than an alternative implementation usinggetattr_static._ProtocolMeta.__instancecheck__ haspreviously been criticised for on performance grounds. This could make things even worse.

Moreover, while the current implementation leads to surprising outcomes for some users, it's possible that changing the implementation here could also lead to surprising outcomes.getattr_static has different semantics tohasattr: sometimes it finds attributes wherehasattr doesn't, and sometimes it doesn't find attributes whenhasattr does. It's hard to predict exactly what the effect of changing this would be, and it could be a breaking change for some users. The semantics of_ProtocolMeta.__instancecheck__ have been the same for a long time now; it's possible that the better course of action might be to simply document that callingisinstance on an instanceobj against a runtime-checkable protocol might result in properties onobj being evaluated.

Possible rebuttal to the defence of the current behaviour

We probably shouldn't prioritise speed over correctness here; users who care deeply about performance probably shouldn't be using runtime-checkable protocols anyway.

Just because the semantics have been the same for several years doesn't mean we should necessarily stick with them if they're bad; sometimes breaking changes are necessary.

A specific case where changing the implementation would lead to different semantics

One reason why I lean towards keeping the current implementation is because I'd like to keep the invariant where theisinstance guard in the following snippet is enough to ensure that accessing thex attribute is safe:

fromtypingimportProtocol,runtime_checkable@runtime_checkableclassHasX(Protocol):x:intdefdo_something(obj:object)->None:ifisinstance(obj,HasX):print(obj.x)

If we changed the implementation of_ProtocolMeta.__instancecheck__ to usegetattr_static or similar, this wouldn't necessarily be the case. Consider:

classFoo:SWITCH:bool=False@propertydefx(self)->int:ifself.SWITCH:return42raiseAttributeErrorobj:object=Foo()# At runtime this is currently a safe call:# - `isinstance(obj, HasX)` returns True if `self.SWITCH` is True, (in which case accessing `x` is safe)# - `isinstance(obj, HasX)` returns False if `self.SWITCH` is False (in which case accessing `x` is unsafe)## If we changed the implementation to use getattr_static, `isinstance(obj, HasX)` would be True# regardless of the value of `self.SWITCH`,# so the `isinstance()` guard in the `do_something` function would be ineffective# at ensuring that the `x` attribute can be safely accesseddo_something(obj)

To me, being able to useisinstance() guards in this way -- to provide structural narrowingat runtime as well as statically, ensuring attributes can always be accessed safely -- is kind of the whole point of theruntime_checkable decorator.

Others have argued, however, that they would find it more intuitive if the runtimeisinstance() check was closer to how static type checkers understood the above snippet. (Static type checkers have no special understanding of the fact that properties can sometimes raiseAttributeError, so will always consider theFoo class above as being a valid subtype ofHasX, contradicting the current behaviour at runtime.)

Thoughts?