You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Typing: undocumented behaviour change for protocols decorated with @final and @runtime_checkable in 3.11 #103171
>>> from typing import*
>>> @final
... @runtime_checkable
... classFoo(Protocol):
... defbar(self): ...
...
>>> classSpam:
... defbar(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)
False
This 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:
changed 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, 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.
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.
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?
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.
__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.
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".
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.
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.
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.
Activity
[-]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[/+]JelleZijlstra commentedon 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_INTERNALSlist 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 commentedon Apr 1, 2023
This might be a bigger issue for PEP-702 though: what if you want to
@deprecateda 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.AlexWaygood commentedon Apr 1, 2023
Sure. In that case, I think we should probably document the behaviour change with a
.. versionchangednotice in the docs somewhere — sound good?JelleZijlstra commentedon Apr 1, 2023
Sure, that's fine.
pythongh-103171: Document behaviour change in 3.11 for runtime-checka…
sobolevn commentedon Apr 1, 2023
I mark all things as
@finalby 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 commentedon Apr 1, 2023
I'm not sure that makes sense. The fact that
@finalsets the__final__attribute is documented, and I don't think we ever intended it to be an internal thing.typing.pydoesn'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.finalPersonally, 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 withissubclass(), which seems pretty unfortunate to me.AlexWaygood commentedon Apr 2, 2023
@ilevkivskyi, do you have any opinions on how
@finaland@runtime_checkableshould interact at runtime?JelleZijlstra commentedon 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 commentedon Apr 2, 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
@finalruntime-checkable protocols, but 3.11 will be the "odd one out".ilevkivskyi commentedon Apr 3, 2023
FWIW I think protocols should not be final. So the behaviour change is fine.
AlexWaygood commentedon Apr 3, 2023
Maybe we should just add a note to the docs for
@finalsaying that combining the decorator with@runtime_checkableisn't supported and might have unpredictable consequences.sobolevn commentedon Apr 3, 2023
But, they can be.
What is the reason not to ignore
__final__attribute? 🤔It surely does not affect
@runtime_checkablein any other manner.I think that having an expected default behaviour in this case is easier than writting a warning in the docs.
AlexWaygood commentedon Apr 5, 2023
#103160 has now been merged, so it is just 3.11 that has the behaviour change now.
JelleZijlstra commentedon 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.[3.11] pythongh-103171: Revert undocumented behaviour change for runt…
[3.11] gh-103171: Revert undocumented behaviour change for runtime-ch…
pythongh-103171: Forward-port new tests for runtime-checkable protoco…
pythongh-103171: Forward-port new tests for runtime-checkable protoco…
gh-103171: Forward-port new tests for runtime-checkable protocols dec…
[3.12] gh-103171: Forward-port new tests for runtime-checkable protoc…