Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-105144: abc: Suppress errors raised by unrelated other subclasses#105159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…assesA more invasive solution forpython#105144; perhaps better kept for 3.13 only.
| ifissubclass(subclass,rcls): | ||
| cls._abc_cache.add(subclass) | ||
| returnTrue | ||
| exceptException: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In the C code I actually suppress all exceptions, not just Exception. Not sure what the right thing to do is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'd say suppressing all exceptions would definitely be bad -- that would mean that we'd be suppressing e.g.KeyboardInterrupt, andSystemExit.
Honestly, I feel pretty uncomfortable about suppressing all subclasses ofException -- could we just doTypeError? If I had a typo in a custom__subclasscheck__ method, I'm not sure I'd want e.g. the resultingAttributeError to be silenced because of the fact thatABCMeta was being used somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You'd still see the error if you use your broken class directly. What this PR changes is that you'll no longer see the error if you use issubclass() checks on a separate class that inherits from the same ABC.
Looks like there's stilllots of diff --git a/Lib/typing.py b/Lib/typing.pyindex 8c874797d2..8caeb915d8 100644--- a/Lib/typing.py+++ b/Lib/typing.py@@ -1733,7 +1733,7 @@ def _allow_reckless_class_checks(depth=3): The abc and functools modules indiscriminately call isinstance() and issubclass() on the whole MRO of a user class, which may contain protocols. """- return _caller(depth) in {'abc', 'functools', None}+ return _caller(depth) in {'functools', None} |
The trouble is that these Lines 105 to 106 inec0082c
But we don't want it to raise if the |
AlexWaygood commentedJun 1, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ultimately, it does sort of feel like |
I feel like this change isn't worth the churn. |
Uh oh!
There was an error while loading.Please reload this page.
A more invasive solution for#105144; perhaps better kept for 3.13 only.
typing.Protocoland unrelatedisinstance()checks #105144