homepage

>>> ImportError(spam='spam')
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
TypeError: ImportError does not take keyword arguments
However, it *does* take keyword arguments:
>>> ImportError(name='spam', path='spam')
ImportError()

Here's a patch with a simple test case.

Thanks for the review, Eric. Here's a new patch.

Looks good to me. Thanks for doing this. If no one objects in the meantime, I'll commit this in a few days.

Eric, do you want me to commit the patch? Should this also be committed to the 3.4 branch?

Here's a new patch which uses assertRaisesRegex instead of assertRaises.

The standard error message for this case is:
 xxx() got an unexpected keyword argument 'foo'
I have no idea where that gets generated (a grep didn't teach me anything useful), but I think it would make sense to use that form for the message.
I think the fix can be applied to 3.4.

Thanks for the review, David.
> The standard error message for this case is:
>
> xxx() got an unexpected keyword argument 'foo'
I found two similar messages in the codebase:
* In Modules/itertoolsmodule.c:
 PyErr_SetString(PyExc_TypeError,
 "zip_longest() got an unexpected keyword argument");
* In Python/ceval.c:
 PyErr_Format(PyExc_TypeError,
 "%U() got an unexpected "
 "keyword argument '%S'",
 co->co_name,
 keyword);
But, in ImportError case it can take more than one keyword arguments:
 ImportError(spam="SPAM", eggs=True)
What error message should be printed for the above case?

Just the first unexpected keyword. That's what happens if you pass more than one unexpected keyword to a python function.

Assigning to Berker to apply his own patch since Eric has not gotten around to this.

Why not use PyArg_ParseTupleAndKeywords()?

I am a little uncomfortable with the empty tuple. It's only used as a placeholder for PyArg_ParseTupleAndKeywords. But this way we can achieve consistent error message. Don't know how to choose.

Ping.

Serhiy's patch looks good to me. It would be nice to add a test for multiple invalid keyword arguments:
 with self.assertRaisesRegex(TypeError, msg):
 ImportError('test', invalid='keyword', another=True)
Using empty_tuple seems reasonable to me. Brett and Eric, what do you think?

Left a review, but basically LGTM.

New changeset 9b8f0db1944f by Serhiy Storchaka in branch '3.5':
Issue #21578: Fixed misleading error message when ImportError called with
https://hg.python.org/cpython/rev/9b8f0db1944f
New changeset 95549f4970d0 by Serhiy Storchaka in branch '3.6':
Issue #21578: Fixed misleading error message when ImportError called with
https://hg.python.org/cpython/rev/95549f4970d0
New changeset 59268ac85f4e by Serhiy Storchaka in branch 'default':
Issue #21578: Fixed misleading error message when ImportError called with
https://hg.python.org/cpython/rev/59268ac85f4e 

Added a test for multiple invalid keyword arguments, added braces, fixed a leak.
But there is other oddity in ImportError constructor (issue28289).