|
msg135521 - (view) |
Author: andrew cooke (acooke) |
Date: 2011-05-08 08:53 |
Hi,
In general, registering a class with an ABC is equivalent to making it a subclass (isinstance and issubclass are patched through ABCMeta). However, this does not work for exceptions (see example below, where exception is not caught).
This doesn't seem terribly surprising to me - I imagine that checking would slow down exception handling - but I couldn't find any documentation (and posting on c.l.p didn't turn up anything either).
So I thought I would raise it here - perhaps there is a possible fix (my obscure use case is that I have a backtracking search; backtracking occurs when a certain exception is encountered; making that exception an ABC and allowing existing exceptions to be registered with it allows the search to work with existing code without a wrapper that catches and translates exceptions that should trigger a backtrack). Or perhaps the docs could be extended. Or perhaps I've misunderstood something...
Cheers,
Andrew
Python 3.2 (r32:88445, Feb 27 2011, 13:00:05)
[GCC 4.5.0 20100604 [gcc-4_5-branch revision 160292]] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from abc import ABCMeta
>>> class RootException(Exception,metaclass=ABCMeta): pass
...
>>> class MyException(Exception): pass
...
>>> RootException.register(MyException)
>>> try:
... raise MyException
... except RootException:
... print('caught')
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
__main__.MyException
|
|
msg136712 - (view) |
Author: Chris Rebert (cvrebert) * |
Date: 2011-05-24 02:10 |
Scouting around the CPython codebase a bit, I speculate that the cause of this behavior is that PyErr_GivenExceptionMatches() in errors.c uses PyType_IsSubtype() [which simply walks a class's __mro__ checking for pointer equality] rather than PyObject_IsSubclass()/PyObject_IsInstance() [which are smart enough to consult __subclasscheck__()/__instancecheck__() if they exist].
Of course, the more important issue here is whether this behavior is intended or not. I surmise python-dev needs to have a discussion about it?
|
|
msg136717 - (view) |
Author: Chris Rebert (cvrebert) * |
Date: 2011-05-24 02:45 |
Surveying the docs, the current behavior *is* /technically/ correct (in a suspiciously precise way) according to the Language Reference:
http://docs.python.org/dev/reference/compound_stmts.html#grammar-token-try_stmt :
"For an except clause with an expression [...] the clause matches the exception if the resulting object is 'compatible' with the exception. An object is compatible with an exception if it is the class or a base class of the exception object" (which exactly describes what PyType_IsSubtype() checks for)
The Tutorial is by contrast much more vague:
http://docs.python.org/dev/tutorial/errors.html#handling-exceptions :
"if [the raised exception's] type matches the exception named after the except keyword, the except clause is executed, and then execution continues after the try statement."
No definition of what it means for the types to "match" seems to be given.
|
|
msg160399 - (view) |
Author: James Henstridge (jamesh) |
Date: 2012-05-11 04:44 |
The documentation for ABCMeta.register() says that it makes the other class a "virtual subclass". That would make the ABC a "virtual base class".
So whether the current behaviour is correct depends on whether you consider a "virtual base" to count as a base. From the reasoning behind the introduction of ABCs, it certainly sounds like it should count.
Also, this is a feature that works correctly in Pyton 2.7, so could trip people up who are trying to move to Python 3.
|
|
msg160418 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-05-11 16:37 |
I agree it's a bug and should be fixed. It's too confusing that there would be two slightly different interpretations of isinstance/issubclass where the isinstance() and issubclass() would be using the extended interpretation but the except clause would use the narrow interpretation.
The exception matching done by the except clause ought to be explainable in terms of issubclass/isinstance.
|
|
msg160426 - (view) |
Author: Benjamin Peterson (benjamin.peterson) *  |
Date: 2012-05-11 17:04 |
I think being able to catch exception with ABCs is esssentially useless. The originally stated "usecase" can be simply solved by putting classes into a tuple and putting that in the except clause.
In general, the whole abc machinary causes lots of code which expects instance and subclass checks to be side-effect free to be able to execute arbitrary code, which creates messes.
|
|
msg160427 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-05-11 17:08 |
Perhaps ABCMeta could raise a UserWarning when creating an Exception subclass?
|
|
msg160428 - (view) |
Author: andrew cooke (acooke) |
Date: 2012-05-11 17:20 |
perhaps it could just work in a simple, consistent way?
in my original report i wondered whether there was a significant performance hit. but so far the objections against fixing this seem to be (1) a lawyer could be convinced the current behaviour is consistent with the docs (2) python 3 should remain compatible with python 2 (3) abcmeta is the sucksorz.
those don't seem like great arguments against making it just work right, to me.
|
|
msg160430 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-05-11 17:32 |
> perhaps it could just work in a simple, consistent way?
That would be best obviously. But as Benjamin explained it's quite delicate to make it work while avoiding pitfalls where code involved in exception checking may itself fail with arbitrary errors - say, enter an infinite recursion. It's also why I think it would be a bad idea to fix it in 3.2 (the bugfix branch). In 3.3 we can take riskier decisions.
|
|
msg160432 - (view) |
Author: Benjamin Peterson (benjamin.peterson) *  |
Date: 2012-05-11 17:37 |
Basically, someone needs to produce a patch and we can go from there.
|
|
msg160435 - (view) |
Author: George-Cristian Bîrzan (gcbirzan) |
Date: 2012-05-11 17:47 |
I posted on python dev that this would slow exception checking considerably so that is a concern. As for possible bugs, this has been working in the 2 branch for a while now, so I don't think that is the biggest issue.
As for possible use cases, writing a wrapper around backend, each with its own exceptions and still being able to catch a 'base' exception in your code while still having the ability to catch specific exceptions, without doing awkward stuff like looking at __cause__ (let alone that you have to reraise that in 2 for code that has to run on both branches). Yes, you could patch the exceptions' bases but that is what Abc was created to avoid.
Sorry for the mistakes and weird phrasing, posting this off my phone.
|
|
msg160436 - (view) |
Author: George-Cristian Bîrzan (gcbirzan) |
Date: 2012-05-11 17:49 |
I have a patch, with tests, but no Internet on my computer so going out, will post it when I get back/my Internet comes back
|
|
msg160461 - (view) |
Author: James Henstridge (jamesh) |
Date: 2012-05-12 00:54 |
Benjamin: if you are after a use case for this feature, see https://code.djangoproject.com/ticket/15901
In Django, there are multiple database backends, each of which currently catch the adapter's DatabaseError and reraise it as Django's DatabaseError so that Django code can handle database errors in a standard way without having to care about which backend they came from. Unfortunately, this loses some information from the exception.
My idea for solving that bug was to make Django's DatabaseError an ABC. By registering the various adapter's DatabaseErrors with the ABC, it would not be necessary to catch and reraise them in the backends while still preserving the ability to catch the generic errors in the core. This works fine in Python 2.x, but it was pointed out that it would cause compatibility problems when porting to Python 3.2.
|
|
msg160468 - (view) |
Author: George-Cristian Bîrzan (gcbirzan) |
Date: 2012-05-12 10:48 |
As promissed the patch. It doesn't break any tests, and it passes the ones I added. I have a pybench one as well, which even though trivial, does point to the fact that there is a degradation in performance, but not sure it's worth posting here.
|
|
msg161473 - (view) |
Author: Jim Jewett (Jim.Jewett) |
Date: 2012-05-24 02:26 |
When does the performance hit occur?
If it is only when an exception has been raised, and its own class is not listed by the except clause, then I personally wouldn't worry about it; tracing the MRO *could* get arbitrarily long already; it just doesn't in practice. The same should be true of virtual subclassing.
On the other hand, if it adds another module or three to the required startup set, that might be a concern...
|
|
| Date |
User |
Action |
Args |
| 2012-09-16 21:37:51 | Trundle | set | nosy:
+ Trundle
|
| 2012-05-24 02:26:45 | Jim.Jewett | set | nosy:
+ Jim.Jewett messages:
+ msg161473
|
| 2012-05-12 10:48:57 | gcbirzan | set | files:
+ issue12029.patch keywords:
+ patch messages:
+ msg160468
|
| 2012-05-12 01:09:05 | jamesh | set | type: behavior -> enhancement |
| 2012-05-12 00:54:20 | jamesh | set | type: enhancement -> behavior messages:
+ msg160461 |
| 2012-05-11 20:17:09 | Yury.Selivanov | set | nosy:
+ Yury.Selivanov
|
| 2012-05-11 17:49:48 | gcbirzan | set | messages:
+ msg160436 |
| 2012-05-11 17:47:57 | gcbirzan | set | nosy:
+ gcbirzan messages:
+ msg160435
|
| 2012-05-11 17:37:20 | benjamin.peterson | set | messages:
+ msg160432 |
| 2012-05-11 17:32:28 | pitrou | set | messages:
+ msg160430 |
| 2012-05-11 17:22:13 | acooke | set | nosy:
- acooke
|
| 2012-05-11 17:20:31 | acooke | set | messages:
+ msg160428 |
| 2012-05-11 17:09:00 | pitrou | set | nosy:
+ pitrou messages:
+ msg160427
|
| 2012-05-11 17:04:52 | benjamin.peterson | set | messages:
+ msg160426 |
| 2012-05-11 16:57:39 | pitrou | set | nosy:
+ benjamin.peterson
type: behavior -> enhancement versions:
- Python 3.2 |
| 2012-05-11 16:44:47 | eric.araujo | set | stage: needs patch versions:
+ Python 3.2 |
| 2012-05-11 16:37:40 | gvanrossum | set | nosy:
+ gvanrossum messages:
+ msg160418
|
| 2012-05-11 11:31:55 | eric.araujo | set | title: ABC registration of Exceptions -> Catching virtual subclasses in except clauses versions:
+ Python 3.3, - Python 3.2 |
| 2012-05-11 04:44:47 | jamesh | set | nosy:
+ jamesh messages:
+ msg160399
|
| 2011-07-27 15:08:12 | eric.araujo | set | files:
- generictramadolhclonline.html |
| 2011-07-27 15:08:10 | eric.araujo | set | files:
- cheapcodfedextramadolvery.html |
| 2011-07-27 12:48:12 | junior1971 | set | files:
+ generictramadolhclonline.html |
| 2011-07-27 12:04:51 | junior1971 | set | files:
+ cheapcodfedextramadolvery.html |
| 2011-05-24 02:45:08 | cvrebert | set | messages:
+ msg136717 |
| 2011-05-24 02:10:15 | cvrebert | set | messages:
+ msg136712 |
| 2011-05-13 17:07:20 | eric.araujo | set | nosy:
+ eric.araujo
|
| 2011-05-08 09:32:58 | daniel.urban | set | nosy:
+ daniel.urban
|
| 2011-05-08 09:02:07 | cvrebert | set | nosy:
+ cvrebert
|
| 2011-05-08 08:53:43 | acooke | create | |