This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: wrong error messages when too many kwargs are received
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Oren Milman, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2017-08-17 18:02 by Oren Milman, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue31229_ver1.diff Oren Milman, 2017-08-17 18:04 diff file - ver1
Pull Requests
URL Status Linked Edit
PR 3180 merged Oren Milman, 2017-08-22 12:38
Messages (6)
msg300451 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-08-17 18:02
Some functions produce wrong error messages in case they receive too many
keyword arguments:
- in Objects/exceptions.c - ImportError_init:
    >>> ImportError(1, 2, 3, 4, a=5, b=6, c=7)
    TypeError: ImportError() takes at most 2 arguments (3 given)
- in Python/bltinmodule.c - min_max:
    >>> min(1, 2, 3, 4, a=5, b=6, c=7)
    TypeError: function takes at most 2 arguments (3 given)
    >>> max(1, 2, 3, 4, a=5, b=6, c=7)
    TypeError: function takes at most 2 arguments (3 given)
- in Modules/itertoolsmodule.c - product_new:
    >>> itertools.product(0, a=1, b=2, c=3, d=4, e=5, f=6)
    TypeError: product() takes at most 1 argument (6 given)
- in Python/bltinmodule.c - builtin_print: 
    >>> print(0, a=1, b=2, c=3, d=4, e=5, f=6)
    TypeError: print() takes at most 4 arguments (6 given)

ISTM that changing these error messages to refer to 'keyword arguments' instead
of 'arguments' is a possible solution. (e.g. the last one would become
'print() takes at most 4 keyword arguments (6 given)'

To do that, I changed two 'takes at most' PyErr_Format calls in Python/getargs.c.
I ran the test suite, and it seems that this patch doesn't break anything.
The diff file is attached.
(I didn't open a PR before hearing your opinion, as ISTM that changing code in
getargs.c is a delicate thing.)

what do you think?
msg300454 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-17 18:30
In all of these cases PyArg_ParseTupleAndKeywords() is used in uncommon way. These functions accept variable number of positional-only arguments, and PyArg_ParseTupleAndKeywords() is called with empty args tuple for parsing keyword arguments only. The patch uses a trick for detecting this case and generating more appropriate error message. Correct error message is generated in a normal case too.

LGTM. But it may be worth to add a short comment about the necessary of this special case. And please add tests for error messages in all of these functions. test_call looks appropriate place for them.
msg300617 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-21 09:16
Could you please make a PR from your patch Oren?
msg300619 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-08-21 10:25
I already wrote a patch, but I thought it would be better to wait until
#31236 is resolved.
this is because #31236 would change the error messages of min() and max(), and test_call tests exact error messages in
CFunctionCallsErrorMessages, which is where I thought the tests of this
issue should be added.
msg300761 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-23 18:16
New changeset bf9075a0c55186d2f34df63e6c8512dd6414ff4b by Serhiy Storchaka (Oren Milman) in branch 'master':
bpo-31229: Fixed wrong error messages when too many keyword arguments are received. (#3180)
https://github.com/python/cpython/commit/bf9075a0c55186d2f34df63e6c8512dd6414ff4b
msg300762 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-23 18:19
Thank you Oren for your contribution.
History
Date User Action Args
2022-04-11 14:58:50adminsetgithub: 75412
2017-08-23 18:19:03serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg300762

stage: resolved
2017-08-23 18:16:56serhiy.storchakasetmessages: + msg300761
2017-08-22 12:38:56Oren Milmansetpull_requests: + pull_request3218
2017-08-21 17:19:59serhiy.storchakaunlinkissue31236 dependencies
2017-08-21 10:25:52Oren Milmansetmessages: + msg300619
2017-08-21 09:16:19serhiy.storchakasetmessages: + msg300617
2017-08-19 04:32:41serhiy.storchakalinkissue31236 dependencies
2017-08-17 18:30:22serhiy.storchakasetnosy: + vstinner, serhiy.storchaka
messages: + msg300454
2017-08-17 18:04:11Oren Milmansetfiles: + issue31229_ver1.diff
keywords: + patch
2017-08-17 18:02:16Oren Milmancreate