classification
Title: Replace PyEval_GetFuncName/PyEval_GetFuncDesc
Type: Stage: patch review
Components: Interpreter Core Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Ronan.Lamy, jdemeyer, miss-islington, petr.viktorin, vstinner
Priority: normal Keywords: patch

Created on 2019-07-21 17:46 by jdemeyer, last changed 2019-11-06 14:27 by petr.viktorin.

Pull Requests
URL Status Linked Edit
PR 14890 merged jdemeyer, 2019-07-21 19:15
PR 15295 open jdemeyer, 2019-08-14 22:04
Messages (13)
msg348255 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-07-21 17:46
PyEval_GetFuncName is bad API because

1. It hardcodes a limited number of function classes (which doesn't even include all function classes in the core interpreter) instead of supporting duck-typing.
2. In case of a "function" object, it relies on a borrowed reference to the function.
3. It is returning a C string instead of a Python string, so we cannot return a new reference even if we wanted to.

Since PyEval_GetFuncName and PyEval_GetFuncDesc are always used together, we might as well replace them by a single function with a proper API.
msg348257 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-07-21 18:02
4. It uses the __name__ instead of the __qualname__
msg349007 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-08-04 21:23
Another solution would be to change the __str__ of various function objects to a prettier output. For example, we currently have

>>> def f(): pass
>>> print(f)
<function f at 0x7f9f4bbe5e18>

We could change this to

>>> def f(): pass
>>> print(f)
f()

and then use "%S" to display the functions in error messages. But I have a feeling that this is a more controversial change than PR 14890.
msg349690 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-14 12:24
Maybe repr(func) should be left unchanged, but str(func) can be enhanced?
msg349691 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-08-14 12:45
> Maybe repr(func) should be left unchanged, but str(func) can be enhanced?

Yes, that is what I meant.
msg349807 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-08-15 15:02
I am not convinced.

I'm wary of making error messages depend on the str representation of a function; that would prevent us from changing it later.
I'm wary of "%S" used in error messages. Those are for the programmer, not the user, so they should prefer __repr__.

I train beginners to recognize "<function f at 0x7f9f4bbe5e18>" as a sign of omitted parentheses. The ugliness is useful: it shows you're dealing with an internal object, not a data value.

So, I think "<function f>" is much better than just "f()". I wouldn't mind "<function f()>" (maybe even with the full signature), but that doesn't quite help this case.
(I don't care much for the "at 0x7f9f4bbe5e18" part, but that's not the issue here.)
msg349903 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-08-17 18:29
> I'm wary of making error messages depend on the str representation of a function; that would prevent us from changing it later.

Why wouldn't we be able to change anything? Typically, the exact string of an error message is NOT part of the API (the exception *type* is, but we're not talking about that).

> I'm wary of "%S" used in error messages. Those are for the programmer, not the user

I'm not following here. Given that Python is a programming language, the user *is* the programmer.

Anyway, you don't have to be convinced. I'm trying to solve a problem here and I have two approaches (PR 14890 and PR 15295). I'm more inclined towards PR 15295, but if you like the idea of PR 14890 better, we can go with that instead.
msg349904 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-08-17 18:45
> I'm wary of "%S" used in error messages.

Maybe you're misunderstanding something. The goal is not really to change error messages, only the way how they are produced. For example, we currently have

>>> def f(): pass
>>> f(**1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: f() argument after ** must be a mapping, not int

This is about how the "f()" in the error message is produced. Currently, this uses PyEval_GetFuncName() and PyEval_GetFuncDesc(). For the reasons explained in this issue, I want to replace that.

I see two ways of doing this:

1. (PR 14890) Write a new function _PyObject_FunctionStr(func) which returns func.__qualname__ + "()" with a suitable fallback if there is no __qualname__ attribute. At some point, we could also introduce a %F format character for this.

2. (PR 15295) Use str(func) in the error message and change various __str__ methods (really tp_str functions) to give a more readable output.
msg351946 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-09-11 15:32
I like PR 14890 better. I like the separation of representation for error messages (where it's clearer that this is a callable) and for __str__.

Also, changing the __str__ of functions would need much wider discussion than on issues/PRs.

I left some comments on the PR.
msg352019 - (view) Author: Jeroen Demeyer (jdemeyer) * (Python triager) Date: 2019-09-11 19:40
> I left some comments on the PR.

I don't see anything. Either I'm doing something wrong or GitHub is messed up.
msg352028 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-09-11 22:44
My bad, I didn't publish the comments. They should be there now.
msg356045 - (view) Author: miss-islington (miss-islington) Date: 2019-11-05 16:53
New changeset bf17d41826a8bb4bc1e34ba6345da98aac779e41 by Miss Islington (bot) (Jeroen Demeyer) in branch 'master':
bpo-37645: add new function _PyObject_FunctionStr() (GH-14890)
https://github.com/python/cpython/commit/bf17d41826a8bb4bc1e34ba6345da98aac779e41
msg356134 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2019-11-06 14:27
With this change, CPython no longer uses PyEval_GetFuncName/PyEval_GetFuncDesc internally.
Shall we deprecate/discourage them?
Shall we expose PyObject_FunctionStr publicly?
History
Date User Action Args
2019-11-06 14:27:20petr.viktorinsetmessages: + msg356134
2019-11-05 16:53:52miss-islingtonsetnosy: + miss-islington
messages: + msg356045
2019-10-31 16:18:01Ronan.Lamysetnosy: + Ronan.Lamy
2019-09-11 22:44:36petr.viktorinsetmessages: + msg352028
2019-09-11 19:40:00jdemeyersetmessages: + msg352019
2019-09-11 15:32:36petr.viktorinsetmessages: + msg351946
2019-08-17 18:45:22jdemeyersetmessages: + msg349904
2019-08-17 18:29:36jdemeyersetmessages: + msg349903
2019-08-15 15:02:26petr.viktorinsetnosy: + petr.viktorin
messages: + msg349807
2019-08-14 22:04:55jdemeyersetpull_requests: + pull_request15018
2019-08-14 12:45:05jdemeyersetmessages: + msg349691
2019-08-14 12:24:08vstinnersetmessages: + msg349690
2019-08-04 21:23:04jdemeyersetmessages: + msg349007
2019-07-21 19:15:17jdemeyersetkeywords: + patch
stage: patch review
pull_requests: + pull_request14673
2019-07-21 18:02:07jdemeyersetmessages: + msg348257
2019-07-21 17:46:15jdemeyercreate