| msg290885 - (view) |
Author: Michael Seifert (MSeifert) * |
Date: 2017-03-31 00:03 |
| Some exceptions thrown by `PyArg_ParseTupleAndKeywords` refer to "function" or "this function" even when a function name was specified. For example: >>> import bisect >>> bisect.bisect_right([1,2,3,4], 2, low=10) TypeError: 'low' is an invalid keyword argument for this function Wouldn't it be better to replace the "this function" part (if given) with the actual function name? |
|
|
| msg290894 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2017-03-31 05:45 |
| LGTM in general. I was going to make similar changes. While we a here, maybe add the function name to other TypeError messages specific to the function? "Required argument '%U' (pos %d) not found" and "Argument given by name ('%U') and position (%d)", but not to "keywords must be strings". Classified this as an enhancement. |
|
|
| msg290896 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-31 06:10 |
| And while we are here, please change "keywords must be strings" to "keyword arguments must be strings" as in PyArg_ValidateKeywordArguments(). And maybe make all TypeError messages starting from a lower letter for uniformity. |
|
|
| msg290902 - (view) |
Author: Michael Seifert (MSeifert) * |
Date: 2017-03-31 09:24 |
| Thank you for the suggestions, I added them to the PR. If you want But are you sure about the "keywords must be strings" -> "keyword arguments must be strings" change? I always thought the key is the "keyword" and the value the "(keyword) argument". |
|
|
| msg290903 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-31 09:27 |
| Martin, could you please take a look? Does the wording of error messages look good English? Do you have to suggest some enhancements? |
|
|
| msg290904 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-31 09:36 |
| > But are you sure about the "keywords must be strings" -> "keyword arguments must be strings" change? I always thought the key is the "keyword" and the value the "(keyword) argument". Now, after you have made change, I have doubts. Yes, it looks ambiguous. Maybe "keyword names"? Martin, David, what are your thoughts? |
|
|
| msg290907 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2017-03-31 12:02 |
| In this test, “keyword arguments” is definitely wrong: >>> f(**{1:2}) -TypeError: f() keywords must be strings +TypeError: f() keyword arguments must be strings To me, a keyword argument is a _value_ passed in using the f(name=. . .) syntax, and the keyword is the name that identifies it (usually becomes a parameter name in the function implementation). So I think the original was better. But if you think the original can be confusing, “keyword names” would also work. The other messages look okay to me. |
|
|
| msg290910 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-31 12:22 |
| I agree. PyArg_ValidateKeywordArguments() fooled me. Now I see that it would be better to change only PyArg_ValidateKeywordArguments(). |
|
|
| msg290931 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2017-03-31 17:29 |
| If you want to be completely unambiguous, you could say "keyword argument names". "keyword argument" appears to mean different things in different contexts; sometimes it means the name and the value together, sometimes one or the other. Usually which one it is is clear from context, but it seems in this case it is not clear. |
|
|
| msg291356 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-04-09 07:47 |
| New changeset 64c8f705c0121a4b45ca2c3bc7b47b282e9efcd8 by Serhiy Storchaka (Michael Seifert) in branch 'master': bpo-29951: Include function name for some error messages in `PyArg_ParseTuple*` (#916) https://github.com/python/cpython/commit/64c8f705c0121a4b45ca2c3bc7b47b282e9efcd8 |
|
|
| msg291357 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-04-09 07:49 |
| Thank you for your contribution Michael. Since most builtin and extension functions and methods now use Argument Clinic and provide a function name, this should be a great enhancement. |
|
|
| msg295553 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-06-09 16:27 |
| New changeset f9f1ccace395a8f65b60dc12567a237b4002fd18 by Victor Stinner (Serhiy Storchaka) in branch 'master': Fix regression in error message introduced in bpo-29951. (#2028) https://github.com/python/cpython/commit/f9f1ccace395a8f65b60dc12567a237b4002fd18 |
|
|