classification
Title: show class name in method invocation TypeError
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Dennis Sweeney, chris.jerdonek, vstinner, xtreak
Priority: normal Keywords: patch

Created on 2020-05-19 07:57 by chris.jerdonek, last changed 2020-06-04 13:20 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20236 merged Dennis Sweeney, 2020-05-19 19:36
PR 20606 closed vstinner, 2020-06-03 12:27
PR 20615 merged vstinner, 2020-06-03 15:47
Messages (20)
msg369324 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-19 07:57
When calling an instance method incorrectly, you will often get a TypeError that is some variation of the following:

    Traceback (most recent call last):
      File "/.../test.py", line 6, in <module>
        a.foo(1)
    TypeError: foo() takes 1 positional argument but 2 were given

However, when multiple classes have method foo() and the type of "a" isn't immediately obvious, this often isn't enough to know what method was being called. Thus, it would be more helpful if the error message includes also the class that foo() belongs to, or alternatively the type of the object. (These can be different when subclasses are involved.)

For comparison, if you call a method that doesn't exist, you will get a message that looks like the following:

    Traceback (most recent call last):
      File "/.../test.py", line 6, in <module>
        a.bar(1)
    AttributeError: 'A' object has no attribute 'bar'

So taking from this as an example, the message in the first case could be something like--

    TypeError: foo() for 'A' object takes 1 positional argument but 2 were given
msg369382 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-19 19:46
The attached PR isn't exactly what you requested, but it's a very minimal code change that uses the existing __qualname__ functionality to change the message to

    TypeError: A.foo() takes 1 positional argument but 2 were given

Does that address those problems?
msg369405 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-19 22:59
Thanks! I think it does.

Also, I see now that using the __qualname__ is better than including the object's type for locating the method because you can have cases like super().foo() or even--

class A:
    def foo(self):
        pass

def bar():
    pass

A.foo = bar
msg369421 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-20 07:09
While trying to write tests, I stumbled across something interesting: _PyObject_FunctionString as discussed here ( https://bugs.python.org/issue37645 ) returns a string that also includes the module name where applicable.  For example, the module name is included behind the qualname in the following situation:

    >>> from random import Random
    >>> Random.seed(a=1, **{"a":2})
    Traceback (most recent call last):
      File "<pyshell#7>", line 1, in <module>
        Random.seed(a=1, **{"a":2})
    TypeError: random.Random.seed() got multiple values for keyword argument 'a'

or:

    >>> class A:
    ...    def foo(bar):
    ...         pass
    ...
    >>> A().foo(bar=1, **{"bar":2})
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: __main__.A.foo() got multiple values for keyword argument 'bar'

From what I can tell, this seems to happen mostly during the CALL_FUNCTION_EX instruction (unpacking *args and **kwargs), while we don't get this level of qualification during the actual do_call_core. Maybe _PyObject_FunctionString should ideally be used everywhere applicable, but there seems to be a different issue with that: the _PyEval_EvalCode function where the error handling occurs doesn't receive a reference to the function, only references to the function's code object and qualname (unless I'm missing to something).

Should the signature of _PyEval_EvalCode be modified? Or should we be satisfied with the half-measure of including the qualname but not the module (at least for now)? Is there a good reason for the different qualifiers for functions when encountering different types of invalid arguments?

There's also a block that I couldn't figure out how to reach in tests from Python code: at https://github.com/python/cpython/blob/master/Python/ceval.c#L4233 ("keywords must be strings" when passing as two arrays), I don't know how to reach this code in a way that isn't a SyntaxError first.
msg369422 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-20 07:25
> Or should we be satisfied with the half-measure of including the qualname but not the module (at least for now)?

This is something I was wondering myself, too (also for other contexts). Let's take things one step at a time and limit ourselves just to __qualname__ in this issue. Including the module name can be discussed in a separate issue. (This question also comes up for the __repr__ of objects -- sometimes it includes the fully qualified name and sometimes it doesn't.)

For your last question, does this work?

>>> def foo(**kwargs): pass
... 
>>> foo(**{1: 2})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keywords must be strings

(Also, the corrected link is here:
https://github.com/python/cpython/blob/master/Python/ceval.c#L4182 )
msg369423 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-20 07:34
Oh, I see now I was hitting a different line:
https://github.com/python/cpython/blob/master/Objects/call.c#L1009

Maybe my suggestion is enough to help you (I didn't really look closely at the code), or maybe you were already aware of that.
msg369424 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-20 07:35
I got this:

>>> class A:
...  def f():
...   pass
...
>>> A.f(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: A.f() takes 0 positional arguments but 1 was given
>>> A.f(**{1:2})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keywords must be strings

I think this is coming from https://github.com/python/cpython/blob/cd8295ff758891f21084a6a5ad3403d35dda38f7/Python/getargs.c#L1636.
msg369425 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-20 07:42
Never mind; I think you're right, and https://github.com/python/cpython/blob/master/Objects/call.c#L1009 is the line.
msg369426 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-20 07:42
Oh, that string is used in even more spots (sorry wasn't looking too closely the first time).
msg369477 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-20 21:29
I just ran the entire test suite with:

--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -4179,6 +4179,7 @@ _PyEval_EvalCode(PyThreadState *tstate,
         Py_ssize_t j;

         if (keyword == NULL || !PyUnicode_Check(keyword)) {
+            printf("THIS CODE WAS RUN!\n");
             _PyErr_Format(tstate, PyExc_TypeError,
                           "%U() keywords must be strings",
                           qualname);

and the line was never printed. So maybe the test coverage (or removal?) should be a separate issue.
msg369492 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-21 05:15
> So maybe the test coverage (or removal?) should be a separate issue.

That sounds good. Want to file the issue?
msg369496 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-21 05:45
Sure -- I'll file the issue.
msg369499 - (view) Author: Dennis Sweeney (Dennis Sweeney) * Date: 2020-05-21 06:10
https://bugs.python.org/issue40706
msg369645 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-22 20:40
New changeset b5cc2089cc354469f12eabc7ba54280e85fdd6dc by Dennis Sweeney in branch 'master':
bpo-40679: Use the function's qualname in certain TypeErrors (GH-20236)
https://github.com/python/cpython/commit/b5cc2089cc354469f12eabc7ba54280e85fdd6dc
msg369646 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-22 20:43
Thanks again, Dennis!
msg369653 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-22 21:27
> _PyObject_FunctionString as discussed here ( https://bugs.python.org/issue37645 ) returns a string that also includes the module name where applicable.

By the way, Dennis, regarding the above, one thing I noticed is that Python doesn't currently expose a convenient way to get the fully qualified name of a class (the "full name" as opposed to the qualified name). It might be worth exploring what that would involve. I think it would be useful, personally.
msg370658 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-03 12:20
> bpo-40679: Use the function's qualname in certain TypeErrors (GH-20236)

This change introduced a regression. See this bug in Cython:
https://github.com/cython/cython/issues/3641#issuecomment-638102096

I wrote a PR to use co->co_name if qualname is NULL (ex: when PyEval_EvalCodeEx() is called). I will post the PR once I finish to run the test suite locally ;-)
msg370664 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-06-03 12:44
Thanks a lot, Victor.
msg370706 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-04 13:19
New changeset 232dda6cbc10860328a83517a6e3ea238ff4147f by Victor Stinner in branch 'master':
bpo-40679: Fix _PyEval_EvalCode() crash if qualname is NULL (GH-20615)
https://github.com/python/cpython/commit/232dda6cbc10860328a83517a6e3ea238ff4147f
msg370707 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-06-04 13:20
> Thanks a lot, Victor.

You're welcome. Using the qualified name instead of the "short" name in error messages is nice enhancement!
History
Date User Action Args
2020-06-04 13:20:23vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg370707

stage: patch review -> resolved
2020-06-04 13:19:08vstinnersetmessages: + msg370706
2020-06-03 15:47:39vstinnersetpull_requests: + pull_request19841
2020-06-03 12:44:06chris.jerdoneksetmessages: + msg370664
2020-06-03 12:27:55vstinnersetstage: resolved -> patch review
pull_requests: + pull_request19834
2020-06-03 12:20:50vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg370658

resolution: fixed -> (no value)
2020-05-22 21:27:28chris.jerdoneksetmessages: + msg369653
2020-05-22 20:43:27chris.jerdoneksetstatus: open -> closed
resolution: fixed
messages: + msg369646

stage: patch review -> resolved
2020-05-22 20:40:25chris.jerdoneksetmessages: + msg369645
2020-05-21 12:22:17chris.jerdoneksetcomponents: + Interpreter Core
versions: + Python 3.10, - Python 3.9
2020-05-21 06:10:32Dennis Sweeneysetmessages: + msg369499
2020-05-21 05:45:42Dennis Sweeneysetmessages: + msg369496
2020-05-21 05:15:31chris.jerdoneksetmessages: + msg369492
2020-05-20 21:29:25Dennis Sweeneysetmessages: + msg369477
2020-05-20 07:42:55chris.jerdoneksetmessages: + msg369426
2020-05-20 07:42:49Dennis Sweeneysetmessages: + msg369425
2020-05-20 07:35:48Dennis Sweeneysetmessages: + msg369424
2020-05-20 07:34:02chris.jerdoneksetmessages: + msg369423
2020-05-20 07:25:59chris.jerdoneksetmessages: + msg369422
2020-05-20 07:09:01Dennis Sweeneysetmessages: + msg369421
2020-05-19 22:59:51chris.jerdoneksetmessages: + msg369405
2020-05-19 19:46:47Dennis Sweeneysetmessages: + msg369382
2020-05-19 19:36:17Dennis Sweeneysetkeywords: + patch
nosy: + Dennis Sweeney

pull_requests: + pull_request19523
stage: patch review
2020-05-19 17:26:03xtreaksetnosy: + xtreak
2020-05-19 07:57:29chris.jerdonekcreate