error message for incorrect call degraded in 3.7

[scoder]

BPO	30534
Nosy	@brettcannon, @rhettinger, @scoder, @vstinner, @serhiy-storchaka
PRs	bpo-30534: Fix error messages when pass keyword arguments #1901 bpo-30592: Fixed error messages for some builtins. #1996

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-06-06.15:56:44.818>
created_at = <Date 2017-06-01.06:32:30.571>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'error message for incorrect call degraded in 3.7'
updated_at = <Date 2017-06-08.11:41:21.286>
user = 'https://github.com/scoder'

bugs.python.org fields:

activity = <Date 2017-06-08.11:41:21.286>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-06-06.15:56:44.818>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2017-06-01.06:32:30.571>
creator = 'scoder'
dependencies = []
files = []
hgrepos = []
issue_num = 30534
keywords = []
message_count = 14.0
messages = ['294906', '294912', '294913', '294914', '294915', '294916', '294917', '294920', '295050', '295079', '295096', '295097', '295269', '295414']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'rhettinger', 'scoder', 'vstinner', 'serhiy.storchaka']
pr_nums = ['1901', '1996']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue30534'
versions = ['Python 3.7']

[scoder]

I'm seeing doctest failures in Cython's test suite with Py3.7 due to the change of an error message:

Failed example:
    func1(arg=None)
Expected:
    Traceback (most recent call last):
    ...
    TypeError: func1() takes no keyword arguments
Got:
    Traceback (most recent call last):
      File "/opt/python/3.7-dev/lib/python3.7/doctest.py", line 1329, in __run
        compileflags, 1), test.globs)
      File "<doctest always_allow_keywords_T295.func1[2]>", line 1, in <module>
        func1(arg=None)
    TypeError: func1() takes exactly one argument (0 given)

I'd normally just adapt the doctest and move on, but this seems like the error message is worse than before, so I thought I'd bring it up here. func1() is implemented as a METH_O C function. Suggesting that it does not accept keyword arguments seems better than saying that it expects "exactly one argument" instead of the exactly one argument that was passed.

[vstinner]

Example with a builtin function, abs(), which uses METH_O.

Before:

haypo@selma$ python3
Python 3.5.3 (default, Apr 24 2017, 13:32:13) 
>>> abs(x=5)
TypeError: abs() takes no keyword arguments

After:

haypo@selma$ ./python
Python 3.7.0a0 (heads/master:85aba23, May 31 2017, 10:29:03) 
>>> abs(x=5)
TypeError: abs() takes exactly one argument (0 given)

In Python 3.5, PyCFunction_Call() starts by checking keyword arguments:

    if (flags == (METH_VARARGS | METH_KEYWORDS)) {
        ...
    }
    else {
        if (kwds != NULL && PyDict_Size(kwds) != 0) {
            PyErr_Format(PyExc_TypeError, "%.200s() takes no keyword arguments",
                         f->m_ml->ml_name);
            return NULL;
        }

        switch (flags) {
        case METH_NOARGS:
            size = PyTuple_GET_SIZE(args);
            if (size != 0) {
                PyErr_Format(PyExc_TypeError,
                    "%.200s() takes no arguments (%zd given)",
                    f->m_ml->ml_name, size);
                return NULL;
            }
            ...

In Python 3.7, _PyMethodDef_RawFastCallKeywords() first check positional arguments:

    switch (flags)
    {
    case METH_NOARGS:
        if (nargs != 0) {
            PyErr_Format(PyExc_TypeError,
                "%.200s() takes no arguments (%zd given)",
                method->ml_name, nargs);
            goto exit;
        }

        if (nkwargs) {
            goto no_keyword_error;
        }

    ...

We can easily exchange the two checks, but IMHO we need an unit test (at least decorated by @cpython_only) to avoid regressions in the future.

[vstinner]

Ah, Python 3.6 is not affected:

haypo@selma$ ./python
Python 3.6.1+ (heads/unpack_dict:ba4fb35, May 31 2017, 16:31:51) 
>>> abs(x=5)
TypeError: abs() takes no keyword arguments

[vstinner]

I introduced the regression with the commit 7fc252a:

"Optimize _PyCFunction_FastCallKeywords(): Issue bpo-29259: Write fast path in _PyCFunction_FastCallKeywords() for METH_FASTCALL, (...) Cleanup also _PyCFunction_FastCallDict(): (...) Move code to raise the "no keyword argument" exception into a new no_keyword_error label."

[vstinner]

See also "[Python-ideas] Positional-only parameters" discussion of last March:
https://mail.python.org/pipermail/python-ideas/2017-March/044956.html

Brett Cannon: "It seems all the core devs who have commented on this are in the positive (Victor, Yury, Ethan, Yury, Guido, Terry, and Steven; MAL didn't explicitly vote). So to me that suggests there's enough support to warrant writing a PEP. (...)"

(No, I didn't write such PEP yet :-p)

Maybe we can also enhance the error message to explain that the function accepts exactly one positional-only argument?

[serhiy-storchaka]

Note that some error messages are "takes no keyword arguments", but others are "does not take keyword arguments" or "doesn't take keyword arguments". They should be unified. What wording is better?

[vstinner]

Note that some error messages are "takes no keyword arguments", but others are "does not take keyword arguments" or "doesn't take keyword arguments".

We already have a _PyArg_NoKeywords() function. Maybe we could add a new _PyErr_NoKeywords() function which would just raise the error?

[serhiy-storchaka]

The details can be different. For example:

>>> 1 .__add__(x=2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: wrapper __add__ doesn't take keyword arguments

The message contains the word "wrapper" and doesn't contains parenthesis. I think it can be extended for containing the type name ("int.__add__()" or "wrapper __add__ of in objects"), and this hard to do with general _PyErr_NoKeywords(). Let first unify messages, then try to add more useful information in messages and create a general API if this is possible.

[brettcannon]

Note that some error messages are "takes no keyword arguments", but others are "does not take keyword arguments" or "doesn't take keyword arguments". They should be unified. What wording is better?

I vote for "takes no keyword arguments".

[serhiy-storchaka]

Updated error messages. Later I'm going to unify error messages for Python and C functions. But this is a different issue.

I can wrote a test, but I don't know good place for it. test_builtin doesn't look a good place to me.

[brettcannon]

The tests could either go into the respective type's tests or a new test module like test_exception_messages could be added that's only to help keep messages in sync with each other.

[serhiy-storchaka]

There is no respective type. abs() is just an arbitrary example. There are perhaps tens functions affected by this bug. test_exception_messages looks an overkill. There are similar tests in test_capi and test_getargs2 that test public PyArg_* functions. But this bug is related to other code.

Hmm, may be test_call is an appropriate test.

[serhiy-storchaka]

New changeset 5eb788b by Serhiy Storchaka in branch 'master':
bpo-30534: Fixed error messages when pass keyword arguments (bpo-1901)
5eb788b

[serhiy-storchaka]

New changeset 6cca5c8 by Serhiy Storchaka in branch 'master':
bpo-30592: Fixed error messages for some builtins. (bpo-1996)
6cca5c8