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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Call isinstance
instead of issubclass
during exception handling
#69723
Comments
Currently Python2.7 calls However, this does not necessarily give one really enough granularity when dealing with base libraries which are not as granular in raising exceptions as one would like (the Python base library before PEP-3151 comes to mind). Currently I used the trick of calling This method is great for sort-of emulating PEP-3151 in Python2.7, and similar work can be done for other libraries: making the exception classes appear more granular based on properties of the instance of the exception. My belief is that by calling the I think this is important because exception-handling is already a place where messy code is likely to occur, and we need all the support we can get in making it just a tad less messy.[**] Note: Python3.5 calls [*] See https://github.com/sjoerdjob/exhacktion/ for how I use the described hack to somewhat emulate PEP-3151 in Python2.7. |
This does not sound like the kind of change we would accept for python 2.7. (I also don't understand what the difference is between calling issubclass on the type and isinstance on the instance in this case. But, then, I haven't looked at the code in question.) |
[Avoiding UTF-8 error] This is an interesting idea that never occurred to me. It would be nice to support it if the performance impact isn’t a problem. By the sound of bpo-12029, performance is barely affected until you actually try to catch a “virtual exception”. So the impact of changing from subclass to instance checking shouldn’t be much worse if we do it right. I agree this would be a new feature for a new release, not for 2.7 or 3.5. BTW here’s another approach using the same sys.exc_info() hack that also works in Python 3: >>> def catch_not_implemented():
... '''Helper to catch "API not implemented" exceptions'''
... [_, exc, _] = sys.exc_info()
... if isinstance(exc, NotImplementedError):
... return BaseException # Catch the exception
... if isinstance(exc, EnvironmentError) and exc.errno == ENOSYS:
... return BaseException # Catch the exception
... if isinstance(exc, HTTPError) and exc.code == HTTPStatus.NOT_IMPLEMENTED:
... return BaseException # Catch the exception
... return () # Don't catch the exception
...
>>> try: raise OSError(ENOSYS, "Dummy message")
... except catch_not_implemented() as err: print(repr(err))
...
OSError(38, 'Dummy message') |
Ah, now I understand. That is definitely not something that could go into 2.7. It is also a level of change that would need to go to python-ideas first, and possibly even a PEP. Closing this pending such a discussion. |
FWIW I don’t see it as a drastic change. If the current patch for bpo-12029 went ahead, I imagine the change for instance checking could look like @@ PyErr_GivenExceptionMatches()
- /* err might be an instance, so check its class. */
- if (PyExceptionInstance_Check(err))
- err = PyExceptionInstance_Class(err);
@@ given_exception_matches_inner()
- res = PyObject_IsSubclass(err, exc);
+ if (PyExceptionInstance_Check(err)) {
+ res = PyObject_IsInstance(err, exc);
+ } else {
+ res = PyObject_IsSubclass(err, exc);
+ } |
The code change may be minor, but the semantic change is not. It needs discussion. |
Let me rephrase that. The semantic change *may* not be. That's what needs discussion :) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: