classification
Title: Strings passed to KeyError do not round trip
Type: behavior Stage: patch review
Components: Interpreter Core Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: lukasz.langa Nosy List: Julian, amaury.forgeotdarc, belopolsky, eric.araujo, lukasz.langa, martin.panter, pitrou, rharris, rhettinger, terry.reedy, vencabot_teppoo
Priority: normal Keywords: patch

Created on 2008-04-17 17:24 by rharris, last changed 2017-10-23 23:37 by terry.reedy.

Files
File name Uploaded Description Edit
testExceptionStringRoundTrip.py rharris, 2008-04-17 17:24 A test demonstrating the issue and showing that KeyError is arbitrarily different
KeyError.patch amaury.forgeotdarc, 2008-04-21 00:14 review
issue2651.diff lukasz.langa, 2010-08-04 10:17 Patch for py3k and stdlib review
Messages (27)
msg65586 - (view) Author: Rick Harris (rharris) Date: 2008-04-17 17:24
Here is a bug in Python 2.5 which would be nice to fix for Py3k (since
we are already breaking compatibility):

Take a string:
s = "Hello"

Create a KeyError exception with that string:
e = KeyError(s)

Counterintuitively, casting the exception to a string doesn't return the
same string:
str(e) != s

Instead, when KeyError is cast to a string it affixes single-quotes
around the string.

I have create a test which shows that the other built-in exceptions
(except for 3 Unicode Errors which seem to be unusual in that they don't
accept just a string), do indeed round-trip the string unaltered.

This actually caused a bug (in an old version of zope.DocumentTemplate).

I am including the test case I wrote for now; I will begin looking into
a solution shortly and hopefully whip up a patch.
msg65587 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-04-17 17:42
Here is a relevant comment inside the KeyError_str function:
    /* If args is a tuple of exactly one item, apply repr to args[0].
       This is done so that e.g. the exception raised by {}[''] prints
         KeyError: ''
       rather than the confusing
         KeyError
       alone.  The downside is that if KeyError is raised with an
explanatory
       string, that string will be displayed in quotes.  Too bad.
       If args is anything else, use the default BaseException__str__().
    */

Why is it so important to round trip?
msg65588 - (view) Author: Rick Harris (rharris) Date: 2008-04-17 19:48
I think it is important to round-trip for at least two reasons:

1) Consistency. Other built-in exceptions behave this way, why should
KeyError be any different? Okay, technically 3 UnicodeErrors don't allow
just strings to be passed in (perhaps they should :-); but for common
exception classes, they all behave the same way.

To quote PEP-20: "Special cases aren't special enough to break the rules"

2) Intuitiveness. Decorating the string with quotes is unexpected; it
has already caused at least one bug and could cause more.

Ensuring intuitive round-trip behavior is an important enough issue that
is explicitly discussed in PEP 327 for the decimal type.

Why can't intuitiveness be restored to KeyError in Py3K?
msg65609 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-04-18 09:36
Here is another form of the same inconsistency:

>>> [].pop(0)
IndexError: pop from empty list
>>> {}.pop(0)
KeyError: 'pop(): dictionary is empty'

And my preferred one:
>>> unicodedata.lookup('"')
KeyError: 'undefined character name \'"\''

KeyError is special in that dict lookup raises the equivalent of
KeyError(key). Since the key may be any kind of (hashable) object, it's
preferable to repr() it.

I can see 3 solutions to the problem:

1- imitate IndexError for lists: the exception do not contain the key.

2- dict lookup builds the complete string message, and raise it
   raise KeyError("key not found: %r" % key)
then KeyError.__str__ can be removed.

3- like IOError, KeyError has "msg" and "key" attributes. then dict
lookup raises
   raise KeyError("key not found", key)
and KeyError.__str__ is something like:
   if self.key is not None:
       return "%s: %r" % (self.msg, self.key)
   else
       return str(self.msg)

Choice 1 is not an improvement.
Choice 2 has the correct behavior but leads to performance problems;
KeyErrors are very very common in the interpreter (namespace lookups...)
and formatting the message is costly.
Choice 3 may cause regression for code that use exception.args[0], but
otherwise seems the best to me. I'll try to come with a patch.
msg65656 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-04-21 00:14
Attached patch changes KeyError: when it has two arguments, they are
formatted with "%s: %r". Otherwise the base repr is called, and this
allows the round trip.

Standard objects (dict, set, UserDict, namedtuple, defaultdict, weak
dictionaries) now raise something like KeyError("not in dict", key). 

At least one place in the stdlib relied on the key being the first
argument to KeyError() (in ConfigParser.py)

I don't know if this incompatibility will break much code. At least we
can say that the current behavior is not documented.
msg66432 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-05-08 18:51
Wouldn't it be nice to also store the offending key as a "key" attribute?
Writing key_error.key is a lot intuitive than key_error.args[0] (or is
it key_error.args[1] ? I've already forgotten :-)).
msg107330 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-08 17:35
KeyError.patch is out of date.  Is anyone motivated enough to update it for py3k?  I like the idea, but don't have spare cycles at the moment.
msg111418 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-07-24 02:24
Alexander, Brett, I could update the patch but first I need thumbs up that this is going to be accepted and some eventual code breaks will be patched (again, I can do that but it has to be accepted on time).

Brett, what to do?
msg111530 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-25 09:15
@Łukasz: please provide an updated patch.
msg112696 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-08-03 21:44
Patch for py3k ready. Includes patches for the stdlib where tests actually failed.
msg112722 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-08-03 23:30
Looks like you didn’t narrow your diff command, since unrelated changes to configparser appear there. svn diff file1 path/file2 :)
msg112766 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-08-04 06:16
Corrected patch attached. You're right, I left in ReST doc changes for configparser. Sorry for that.
msg112779 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-04 09:44
In KeyError_str, I think the following code shouldn't be deleted:

-    if (PyTuple_GET_SIZE(self->args) == 1) {
-        return PyObject_Repr(PyTuple_GET_ITEM(self->args, 0));
msg112781 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-08-04 10:17
Patch updated to include a roundtrip test in test_exceptions.
msg112782 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-08-04 10:19
FTR regarding for Antoine's comment above: that code should in fact be removed :)
msg112794 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-04 12:14
Latest patch looks good. Note that you could use PyUnicode_FromFormat() instead of building the format string manually.
msg112994 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-08-05 16:20
Unassigning because others seem to be ahead of me reviewing this patch.   I will keep an eye on this, though.

Please note that I marked issue #614557 to depend on this one.  Adding 'key' attribute to exceptions is the subject of that issue.

While I support these features, I would really like to see a more comprehensive redesign as a part of PEP 3151.  I think it would be a mistake to rush something in before PEP 3151 is implemented and then discover that it was not the best choice as a part of bigger picture.
msg113331 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2010-08-08 21:44
For the record, I am -1 for this change after discussion on #python-dev. There are three major arguments against the proposed approach:

1. Moratorium. If we don't stick to the rules set by ourselves, nobody will take us seriously. No exceptions, this has to wait.

2. Backwards incompatibility. The current implementation is backwards incompatible and doesn't provide a reliable way of returning `e.key`. There was an alternative discussed about including key= and message= keyword arguments to the exception. This would require writing an additional helper routine in C for dealing with keyword arguments and is thus a bit of a broader problem.

3. PEP 3151. While still in draft form, this PEP prepares a solution for more generic and consistent behaviour of exceptions. It mentions IOError and OSError at the moment but Alexander is right: if the PEP 3151 approach chooses a new form of argument handling for exceptions, lookup errors should try to conform with that.

For the above reasons, I would resolve this issue as 'after moratorium' and prepare a superseder that gathers all PEP 3151 related issues in the tracker.
msg154043 - (view) Author: David (vencabot_teppoo) Date: 2012-02-23 03:14
I'm +1 for fixing this behavior for the same reasons that are mentioned in the OP: consistency and predictability. I raised this issue as #14086, and I was referred to this issue before closing mine as a duplicate.

It took me a while to figure out why I was getting unexpected escaped quotation marks in my strings, and it turned out that it was because I was passing strings back and forth as Exception arguments (tagging built-in Exceptions with a little bit of extra information when they occurred and re-raising), and every time that it occurred with a KeyError (and only with a KeyError), the string would grow another pair of quotation marks.

In my issue, I bring up the documentation in the Python Tutorial about Exception.args and Exception.__str__(); it states very plainly and simply (as it should be) that the __str__() method is there to be able to conveniently print Exception arguments without calling .args, and, when an unhandled Exception stops Python, the tail-end of the message (the details) of the exception will be the arguments that it was given. This is not the case with KeyError.

str(KeyError("Foo")) should be equal to "Foo", as it would be with any other Exception and as is the documented behavior of built-in Exceptions, at least in the tutorial (which I realize isn't the be-all, end-all document). The documented behavior makes more sense.
msg240518 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-04-12 01:17
The moratorium is over as far as I understand, and PEP 3151 (OSError changes) has already been implemented.

My understanding is that exception messages are not generally part of the API. I think that avoiding this surprising quirk is more important than retaining backwards compatibility with the formatted exception message. But if it cannot be changed, we can at least document the quirk.
msg242293 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2015-04-30 23:20
Agreed that this can be addressed now for Python 3.5.
msg274844 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-09-07 17:26
The current behavior seems to be a recurring source of confusion and bugs, so something needs to change.  I'm thinking that the least egregious thing to do is to remove (in 3.6) the special case code for KeyError.  The small downside is that KeyError('')  wouldn't look as good.  The benefit is that we eliminate an unexpected special case and make KeyErrors behave like all the other exceptions including LookupError. 

I'm against the two-args solution as being too disruptive.  Since args[0] is currently the only reliable way of extracting the key, prepending a message field will likely break much existing code that really wants to access the key.
msg274847 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2016-09-07 17:31
What are you suggesting? Something like:

  KeyError: key 'foobar' not found in dict

? If so, +1 from me.
msg274860 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2016-09-07 18:29
No, the suggestion is to only adopt the first part of the patch from 2010, which is to revert KeyError to behave like LookupError again:

  >>> print(LookupError('key'))
  key
  >>> print(KeyError('key'), 'now')
  'key' now
  >>> print(KeyError('key'), 'in 3.6')
  key in 3.6

In other words, there is no descriptive message while stringifying KeyError. Having an API with two arguments was disruptive because it moved the key from e.args[0] to e.args[1].

Raymond, would it be acceptable to create a two-argument form where the *second* argument is the message? That way we could make descriptive error messages for dicts, sets, etc. possible. In this world:

  >>> print(KeyError('key', 'key {!r} not found in dict'), 'in 3.6')
  key 'key' not found in dict in 3.6

Do you think any code depends on `str(e) == str(e.args[0])`?
msg274864 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2016-09-07 18:40
> No, the suggestion is to only adopt the first part of the patch from 2010, which is to revert KeyError to behave like LookupError again

That ship has sailed long ago. 2.7, 3.4 and 3.5 (the three major Python versions currently in use) all have the same behaviour, and nobody seems to complain very loudly:

>>> {}['foo']
Traceback (most recent call last):
  File "<ipython-input-2-3761c7dc3711>", line 1, in <module>
    {}['foo']
KeyError: 'foo'

>>> KeyError('foo')
KeyError('foo')
>>> print(KeyError('foo'))
'foo'
msg275553 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2016-09-10 03:12
So actually the issue long predates Python 2.5...

https://hg.python.org/cpython/rev/0401a0ead1eb

Now I'm not so sure it's worth touching it anymore ;)
msg304851 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-10-23 23:37
A new Stackoverflow question gives a better illustration of how special-casing KeyError can be a nuisance.
https://stackoverflow.com/questions/46892261/new-line-on-error-message-in-idle-python-3-3/46899120#46899120
From a current repository build instead of 3.3:

>>> s = 'line\nbreak'
>>> raise Exception(s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
Exception: line
break
>>> raise KeyError(s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'line\nbreak'
>

The OP wanted to get the line break to break without fudging the code to catch Exception rather than KeyError.  I proposed catching KeyError and then 'print(err.args[0]' instead of 'print(err)'.

Why this makes a difference, and why KeyError is unique in needing this, is obvious after I found this issue and read the code comment quoted by Amaury.  But it sure wasn't before.

The rational for applying repr only applies when there is exactly one arg of value ''.  So I think the fix should be to only apply it when args *is* ('',).  There is no reason to quote a non-blank message -- and suppress any formatting a user supplies.
History
Date User Action Args
2017-10-23 23:37:04terry.reedysetnosy: + terry.reedy
messages: + msg304851
2016-09-10 03:12:19lukasz.langasetmessages: + msg275553
2016-09-10 00:14:27lukasz.langalinkissue27268 dependencies
2016-09-07 18:40:53pitrousetmessages: + msg274864
2016-09-07 18:29:49lukasz.langasetmessages: + msg274860
2016-09-07 17:31:04pitrousetmessages: + msg274847
2016-09-07 17:26:16rhettingersetmessages: + msg274844
2015-09-05 04:37:49rhettingersetnosy: + rhettinger
2015-09-04 17:40:25lukasz.langasetversions: + Python 3.6, - Python 3.5
2015-04-30 23:20:29lukasz.langasetassignee: lukasz.langa
messages: + msg242293
versions: + Python 3.5, - Python 3.2, Python 3.3
2015-04-12 01:17:38martin.pantersetnosy: + martin.panter
messages: + msg240518
2012-02-23 03:14:15vencabot_teppoosetnosy: + vencabot_teppoo
messages: + msg154043
2012-02-23 03:00:06eric.araujolinkissue14086 superseder
2011-12-02 16:58:11ezio.melottisetnosy: - BreamoreBoy

versions: + Python 3.3
2011-08-19 01:51:11Juliansetnosy: + Julian
2011-08-19 01:36:40meador.ingesetstage: needs patch -> patch review
2010-08-08 21:44:45lukasz.langasetmessages: + msg113331
2010-08-05 16:40:13brett.cannonsetnosy: - brett.cannon
2010-08-05 16:20:30belopolskysetassignee: belopolsky -> (no value)
messages: + msg112994
2010-08-04 12:14:56pitrousetmessages: + msg112794
2010-08-04 10:19:09lukasz.langasetmessages: + msg112782
2010-08-04 10:18:02lukasz.langasetfiles: + issue2651.diff

messages: + msg112781
2010-08-04 10:17:31lukasz.langasetfiles: - issue2651.diff
2010-08-04 09:44:11pitrousetmessages: + msg112779
2010-08-04 06:16:52lukasz.langasetfiles: + issue2651.diff

messages: + msg112766
2010-08-04 06:13:38lukasz.langasetfiles: - issue2651.diff
2010-08-03 23:30:40eric.araujosetnosy: + eric.araujo
messages: + msg112722
2010-08-03 21:44:35lukasz.langasetfiles: + issue2651.diff

messages: + msg112696
2010-07-25 09:15:10BreamoreBoysetnosy: + BreamoreBoy

messages: + msg111530
stage: needs patch
2010-07-24 02:24:24lukasz.langasetnosy: + brett.cannon, lukasz.langa
messages: + msg111418
2010-06-08 20:16:51belopolskylinkissue614557 dependencies
2010-06-08 17:35:05belopolskysetmessages: + msg107330
2010-05-28 05:16:04belopolskysetversions: + Python 3.2, - Python 2.5
2010-05-28 05:14:49belopolskysetassignee: belopolsky

nosy: + belopolsky
2008-05-08 18:51:50pitrousetnosy: + pitrou
messages: + msg66432
2008-04-21 00:15:01amaury.forgeotdarcsetfiles: + KeyError.patch
keywords: + patch
messages: + msg65656
2008-04-18 09:36:16amaury.forgeotdarcsetmessages: + msg65609
2008-04-17 19:48:04rharrissetmessages: + msg65588
2008-04-17 17:42:09amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg65587
2008-04-17 17:24:40rharriscreate