Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add introspection information for builtins #63873

Closed
larryhastings opened this issue Nov 20, 2013 · 16 comments
Closed

Add introspection information for builtins #63873

larryhastings opened this issue Nov 20, 2013 · 16 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@larryhastings
Copy link
Contributor

BPO 19674
Nosy @gvanrossum, @warsaw, @brettcannon, @birkenfeld, @ncoghlan, @larryhastings, @serhiy-storchaka
Files
  • larry.introspection.for.builtins.patch.1.txt: Rough patch with a first effort.
  • larry.introspection.for.builtins.patch.2.txt
  • larry.introspection.for.builtins.patch.3.txt
  • larry.introspection.for.builtins.patch.4.txt
  • 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/larryhastings'
    closed_at = <Date 2013-11-23.23:39:10.883>
    created_at = <Date 2013-11-20.23:52:26.200>
    labels = ['interpreter-core', 'type-bug']
    title = 'Add introspection information for builtins'
    updated_at = <Date 2013-11-23.23:39:10.883>
    user = 'https://github.com/larryhastings'

    bugs.python.org fields:

    activity = <Date 2013-11-23.23:39:10.883>
    actor = 'larry'
    assignee = 'larry'
    closed = True
    closed_date = <Date 2013-11-23.23:39:10.883>
    closer = 'larry'
    components = ['Interpreter Core']
    creation = <Date 2013-11-20.23:52:26.200>
    creator = 'larry'
    dependencies = []
    files = ['32739', '32763', '32785', '32815']
    hgrepos = []
    issue_num = 19674
    keywords = []
    message_count = 16.0
    messages = ['203549', '203550', '203551', '203575', '203576', '203583', '203607', '203667', '203672', '203684', '203797', '203827', '203908', '204026', '204120', '204129']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'barry', 'brett.cannon', 'georg.brandl', 'ncoghlan', 'larry', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue19674'
    versions = ['Python 3.4']

    @larryhastings
    Copy link
    Contributor Author

    Let's see if we can get introspection information for builtins using the Clinic for 3.4.

    Georg suggested part of the approach while we were hanging out in Tokyo. I'd considered it previously before but dispensed with the idea because it seemed too loopy. Actually it seems to work great.

    The approach:

    • Clinic generates an extra first line for the docstring, that looks like "def (...)\n". (Note: no function name!)

    • The PyCFunctionObject __doc__ getter detects this line and skips it if present.

    • Add a new getter to PyCFunctionObject, which I've called "__textsig__", that returns this first line if present in the docstring. (It skips the "def " at the front, and clips it at the newline.)

    • inspect now notices if it's passed in a PyCFunctionObject. If it gets one, it checks to see if it has a valid __textsig__. If it does, it parses it (using ast.parse) and produces a valid signature.

    Advantages of this approach:

    • It was easy to do and took very few lines of code.

    • For signatures that are impossible to convert to Clinic, we can
      write the metadata by hand.

    Disadvantages of this approach:

    • Uh, nothing, really!

    The next step is probably to convert pydoc to use inspect.signature instead of the manky old methods it currently uses. After that, clean up the patch, and add a unit test or two.

    What do you think?

    @larryhastings larryhastings self-assigned this Nov 20, 2013
    @larryhastings larryhastings added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Nov 20, 2013
    @larryhastings
    Copy link
    Contributor Author

    If you review, you can ignore the changes to the .c files, those only got touched because of the new autogenerated "def (" lines in the docstrings. No other changes there.

    @larryhastings
    Copy link
    Contributor Author

    (Well, except for Object/methodobject.c.)

    @gvanrossum
    Copy link
    Member

    I think there's a similar but slightly different convention in Sphinx
    autodoc -- the first line of the docstring can be "name(...)" . Isn't that
    better than def?

    @larryhastings
    Copy link
    Contributor Author

    I was deliberately trying to avoid something that a person might do by accident. Also, "def (" is consistently only five bytes, whereas "pterodactyl (" or whatever will often be much longer, in case static data size is a concern.

    But I could switch it to that if you think that's better. Certainly it's a tiny amount more readable.

    (I don't think this would make the docstrings compatible with sphinx autodoc though.)

    @serhiy-storchaka
    Copy link
    Member

    Great! See also bpo-16842 and bpo-16638.

    @ncoghlan
    Copy link
    Contributor

    Sounds good to me (with either Larry's or Guido's spelling).

    We should ensure this still works with the config option that disables docstrings entirely.

    @brettcannon
    Copy link
    Member

    For Nick: "still works" as in the metadata is still available without docstrings or "still works" as in it won't crash?

    @larryhastings
    Copy link
    Contributor Author

    I changed it to "<callablename>(". Patch attached. Still works.

    @ncoghlan
    Copy link
    Contributor

    "still works" as in "doesn't crash and the docstrings are still missing to
    save memory".

    @larryhastings
    Copy link
    Contributor Author

    Anybody have a better name for __textsig__ ?

    @brettcannon
    Copy link
    Member

    __signature_text__? __textsignature__? No need to abbreviate that much since there is no built-in function equivalent and people are not expected to work with it.

    @larryhastings
    Copy link
    Contributor Author

    I went with __text_signature__. I also did some more polishing, and added / fixed the unit tests, and even added a Misc/NEWS. Is it good enough to go in before the beta?

    @ncoghlan
    Copy link
    Contributor

    +1 from me (I see this as the key benefit of argument clinic).

    For 3.5, we can look at ducktyping on the attribute, but for now I think
    it's worth keeping that text format internal to CPython.

    @larryhastings
    Copy link
    Contributor Author

    Fresh patch attached. pydoc now uses inspect.signature instead of inspect.getfullargspec to generate the arguments for the function, and supports builtins. That's everything :D

    Planning on checking this in pretty soon, to get it in for beta (which hopefully still gets tagged today).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2013

    New changeset 78ec18f5cb45 by Larry Hastings in branch 'default':
    Issue bpo-19674: inspect.signature() now produces a correct signature
    http://hg.python.org/cpython/rev/78ec18f5cb45

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants