Skip to content

Replace PyEval_GetFuncName/PyEval_GetFuncDesc #81826

Open
Open
Replace PyEval_GetFuncName/PyEval_GetFuncDesc#81826
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)topic-C-API

Description

@jdemeyer
jdemeyer
opened on Jul 21, 2019
BPO 37645
Nosy @vstinner, @encukou, @rlamy, @jdemeyer, @miss-islington
PRs
  • bpo-37645: add new function _PyObject_FunctionStr() #14890
  • bpo-37645: simplify __str__ of function objects #15295
    		•

    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 = None
closed_at = None
created_at = <Date 2019-07-21.17:46:15.048>
labels = ['interpreter-core', '3.9']
title = 'Replace PyEval_GetFuncName/PyEval_GetFuncDesc'
updated_at = <Date 2019-11-06.14:27:20.317>
user = 'https://github.com/jdemeyer'

    bugs.python.org fields:

    activity = <Date 2019-11-06.14:27:20.317>
actor = 'petr.viktorin'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2019-07-21.17:46:15.048>
creator = 'jdemeyer'
dependencies = []
files = []
hgrepos = []
issue_num = 37645
keywords = ['patch']
message_count = 13.0
messages = ['348255', '348257', '349007', '349690', '349691', '349807', '349903', '349904', '351946', '352019', '352028', '356045', '356134']
nosy_count = 5.0
nosy_names = ['vstinner', 'petr.viktorin', 'Ronan.Lamy', 'jdemeyer', 'miss-islington']
pr_nums = ['14890', '15295']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue37645'
versions = ['Python 3.9']

    Activity

    1. jdemeyer commented on Jul 21, 2019

      @jdemeyer
      jdemeyer
      on Jul 21, 2019
      ContributorAuthor

      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.

    2. jdemeyer
      added
      3.9 (EOL)end of life
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on Jul 21, 2019

    3. jdemeyer commented on Jul 21, 2019

      @jdemeyer
      jdemeyer
      on Jul 21, 2019
      ContributorAuthor
      1. It uses the __name__ instead of the __qualname__

    4. jdemeyer commented on Aug 4, 2019

      @jdemeyer
      jdemeyer
      on Aug 4, 2019
      ContributorAuthor

      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.

    5. vstinner commented on Aug 14, 2019

      @vstinner
      vstinner
      on Aug 14, 2019
      Member

      Maybe repr(func) should be left unchanged, but str(func) can be enhanced?

    6. jdemeyer commented on Aug 14, 2019

      @jdemeyer
      jdemeyer
      on Aug 14, 2019
      ContributorAuthor

      Maybe repr(func) should be left unchanged, but str(func) can be enhanced?

      Yes, that is what I meant.

    7. encukou commented on Aug 15, 2019

      @encukou
      encukou
      on Aug 15, 2019
      Member

      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.)

    8. jdemeyer commented on Aug 17, 2019

      @jdemeyer
      jdemeyer
      on Aug 17, 2019
      ContributorAuthor

      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.

    9. jdemeyer commented on Aug 17, 2019

      @jdemeyer
      jdemeyer
      on Aug 17, 2019
      ContributorAuthor

      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.

    10. encukou commented on Sep 11, 2019

      @encukou
      encukou
      on Sep 11, 2019
      Member

      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.

    11. jdemeyer commented on Sep 11, 2019

      @jdemeyer
      jdemeyer
      on Sep 11, 2019
      ContributorAuthor

      I left some comments on the PR.

      I don't see anything. Either I'm doing something wrong or GitHub is messed up.

    12. encukou commented on Sep 11, 2019

      @encukou
      encukou
      on Sep 11, 2019
      Member

      My bad, I didn't publish the comments. They should be there now.

    13. miss-islington commented on Nov 5, 2019

      @miss-islington
      miss-islington
      on Nov 5, 2019
      Contributor

      New changeset bf17d41 by Miss Islington (bot) (Jeroen Demeyer) in branch 'master':
      bpo-37645: add new function _PyObject_FunctionStr() (GH-14890)
      bf17d41

    14. encukou commented on Nov 6, 2019

      @encukou
      encukou
      on Nov 6, 2019
      Member

      With this change, CPython no longer uses PyEval_GetFuncName/PyEval_GetFuncDesc internally.
      Shall we deprecate/discourage them?
      Shall we expose PyObject_FunctionStr publicly?

    15. ezio-melotti
      transferred this issue fromon Apr 10, 2022

    16. sobolevn commented on Oct 16, 2022

      @sobolevn
      sobolevn
      on Oct 16, 2022
      Member

      I've recently covered these two functions in tests, so this issue caught my eye.
      Can we start the deprecation process? Or is there anything holding us back?

    17. encukou commented on Oct 17, 2022

      @encukou
      encukou
      on Oct 17, 2022
      Member

      Before deprecating them, we should provide a good replacement for the use case they served.
      It could mean exposing PyObject_FunctionStr, which would mean documenting it, and deciding what kind of guarantees we have for it. Is the output gonna change? Or is this something like “the perferred way to reference callable objects” -- and in that case, why isn't it the same as str?
      Or instead of exposing PyObject_FunctionStr there could just be a “recipe” with hints on how to replace them.

    18. erlend-aasland
      added
      topic-C-API
      and removed
      3.9 (EOL)end of life
      on Jan 12, 2024

    19. vstinner commented on Mar 18, 2024

      @vstinner
      vstinner
      on Mar 18, 2024
      Member

      I found 3 projects using PyEval_GetFuncName() in PyPI top 8,000 projects (at 2024-03-16):

      • Nuitka (2.1.2)
      • pyuwsgi-2.0.23.post0
      • uwsgi (2.0.24)

      Code:

      Nuitka-2.1.2.tar.gz: Nuitka-2.1.2/nuitka/build/static_src/HelpersCalling.c: return PyEval_GetFuncName(PyMethod_GET_FUNCTION(object));
pyuwsgi-2.0.23.post0.tar.gz: pyuwsgi-2.0.23.post0/plugins/python/profiler.c: PyEval_GetFuncName(arg), code->co_argcount, code->co_stacksize);
uwsgi-2.0.24.tar.gz: uwsgi-2.0.24/plugins/python/profiler.c: PyEval_GetFuncName(arg), code->co_argcount, code->co_stacksize);

      I found no matches for PyEval_GetFuncDesc().

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      interpreter-core(Objects, Python, Grammar, and Parser dirs)topic-C-API

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions