GeezerVison is showing github.com — Typing: undocumented behaviour change for protocols decorated with `@final` and `@runtime_checkable` in 3.11 · Issue #103171 · python/cpython
Typing: undocumented behaviour change for protocols decorated with @final and @runtime_checkable in 3.11 #103171DescriptionPython 3.11 introduced an undocumented behaviour change for protocols decorated with both @final and @runtime_checkable. On 3.10:
>>> from typing import *
>>> @final
... @runtime_checkable
... class Foo(Protocol):
... def bar(self): ...
...
>>> class Spam:
... def bar(self): ...
...
>>> issubclass(Spam, Foo)
True
>>> isinstance(Spam(), Foo)
True On 3.11:
>>> from typing import *
>>> @final
... @runtime_checkable
... class Foo(Protocol):
... def bar(self): ...
...
>>> class Spam:
... def bar(self): ...
...
>>> issubclass(Spam, Foo)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<frozen abc>", line 123, in __subclasscheck__
File "C:\Users\alexw\coding\cpython\Lib\typing.py", line 1547, in _proto_hook
raise TypeError("Protocols with non-method members"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Protocols with non-method members don't support issubclass()
>>> isinstance(Spam(), Foo)
FalseThis is because, following 0bbf30e (by @JelleZijlstra), the @final decorator sets a __final__ attribute wherever it can, so that it is introspectable by runtime tools. But the runtime-checkable-protocol isinstance() machinery doesn't know anything about the __final__ attribute, so it assumes that the __final__ attribute is just a regular protocol member.
This should be pretty easy to fix: we just need to add __final__ to the set of "special attributes" ignored by runtime-checkable-protocol isinstance()/issubclass() checks here:
|
_TYPING_INTERNALS = frozenset({ |
|
'__parameters__', '__orig_bases__', '__orig_class__', |
|
'_is_protocol', '_is_runtime_protocol' |
|
}) |
@JelleZijlstra do you agree with that course of action?
Linked PRs
Activitychanged the title [-]Undocumented behaviour change for protocols decorated with `@final` and `@runtime_checkable` in 3.11[/-] [+]Typing: undocumented behaviour change for protocols decorated with `@final` and `@runtime_checkable` in 3.11[/+] on Apr 1, 2023JelleZijlstra commented on Apr 1, 2023 My first instinct is to leave this as is. Marking Protocols as final is a weird thing to do in the first place (the whole point of protocols is to subclass from them, though usually implicitly). Adding more names to that _TYPING_INTERNALS list is risky because if people use those names in their protocols, we'll silently ignore them. You could imagine a Protocol that is meant to match only classes decorated with @final. JelleZijlstra commented on Apr 1, 2023 This might be a bigger issue for PEP-702 though: what if you want to @deprecated a runtime-checkable Protocol? That makes more sense than making it final, so I'll amend the PEP to say that __deprecated__ should be ignored by @runtime_checkable. Reacted by Alex Waygood and sunmy2019 AlexWaygood commented on Apr 1, 2023
My first instinct is to leave this as is. Marking Protocols as final is a weird thing to do in the first place (the whole point of protocols is to subclass from them, though usually implicitly). Adding more names to that _TYPING_INTERNALS list is risky because if people use those names in their protocols, we'll silently ignore them. You could imagine a Protocol that is meant to match only classes decorated with @final.
Sure. In that case, I think we should probably document the behaviour change with a .. versionchanged notice in the docs somewhere — sound good? JelleZijlstra commented on Apr 1, 2023 sobolevn commented on Apr 1, 2023 I mark all things as @final by default. Because inside an internal code-base no things should be extended without a good reason.
So, I would consider this as a bug. __final__ is an internal thing that should not be exposed to users and should not affect the runtime of regular operations. AlexWaygood commented on Apr 1, 2023
__final__ is an internal thing that should not be exposed to users and should not affect the runtime of regular operations.
I'm not sure that makes sense. The fact that @final sets the __final__ attribute is documented, and I don't think we ever intended it to be an internal thing. typing.py doesn't need the attribute to be set at all; the only reason why we introduced the behaviour in 3.11 where it sets the attribute wherever possible was so that third-party tools could more easily introspect whether a method or class had been decorated with @final. That's the opposite of it being an internal thing. https://docs.python.org/3/library/typing.html#typing.final
Personally, I wasn't initially sure this new behaviour makes sense for isinstance(), but I can see arguments either way. What really concerns me is the new behaviour with issubclass(), which seems pretty unfortunate to me. AlexWaygood commented on Apr 2, 2023 @ilevkivskyi, do you have any opinions on how @final and @runtime_checkable should interact at runtime? JelleZijlstra commented on Apr 2, 2023 I haven't tried, but won't the change from #103160 fix this issue, because the __final__ attribute is added after we compute the set of attributes? AlexWaygood commented on Apr 2, 2023
I haven't tried, but won't the change from #103160 fix this issue, because the __final__ attribute is added after we compute the set of attributes?
Good point, #103160 does indeed fix this issue!
I wasn't planning on backporting #103160 (if it's even accepted), though, as I was thinking of it as a performance optimisation (and it does change behaviour in a few other subtle ways, as discussed in the PR thread). So if #103160 is merged, that means that 3.12 will have the same behaviour as we had in 3.10 for @final runtime-checkable protocols, but 3.11 will be the "odd one out". ilevkivskyi commented on Apr 3, 2023 FWIW I think protocols should not be final. So the behaviour change is fine. AlexWaygood commented on Apr 3, 2023 Maybe we should just add a note to the docs for @final saying that combining the decorator with @runtime_checkable isn't supported and might have unpredictable consequences. sobolevn commented on Apr 3, 2023
I think protocols should not be final
But, they can be.
What is the reason not to ignore __final__ attribute? 🤔
It surely does not affect @runtime_checkable in any other manner.
I think that having an expected default behaviour in this case is easier than writting a warning in the docs. AlexWaygood commented on Apr 5, 2023
Good point, #103160 does indeed fix this issue!
I wasn't planning on backporting #103160 (if it's even accepted), though, as I was thinking of it as a performance optimisation (and it does change behaviour in a few other subtle ways, as discussed in the PR thread). So if #103160 is merged, that means that 3.12 will have the same behaviour as we had in 3.10 for @final runtime-checkable protocols, but 3.11 will be the "odd one out".
#103160 has now been merged, so it is just 3.11 that has the behaviour change now. JelleZijlstra commented on Jun 7, 2023 I think we should fix 3.11 so that it excludes __final__ when looking at protocol compatibility, as that will make 3.11 behave consistently with both 3.10 and 3.12. added 2 commits that reference this issue on Jun 7, 2023gh-103171: Forward-port new tests for runtime-checkable protocols dec…
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|