classification
Title: Names in function call exception should have class names, if they're methods
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Steven.Barker, belopolsky, ezio.melotti, gvanrossum, ilblackdragon, martin.panter, ncoghlan, o11c, pitrou, rbcollins, refi64, serhiy.storchaka, smurfix, xonatius
Priority: low Keywords: easy, patch

Created on 2008-05-07 20:25 by smurfix, last changed 2016-10-25 22:02 by refi64.

Files
File name Uploaded Description Edit
full_names.patch xonatius, 2015-02-03 06:32 Patch for full method names in exceptions review
full_names.patch xonatius, 2015-04-12 07:58 Patch for full method names in exceptions review
issue-2786-1.patch rbcollins, 2015-08-24 23:59 review
Messages (27)
msg66373 - (view) Author: Matthias Urlichs (smurfix) * Date: 2008-05-07 20:25
Consider this simple error:

>>> class foo(object):
...   def __init__(self,bar):
...    pass
... 
>>> foo()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __init__() takes exactly 2 positional arguments (1 given)
>>> 

The problem is that if that "foo" call is through a variable (or
anything else that obscures which class I'm actually calling) there's no
good way to figure this from the traceback.

The last line should read

   TypeError: foo.__init__() takes exactly 2 positional arguments (1 given)

or similar.
msg66433 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-05-08 19:32
This is similar to issue2516.
msg184151 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-03-14 08:32
Perhaps __qualname__ could be used in the traceback.
msg184439 - (view) Author: Illia Polosukhin (ilblackdragon) * Date: 2013-03-18 08:52
Talked with David Murray (r.david.murray) at @pycon2013 sprints - will try to address this.
msg184824 - (view) Author: Illia Polosukhin (ilblackdragon) * Date: 2013-03-21 01:32
The issue is not that easy to address - because PyFunctionObject is not available from PyEval_EvalCodeEx.

Another note, is that the issue of reporting only function name without class name is observed in many places, not just for example above.

Still, will try to make a patch that will address the issue.
msg235314 - (view) Author: Daniil Bondarev (xonatius) * Date: 2015-02-03 06:32
Made a straightforward patch for this. Probably not that pretty, so suggestions are welcome.

Note that some function names will become pretty long in exceptions:

>>> class A:
...     def __init__(self):
...         def f():
...             pass
...         f(1,2,3)
...
>>> A()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 5, in __init__
TypeError: A.__init__.<locals>.f() takes 0 positional arguments but 3 were given

Passing UTs from lastest snapshot.
msg235315 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-02-03 06:34
I wonder if you could add this to the new code in http://bugs.python.org/issue17911 which I'm hoping to commit this week.
msg235462 - (view) Author: Daniil Bondarev (xonatius) * Date: 2015-02-06 04:01
As long as you think It fits into your issue, I'm ok with adding this patch to it (: it's different files, so no conflicts. Should I just add this patch to your ticket?
msg235476 - (view) Author: Matthias Urlichs (smurfix) * Date: 2015-02-06 10:01
Please do.
msg238772 - (view) Author: Daniil Bondarev (xonatius) * Date: 2015-03-21 08:13
issue17911 was submitted. I pulled latest updates and rechecked that "./python -m test -uall" passing with the same patch.
msg240546 - (view) Author: Daniil Bondarev (xonatius) * Date: 2015-04-12 07:58
Addrressed feedback: splitted long line in multiple in tests.
msg249081 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-08-24 23:49
Ok, so this is an API and ABI change. I'm going to do a variant without that downside.
msg249084 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-08-24 23:59
And herewith.
msg249087 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-08-25 00:13
Forgot docs - I'll do before committing, but not worried about review of them so much.
msg249174 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-08-26 02:07
Moving target version to 3.6 (since we're discussing adding a new public C API).

This is an interesting problem, as seeing an expanding API like this (PyEval_EvalCode -> PyEval_EvalCodeEx -> PyEval_EvalCodeEx2) suggests to me that we actually have a missing abstraction layer trying to get out.

My first reaction was to say "Hey, let's just make the eval loop function aware, and add PyEval_EvalFunction", but that creates a conceptual dependency loop we don't really want. 

So I asked myself a different question: why aren't we storing the qualname as a code object attribute, even though it's a known constant at compile time?

Looking at PEP 3155 (https://www.python.org/dev/peps/pep-3155/ ) it doesn't look like the question came up, and I don't recall it coming up either.

However, looking at https://hg.python.org/cpython/file/default/Python/compile.c I also don't see any reason why we *couldn't* provide a qualname field on code objects, such that f.__code__.co_qualname would give the same answer as f.__qualname__. The compiler already knows the qualname at compile time, we're just currently storing it as a separate constant that gets composed together with the code object at runtime.

My suspicion is that the reason this *wasn't* done is because it's a slightly more intrusive change to the code generation pipeline, but I currently suspect following up with that increase in compiler complexity would be a better option than making the PyEval_* API more complicated.
msg249175 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-08-26 02:08
Moving this back to patch review while we consider the alternative approach of moving qualname storage into code objects, rather than continuing to transport qualnames separately.
msg249176 - (view) Author: Ben Longbons (o11c) * Date: 2015-08-26 02:19
Code objects currently have no mutable fields.

So what are you planning to do about things like:

Foo = namedtuple('Foo', 'x y')
def frob(self):
    return self.x + self.y
# probably actually done via a @member(Foo) decorator
# so adding more code here is not a problem.
Foo.frob.__qualname__ = 'Foo.frob'
Foo.frob = frob
del frob
msg249178 - (view) Author: Ben Longbons (o11c) * Date: 2015-08-26 03:42
I made a minimal gist of my motivating code:

https://gist.github.com/o11c/ce0c2ff74b87ea71ad46
msg249185 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-08-26 07:21
The reason I didn't put __qualname__ on code objects is that code objects don't have a __module__ either. That information, up to now, belongs on the module.

Conceptually, a code object could very well be used by multiple functions living in different modules. Now, in practice, I'm not sure that happens (except when constructing function objects on your own, which is not a terribly supported usecase I think).

Also, Ben makes a very point about being able to change a __qualname__. In particular, the user should *still* be able to change a function's __qualname__ even if it's technically stored on the code object.
msg249186 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-08-26 07:22
Correcting myself:

> That information, up to now, belongs on the module.

on the function, of course.
msg249188 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-08-26 08:21
Note to self: this is about changing the name in an exception message, not in the back-trace of call sites that triggered an exception :)
msg249193 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-08-26 09:59
With __qualname__ being mutable, I agree that adding __code__.co_qualname wouldn't be a substitute for that. Instead, similar to the relationship between __name__ and __code__.co_name, they would start out the same, but the function attribute may change later (e.g. through the use of functools.wraps).

However, that also highlights why we need to use the compile time qualname in the traceback, rather than the runtime one: because we currently use the compile time name from the code object rather than the runtime name from the function.

A test case to demonstrate that:

>>> def f():
...     1/0
... 
>>> import functools
>>> @functools.wraps(f)
... def g():
...     return f()
... 
>>> g()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in g
  File "<stdin>", line 2, in f
ZeroDivisionError: division by zero
>>> g.__qualname__
'f'
>>> g.__name__
'f'

Note that "g" still appears in the relevant traceback line, even though both __name__ and __qualname__ have been updated to refer to "f". For a traceback we want to know where the source of the function actually lives, not where it claims to be from for human introspection purposes.

As far as the comparison to __module__ goes, I think that's a different case - we couldn't add that to the code object even if we wanted to, because the compiler doesn't know the module name, it only knows the file name. It's also possible to take an already compiled python file, move it to a different directory, and have the module name change due to the new location in the package hierarchy. __qualname__ is different as, like __name__, it's entirely local to the module and hence available to the compiler at compile time.
msg249194 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-08-26 10:17
Hmm, I'd made the same mistake Martin did - I was looking at the tracebacks moreso than at the exception message itself. Looking at the argument unpacking exception message in the context of the test case above, that also uses __code__.co_name rather than the runtime function name:

>>> g(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: g() takes 0 positional arguments but 1 was given

In this case though, I think that's arguably a problem, as we probably want error messages (unlike tracebacks) to include the "intended for human consumption" name, but they currently don't, they expose the name of the wrapper function if it's actually constraining its arguments rather than passing them through for subsequent validation:

>>> def make_wrapper(f):
...     @functools.wraps(f)
...     def wrapper():
...         return f()
...     return wrapper
... 
>>> @make_wrapper
... def g():
...     return f()
... 
>>> g(1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: wrapper() takes 0 positional arguments but 1 was given

That makes me more inclined towards a solution like Daniil's patch, but the growing signature of PyEval_Code* still bothers me. Perhaps we could collect the various runtime state arguments up into a "PyEvalScope" struct and make the new API PyEval_EvalCodeInScope(PyObject *code, PyObject *scope)?
msg249268 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-08-27 22:43
Here are some options.

a) Don't make the new thing public - instead export within Python.exe the existing private symbol _...withNames. Pros: no change to public API. Cons: extension modules using the public API cannot make these sorts of errors clearer.

b) Add a new API. Either narrow (add the parameter) or big (add a struct). Pros: everyone treated equally. Cons: More API surface area, and more complex calling convention.

c) use symbol versioning to change the existing API. Cons: more complex build-time detection for users. Pros: API surface area held constant.

d) Don't fix it. :)

I don't have a particular preference, though the struct thing is a wash IMO - it saves adding a serial to the API, at the cost of having a versioned datastructure which needs an embedded serial number. [Except perhaps that you can use symbol versioning to deal with evolutions of the struct - but thats fairly complex to carry off well, and I think this would be the first example w/in Python].
msg249310 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-08-29 02:25
I think a) is worth doing regardless - in many cases, third party libraries will be dealing with function, generator or coroutine objects, rather than with code objects directly, so they'll benefit even if the updated API remains private to the interpreter.

So let's narrow the scope of this particular issue to just that, and defer the idea of a new public API for the time being.
msg249558 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-02 17:28
See also issue4322.
msg272016 - (view) Author: Steven Barker (Steven.Barker) * Date: 2016-08-05 05:48
A few weeks ago I reported issue 27389 which looks to be a duplicate of this issue. Has any progress been made on this issue?
History
Date User Action Args
2016-10-25 22:02:52refi64setnosy: + refi64
2016-10-25 21:57:41martin.panterlinkissue28536 superseder
2016-08-05 06:45:11serhiy.storchakalinkissue27389 superseder
2016-08-05 05:48:10Steven.Barkersetnosy: + Steven.Barker
messages: + msg272016
2015-09-02 21:10:13martin.panterlinkissue4322 dependencies
2015-09-02 17:28:49serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg249558
2015-08-29 02:25:05ncoghlansetmessages: + msg249310
2015-08-27 22:43:21rbcollinssetmessages: + msg249268
2015-08-26 10:17:34ncoghlansetmessages: + msg249194
2015-08-26 09:59:55ncoghlansetmessages: + msg249193
2015-08-26 08:21:10martin.pantersetmessages: + msg249188
title: Names in traceback should have class names, if they're methods -> Names in function call exception should have class names, if they're methods
2015-08-26 07:22:56pitrousetmessages: + msg249186
2015-08-26 07:21:28pitrousetnosy: + gvanrossum
2015-08-26 07:21:16pitrousetmessages: + msg249185
2015-08-26 03:42:37o11csetmessages: + msg249178
2015-08-26 02:19:27o11csetnosy: + o11c
messages: + msg249176
2015-08-26 02:08:54ncoghlansetmessages: + msg249175
stage: commit review -> patch review
2015-08-26 02:07:16ncoghlansetnosy: + ncoghlan

messages: + msg249174
versions: + Python 3.6, - Python 3.4
2015-08-25 00:13:45rbcollinssetmessages: + msg249087
2015-08-24 23:59:48rbcollinssetfiles: + issue-2786-1.patch

messages: + msg249084
2015-08-24 23:49:02rbcollinssetmessages: + msg249081
2015-08-24 00:19:08rbcollinssetstage: needs patch -> commit review
2015-04-12 07:58:48xonatiussetfiles: + full_names.patch

messages: + msg240546
2015-03-21 08:13:35xonatiussetmessages: + msg238772
2015-02-06 10:01:52smurfixsetmessages: + msg235476
2015-02-06 04:01:12xonatiussetmessages: + msg235462
2015-02-03 07:41:36martin.pantersetnosy: + martin.panter
2015-02-03 06:34:40rbcollinssetnosy: + rbcollins
messages: + msg235315
2015-02-03 06:32:18xonatiussetfiles: + full_names.patch

nosy: + xonatius
messages: + msg235314

keywords: + patch
2014-12-12 01:17:28Arfreversetnosy: + Arfrever
2013-03-21 01:32:36ilblackdragonsetmessages: + msg184824
2013-03-18 08:52:00ilblackdragonsetnosy: + ilblackdragon
messages: + msg184439
2013-03-14 08:32:50ezio.melottisetversions: + Python 3.4, - Python 3.2
nosy: + pitrou, ezio.melotti

messages: + msg184151

keywords: + easy
stage: needs patch
2010-08-07 20:50:42terry.reedysetversions: + Python 3.2, - Python 2.6, Python 3.0
2008-05-11 22:26:55georg.brandlsetpriority: low
type: behavior -> enhancement
components: + Interpreter Core
versions: + Python 2.6, Python 3.0
2008-05-08 19:32:48belopolskysetnosy: + belopolsky
messages: + msg66433
2008-05-07 20:25:57smurfixcreate