gh-104555: Runtime-checkable protocols: Don't let previous calls to isinstance() influence whether issubclass() raises an exception#104559
Conversation
…ng._ProtocolMeta.__instancecheck__
|
On |
Sorry, something went wrong.
sunmy2019
left a comment
There was a problem hiding this comment.
LGTM
But I still think __subclasshook__ depending on the caller is the weirdest part (of #104555 (comment)). Though it cannot be changed or is hard to change.
Sorry, something went wrong.
Co-authored-by: Carl Meyer <carl@oddbird.net>
…d/cpython into fix-runtime-protocols
|
Oh, we will still hit the problematic Consider this from typing import runtime_checkable, Generic, Protocol, TypeVar
T = TypeVar("T")
@runtime_checkable
class Spam(Protocol[T]):
x: T
class Eggs(Generic[T]):
def __init__(self, x: T) -> None:
self._x = x
def __getattr__(self, attr: str) -> T:
if attr == "x":
return self._x
raise AttributeError(attr)
print(isinstance(Eggs(42), Spam))
print(issubclass(Eggs, Spam))Running on 3.10, 3.11 -> TypeError |
Sorry, something went wrong.
|
I think one possible solution is that "do not let |
Sorry, something went wrong.
|
@sunmy2019, I think we were thinking the same thought simultaneously :) How does it look to you after 5f72b82? (It's basically a completely different approach now.) |
Sorry, something went wrong.
ABCMeta.__instancecheck__ cache in typing._ProtocolMeta.__instancecheck__|
This new approach also has the advantage that it seems like it might provide a speedup relative to |
Sorry, something went wrong.
|
This new approach looks better. Great Job! |
Sorry, something went wrong.
carljm
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
Co-authored-by: Carl Meyer <carl@oddbird.net>
|
Thanks @JelleZijlstra for helping debug this, @sunmy2019 for spotting the flaws in my original approach, and everybody for reviewing! |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot AMD64 Windows11 Bigmem 3.x has failed when building commit b27fe67. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/1079/builds/1325 Failed tests:
Failed subtests:
Summary of the results of the build (if available): == Tests result: FAILURE then FAILURE == 428 tests OK. 10 slowest tests:
1 test failed: 36 tests skipped: 1 re-run test: Total duration: 47 min 52 sec Click to see traceback logsTraceback (most recent call last):
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\support\__init__.py", line 979, in wrapper
return f(self, maxsize)
^^^^^^^^^^^^^^^^
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 540, in test_case_md5_uintmax
self.check('md5', b'A'*size, '28138d306ff1b8281f1a9067e1a1a2b3')
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 394, in check
self.check_file_digest(name, data, hexdigest)
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 407, in check_file_digest
f.write(data)
OSError: [Errno 28] No space left on device
Traceback (most recent call last):
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\support\__init__.py", line 979, in wrapper
return f(self, maxsize)
^^^^^^^^^^^^^^^^
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 535, in test_case_md5_huge
self.check('md5', b'A'*size, 'c9af2dff37468ce5dfee8f2cfc0a9c6d')
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 394, in check
self.check_file_digest(name, data, hexdigest)
File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 407, in check_file_digest
f.write(data)
OSError: [Errno 28] No space left on device
|
Sorry, something went wrong.
…s to `isinstance()` influence whether `issubclass()` raises an exception (python#104559) Co-authored-by: Carl Meyer <carl@oddbird.net>
ABCMeta.__instancecheck__cachesisinstance()calls against classes that haveABCMetaas their metaclass. It uses these cache entries not only to inform how futureisinstance()calls behave, but also howissubclass()calls behave. That means that onmainwe now have some rather unfortunate behaviour when it comes to runtime-checkable protocols, due to the fact thattyping._ProtocolMetais a subclass ofABCMeta, andtyping._ProtocolMeta.__instancecheck__callssuper().__instancecheck__()too soon (following 47753ec):This PR fixes the incorrect behaviour. It means that these
isinstance()checks will be about twice as slow as they are onmain:But they'll still be much faster than they are on 3.11. Use of
ABCMeta.registerwith protocols is pretty rare anyway, as far as I know, sinceABCMeta.registerisn't supported by type checkers. Other kinds ofisinstance()checks do not suffer a significant performance regression.Fixes #104555.
(Skipping news, as this bug doesn't exist on any released version of Python.)
main#104555