| msg92281 - (view) |
Author: Alan Isaac (aisaac) |
Date: 2009-09-05 16:24 |
| In Python 2.6 if I subclass Exception and intialize my instances with a `message` attribute, I get a DeprecationError via BaseException. Of course there is no problem in Py3, because adding a `message` attribute to instances of a subclass is in fact **not** a problem and will **not** be disallowed. I.e., the DeprecationError is raised incorrectly in Python 2.6. I am not sure how to raise a DeprecationError *only* in the correct cases, but certainly the current behavior can be much improved. As a crude example, if the `args` passed to BaseException has length 0, then a flag could be set not to raise this DeprecationError. Even if not perfect, this would be **much** better than the current behavior, since it is common practice not to pass the subclass's initialization arguments on to Exception.__init__. This behavior can be expected to affect entire libraries and therefore should be fixed. (Eg., it affects docutils.) I.e., "change the name of your variable" is the wrong answer to this bug report. |
|
|
| msg92295 - (view) |
Author: Jean-Paul Calderone (exarkun) *  |
Date: 2009-09-05 20:15 |
| This also affects Twisted. We worked around it a couple months ago by putting a read-only `message` property onto our Exception subclass (Here's the revision: <http://twistedmatrix.com/trac/changeset/27062>). This seemed reasonable enough, but it turns out that it's incompatible with PyPy so we'll probably have to do something else instead. |
|
|
| msg92296 - (view) |
Author: ivank (ivank) |
Date: 2009-09-05 21:03 |
| That trac link should be http://twistedmatrix.com/trac/changeset/27062 |
|
|
| msg92362 - (view) |
Author: Duncan Grisby (dgrisby) |
Date: 2009-09-07 11:57 |
| This affects my application too. We have a large body of code that uses exception classes automatically generated from CORBA IDL, with attributes named "message". It is infeasible for us to change to use a different attribute name. We've ended up with dirty hacks all over the place to suppress the warnings. |
|
|
| msg92384 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-07 19:11 |
| If someone can come up w/ a patch to make this work for all of you I would be happy to review it and backport to 2.6. But the deprecation warning cannot go away as it's needed for anyone who came to rely on the feature in their 2.5 code from Python. |
|
|
| msg92386 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2009-09-07 19:35 |
| After looking at this more carefully, I find myself wondering what exactly is being deprecated at all. Brett said: > it's needed for anyone who came to rely on the feature in their 2.5 code from Python. Can someone help me understand what the feature is? |
|
|
| msg92387 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-07 19:44 |
| On Mon, Sep 7, 2009 at 12:35, Jean-Paul Calderone<report@bugs.python.org> wrote: > > Jean-Paul Calderone <exarkun@divmod.com> added the comment: > > After looking at this more carefully, I find myself wondering what > exactly is being deprecated at all. The message attribute as introduced in Python 2.5 based on http://www.python.org/dev/peps/pep-0348/ . > > Brett said: >> it's needed for anyone who came to rely on the feature in their 2.5 > code from Python. > > Can someone help me understand what the feature is? > > ---------- > > _______________________________________ > Python tracker <report@bugs.python.org> > <http://bugs.python.org/issue6844> > _______________________________________ > |
|
|
| msg92388 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2009-09-07 20:01 |
| Hm. That PEP is marked as rejected, though. I guess it was partially implemented, those changes included in the Python 2.5 release, and then it was decided that it was a bad idea, rejected, and the changes undone for 3.x (what about 2.7)? Or did something else happen? I did a simple search on the PEP for the word "message", but the word doesn't appear anywhere in the text! So I guess it's a feature that's implied by the PEP, but I would appreciate some help figuring out the implication. Here's what I do see: - If one argument is passed to BaseException.__init__, then the value is used as the value for the message attribute. - If zero or more than one arguments are passed, then the attribute is given the empty string for a value. - If zero or one arguments are passed, the message attribute is used as the string representation of the exception instance. The deprecation warning emitted in 2.6 doesn't tell me which of these things is deprecated, though. Is there something more to the message attribute, or are one or more of the above list things which should no longer be relied on? |
|
|
| msg92390 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-07 20:32 |
| On Mon, Sep 7, 2009 at 13:01, Jean-Paul Calderone<report@bugs.python.org> wrote: > > Jean-Paul Calderone <exarkun@divmod.com> added the comment: > > Hm. That PEP is marked as rejected, though. I guess it was partially > implemented, those changes included in the Python 2.5 release, and then > it was decided that it was a bad idea, rejected, and the changes undone > for 3.x (what about 2.7)? Or did something else happen? > I screwed up and pasted in the URL for the wrong PEP. Sorry about that. Written a few too many PEPs involving exceptions. The correct one is http://www.python.org/dev/peps/pep-0352/ . > I did a simple search on the PEP for the word "message", but the word > doesn't appear anywhere in the text! So I guess it's a feature that's > implied by the PEP, but I would appreciate some help figuring out the > implication. > > Here's what I do see: > > - If one argument is passed to BaseException.__init__, then the value > is used as the value for the message attribute. > - If zero or more than one arguments are passed, then the attribute is > given the empty string for a value. > - If zero or one arguments are passed, the message attribute is used > as the string representation of the exception instance. > > The deprecation warning emitted in 2.6 doesn't tell me which of these > things is deprecated, though. Is there something more to the message > attribute, or are one or more of the above list things which should no > longer be relied on? The 'message' attribute itself is deprecated as it didn't exist prior to being introduced in 2.5. The original idea was to not let BaseException take multiple arguments to its constructor and have message be what replaced 'args'. But when I tried to do that at PyCon 2007 the pain was too great so it was decided a more long-term strategy to making BaseException accept a single argument was needed. So the 'message' attribute that got added to BaseException was retracted. |
|
|
| msg92392 - (view) |
Author: Alan Isaac (aisaac) |
Date: 2009-09-07 21:53 |
| > The 'message' attribute itself is deprecated > as it didn't exist prior to being introduced in 2.5. That seems to me to be the wrong way to phrase it, and indeed that kind of phrasing implies the current bug. For example, it leads to the incorrect statement that "The 'message' attribute ... didn't exist prior to being introduced in 2.5." But looking at the docutils and Twisted code bases, to take two examples, tells us that it **did** exist: a slew of instances had this attribute. The correct statement, that BaseException did not initialize a `message` attribute, is an entirely different matter. Imo, deprecating setting and accessing an **instance attribute** is just flat out wrong and grossly violates inheritability promises. As we have seen. I think (?) that what was desired to be deprecated is the combination of - setting a message attribute via BaseException.__init__, AND - accessing an instances message attribute that was set *this way* But in fact the setting cannot really be deprecated because it is implicit: it is something currently done by BaseException, not by the user. So I think (?) the best we can do is look at whether the user initializes BaseException (**not** the derived classes) with a single argument, which is a nonempty string, and then tries to access this as a `message` attribute. Which is why I originally proposed setting a flag when BaseException.__init__ is called and conditioning the deprecation warning on this flag. (E.g., the flag could be set if and only if the user calls BaseException.__init__(instance, string).) In any case, I think Jean-Paul is asking exactly the right question (i.e., just what exactly is being deprecated?). A more careful answer will hopefully lead to less buggy DeprecationWarning. |
|
|
| msg92393 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2009-09-07 21:57 |
| Alright. So in Python 3.1, this is the behavior: >>> BaseException().message (attribute error) >>> BaseException("foo").message (attribute error) >>> BaseException("foo", "bar").message (attribute error) >>> x = BaseException() >>> x.message = "foo" >>> x.message 'foo' >>> x = BaseException("foo") >>> x.message = "bar" >>> x.message 'bar' >>> x = BaseException("foo") >>> x.message = "bar" >>> x.message 'bar' So I propose the following as the new behavior for 2.x: >>> BaseException().message (deprecation warning) '' >>> BaseException("foo").message (deprecation warning) 'foo' >>> BaseException("foo", "bar").message (deprecation warning) '' >>> x = BaseException() >>> x.message = "foo" >>> x.message 'foo' >>> x = BaseException("foo") >>> x.message = "bar" >>> x.message 'bar' >>> x = BaseException("foo", "bar") >>> x.message = "baz" >>> x.message 'baz' Summarized: emit a warning when the same code in Python 3.1 would raise an exception; let all other cases pass. There is one other case that I would think about changing, but I don't see how it can, given the behavior that is implemented in 3.1 already. BaseException("a message") is a Python 2.5-supported way of creating an exception with a value for its message attribute. This no longer works in Python 3.1. So, arguably, this is another case where a deprecation warning should be emitted. However, this would be pretty obnoxious, since BaseException("a message") in Python 2.4 (by way of Exception("a message"), of course, since Python 2.4 did not have BaseException) was perfectly valid. It seems like BaseException(string) should have been deprecated and BaseException(tuple) been made the preferred API. That's for another time, though. How does the above proposed deprecation behavior sound? |
|
|
| msg92394 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-07 22:07 |
| On Mon, Sep 7, 2009 at 14:57, Jean-Paul Calderone<report@bugs.python.org> wrote: > > Jean-Paul Calderone <exarkun@divmod.com> added the comment: > > Alright. So in Python 3.1, this is the behavior: > >>>> BaseException().message > (attribute error) >>>> BaseException("foo").message > (attribute error) >>>> BaseException("foo", "bar").message > (attribute error) >>>> x = BaseException() >>>> x.message = "foo" >>>> x.message > 'foo' >>>> x = BaseException("foo") >>>> x.message = "bar" >>>> x.message > 'bar' >>>> x = BaseException("foo") >>>> x.message = "bar" >>>> x.message > 'bar' > > So I propose the following as the new behavior for 2.x: > >>>> BaseException().message > (deprecation warning) > '' >>>> BaseException("foo").message > (deprecation warning) > 'foo' >>>> BaseException("foo", "bar").message > (deprecation warning) > '' >>>> x = BaseException() >>>> x.message = "foo" >>>> x.message > 'foo' >>>> x = BaseException("foo") >>>> x.message = "bar" >>>> x.message > 'bar' >>>> x = BaseException("foo", "bar") >>>> x.message = "baz" >>>> x.message > 'baz' > > Summarized: emit a warning when the same code in Python 3.1 would raise > an exception; let all other cases pass. > > There is one other case that I would think about changing, but I don't > see how it can, given the behavior that is implemented in 3.1 already. > BaseException("a message") is a Python 2.5-supported way of creating an > exception with a value for its message attribute. This no longer works > in Python 3.1. So, arguably, this is another case where a deprecation > warning should be emitted. However, this would be pretty obnoxious, > since BaseException("a message") in Python 2.4 (by way of Exception("a > message"), of course, since Python 2.4 did not have BaseException) was > perfectly valid. It seems like BaseException(string) should have been > deprecated and BaseException(tuple) been made the preferred API. That's > for another time, though. > > How does the above proposed deprecation behavior sound? Works for me. If someone can create a patch for that I will review it and apply it to 2.6 (the warning will be removed from 2.7 and so will the attribute). |
|
|
| msg92439 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2009-09-09 01:57 |
| Can I add a field to the PyBaseExceptionObject struct? |
|
|
| msg92442 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-09 02:18 |
| I had a feeling you were going to ask that. =) I think it's fine, and from what I can tell from PEP 384 it's okay as long as it is in no way publicly exposed. But I have added Martin to the nosy list to make sure I am not messing up the ABI somehow in a micro release. |
|
|
| msg92454 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-09 16:22 |
| Making this a release blocker to see what Barry thinks of this. |
|
|
| msg92556 - (view) |
Author: Alan Isaac (aisaac) |
Date: 2009-09-12 23:30 |
| Since you are working on this, can you see if http://bugs.python.org/issue6108 is related or in any case can be fixed at the same time? Thanks. |
|
|
| msg92562 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2009-09-13 05:08 |
| FWIW, I'm waiting to hear about the acceptability of adding fields to the exception structure(s) before I work on this patch. |
|
|
| msg92679 - (view) |
Author: Barry A. Warsaw (barry) * |
Date: 2009-09-16 12:24 |
| I agree that this is a release blocker for 2.6.3 |
|
|
| msg92687 - (view) |
Author: Jean-Paul Calderone (exarkun) * |
Date: 2009-09-16 14:05 |
| I'm not sure I'll be able to work on this again for a while after this morning, so here's a patch. I don't really understand how the exception structs are involved here, so I don't really know why the patch works, but it seems to. If there's something wrong with it, someone else may need to fix it in order to have it ready for 2.6.3. |
|
|
| msg92708 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2009-09-16 19:20 |
| The patch leads to crashes with all the exceptions that have their own structs; since they are derived from BaseException they must start with the same binary layout as PyBaseExceptionObject (pointers to any exception will be cast to PyBaseExceptionObject). This also shows one way in which this change can mess up third-party extensions (though I don't know if anybody does that): if some extension created new exception classes doing it the way we do, e.g. for SyntaxError. However, I have a different suggestion: we could put a user-set "message" in the __dict__. The getter would then check for that first and only warn if it falls back to self->message. See attached patch. |
|
|
| msg92709 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2009-09-16 19:28 |
| Huh. I just made some tests to find out if exceptions with a message set survive pickling in 2.6.2 and unpickling in patched trunk. I found that the message attribute isn't pickled at all in 2.6.2, so there should be no cross-version compatibility problems with pickles. |
|
|
| msg92710 - (view) |
Author: Alan Isaac (aisaac) |
Date: 2009-09-16 19:38 |
| I hope it is not too annoying to link these ... I asked thhis of Jean-Paul but now I'll ask it of George. Since you are working on this, can you see if http://bugs.python.org/issue6108 is related or in any case can be fixed at the same time? Thanks. |
|
|
| msg92712 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2009-09-16 19:41 |
| Yes, it should be fixed, but it is not related. I'm setting it as a release blocker as well. |
|
|
| msg92714 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2009-09-16 20:01 |
| Just looked at the patch and it looks good to me. I say go ahead and commit, Georg. |
|
|
| msg92718 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2009-09-16 20:35 |
| OK, I added another test for pickling, committed in r74845, and backported to 2.6 in r74848. |
|
|