classification
Title: Fix pluralization in TypeError messages in getargs.c
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ppperry, rhettinger, serhiy.storchaka, terry.reedy, xtreak
Priority: normal Keywords: patch

Created on 2018-07-23 05:54 by xtreak, last changed 2018-12-24 13:18 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8438 merged xtreak, 2018-07-24 12:17
Messages (4)
msg322176 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-07-23 05:54
This ticket is to address comments by @terry.reedy on GitHub for PR https://github.com/python/cpython/pull/8395.

Ref : https://github.com/python/cpython/pull/8395#issuecomment-406894241

Tests: In the 3 tests before this, the suffix 0, 2, or 3 is the number of arguments given. This would suggest 0a, 0b, 2a, and 3a for the suffixes here. I like 1min, 2min, 1max, and 2max even better. You can wait a bit for other reviews before changing.

Blurb:

Let us be more specific. For the patch as is,
Fix pluralization of 'argument' in TypeError message with 'expected at least/most 1 argument', instead of '1 arguments'.

But if we add all the remaining fixes needed in this file (see below), we could write.
Fix pluralization in TypeError messages in getargs.c: '1 argument' instead of '1 arguments' and '1 element' instead of '1 elements'.

Fixes: This patch follows existing expressions formatted into argument%s:
line 380: (nargs < min ? min : max) == 1 ? "" : "s",
lines 1667 and 2093: (len == 1) ? "" : "s", (parens not needed)

Scope: I think we should finish the job in the file. The next line in both else clauses,
"unpacked tuple should have %s%zd elements,"
should get the same treatment. This may break existing tests. (These are the only two formatted 'elements'.

Fixes for the following will break some test, which will then need fixing also.
542: "expected %d arguments, not %" PY_FORMAT_SIZE_T "d",
1720, "%.200s%s takes %s %d positional arguments"
1799, "%.200s%s takes %s %d positional arguments"
2106, "%200s%s takes %s %d positional arguments (%d given)",
2154: "%.200s%s takes %s %d positional arguments"

I checked all PyErr_Format entries and did not see anything other than %d arguments and %d elements.
msg322296 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-24 12:28
See also "expected %d arguments, got %zd" in Objects/typeobject.c.
msg322298 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-07-24 12:58
Thanks, this needs to be fixed only in `check_num_args` and can help in all the other functions like __lt__, __delattr__ etc. in the file that use this.

Before suggestion : 

➜  cpython git:(bpo34193) ✗ ./python -c "a = {}; a.__delattr__()"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: expected 1 arguments, got 0

After suggested change : 

➜  cpython git:(bpo34193) ✗ ./python -c "a = {}; a.__delattr__()"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: expected 1 argument, got 0 


Is there some doc on clarification of the difference between 'required' argument and positional argument errors. I was trying to write to tests for the changes and I couldn't find functions that hit the exact branch of code. Some of them generate required argument error and some of them generate positional argument expected error. I looked up in the argument clinic docs (https://docs.python.org/3/howto/clinic.html) but it's little hard for me to grasp as a beginner. Any pointers will be helpful.


Thanks
msg332424 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-24 13:14
New changeset 6326278e8a3a4b6ac41a74effa63331b1b9fdf5c by Serhiy Storchaka (Xtreak) in branch 'master':
bpo-34193: Fix pluralization in getargs.c and test cases. (GH-8438)
https://github.com/python/cpython/commit/6326278e8a3a4b6ac41a74effa63331b1b9fdf5c
History
Date User Action Args
2018-12-24 13:18:25serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-12-24 13:14:23serhiy.storchakasetmessages: + msg332424
2018-07-24 12:58:46xtreaksetmessages: + msg322298
2018-07-24 12:28:53serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg322296
2018-07-24 12:17:17xtreaksetkeywords: + patch
stage: patch review
pull_requests: + pull_request7963
2018-07-23 14:22:29ppperrysetnosy: + ppperry
2018-07-23 05:54:16xtreakcreate