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

Argument Clinic: support for simple expressions? #64425

Closed
birkenfeld opened this issue Jan 12, 2014 · 24 comments
Closed

Argument Clinic: support for simple expressions? #64425

birkenfeld opened this issue Jan 12, 2014 · 24 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@birkenfeld
Copy link
Member

BPO 20226
Nosy @birkenfeld, @larryhastings, @meadori, @zware, @serhiy-storchaka
Files
  • larry.clinic.rollup.patch.two.diff.1.txt
  • larry.clinic.rollup.patch.two.diff.2.txt
  • larry.clinic.rollup.patch.two.diff.3.txt
  • larry.clinic.rollup.patch.two.diff.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 2014-01-16.19:33:41.897>
    created_at = <Date 2014-01-12.10:29:49.464>
    labels = ['type-feature']
    title = 'Argument Clinic: support for simple expressions?'
    updated_at = <Date 2014-01-26.04:48:06.172>
    user = 'https://github.com/birkenfeld'

    bugs.python.org fields:

    activity = <Date 2014-01-26.04:48:06.172>
    actor = 'larry'
    assignee = 'larry'
    closed = True
    closed_date = <Date 2014-01-16.19:33:41.897>
    closer = 'larry'
    components = []
    creation = <Date 2014-01-12.10:29:49.464>
    creator = 'georg.brandl'
    dependencies = []
    files = ['33473', '33492', '33499', '33500']
    hgrepos = []
    issue_num = 20226
    keywords = []
    message_count = 24.0
    messages = ['207946', '207947', '207956', '207957', '208135', '208139', '208175', '208176', '208177', '208178', '208253', '208254', '208255', '208259', '208260', '208266', '208288', '208290', '208291', '208292', '208293', '208302', '208303', '208314']
    nosy_count = 6.0
    nosy_names = ['georg.brandl', 'larry', 'meador.inge', 'python-dev', 'zach.ware', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue20226'
    versions = ['Python 3.4']

    @birkenfeld
    Copy link
    Member Author

    Take for example select.epoll.__new__():

    static PyObject *
    pyepoll_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
    {
        int flags = 0, sizehint = FD_SETSIZE - 1;
        static char *kwlist[] = {"sizehint", "flags", NULL};
        if (!PyArg_ParseTupleAndKeywords(args, kwds, "|ii:epoll", kwlist,
                                         &sizehint, &flags))
            return NULL;

    How should that be handled? Currently I can't specify FD_SETSIZE - 1 as the default value, but I don't want a magic -909 either :)

    @serhiy-storchaka
    Copy link
    Member

    See also a discussion in bpo-20193. zlib needs this in many functions.

    @larryhastings
    Copy link
    Contributor

    Yes, this is supported, just undocumented (for now). The magic you want is, for example in Modules/_sre.c:

    endpos: Py_ssize_t(c_default="PY_SSIZE_T_MAX") = sys.maxsize

    @birkenfeld
    Copy link
    Member Author

    Nice, thanks.

    @larryhastings
    Copy link
    Contributor

    The attached patch extends Argument Clinic and the inspect module: now you may use simple (!) expressions as default values. So for example "sys.maxsize - 1" should now work fine.

    • Names may be in the current module; as a backup they will also be looked up in sys.modules.
    • You may look up attributes of names.
    • You may subscript into names.
    • You may use math operators, both unary and binary.
    • int/float/string literals are (still) fine.
    • You may not use: function calls, list/set/dict/tuple/bytes literals, generator expressions or comprehensions, if expressions, anything fun.

    Other changes in this diff:

    • "doc_default" is gone, it no longer makes sense.
    • "py_default" and "c_default" handling is simpler and more predictable.
    • Minor doc fix suggested on tracker.

    The docs still need work. More tests would be good too but I'm already overwhelmed by work--could one of you guys contribute some tests?

    @larryhastings larryhastings added the type-feature A feature request or enhancement label Jan 15, 2014
    @meadori
    Copy link
    Member

    meadori commented Jan 15, 2014

    Larry, nice. I am verified that your latest patch fixes my third sqlite3
    nit from bpo-20178.

    @zware
    Copy link
    Member

    zware commented Jan 15, 2014

    Just confirmed that this patch fixes the traceback I was getting from winreg in bpo-20172, but this has a new oddity:

    >>> help(winreg)
    ...
        ConnectRegistry(computer_name=<object object at 0x0190E148>, key=<object object at 0x0190E148>)
    ...

    Every param of every function has this kind of thing, positional or keyword, default or no. I'll try to look into it, but my hopes aren't high (much of clinic's inner workings are still mysterious to me).

    @larryhastings
    Copy link
    Contributor

    That's okay, email me a sample at

    larry at hastings dot org

    and I'll take a look at it. Be sure to tell me which patch(es) were applied at the time.

    (Note: all you need to send is the Clinic block.)

    @zware
    Copy link
    Member

    zware commented Jan 15, 2014

    Actually, I can reproduce on tip with just this patch applied; os.chmod shows it. And I was wrong, params that have a default are correct, it's just ones without.

    @larryhastings
    Copy link
    Contributor

    I'd prefer it if you emailed me as I asked. That will help me get to it a lot quicker.

    @larryhastings
    Copy link
    Contributor

    Okay, I have a fix for the help(os.chmod) problem, that'll be in the next refreshed patch.

    @serhiy-storchaka
    Copy link
    Member

    Why not allow arbitrary string as py_default?

    @larryhastings
    Copy link
    Contributor

    Isn't that what it does?

    I may actually remove py_default. Nobody uses it, and I don't think you need it--you just specify what you want as the real default value.

    @larryhastings
    Copy link
    Contributor

    Second rollup patch. More small fixes, many suggested by other folks.

    @serhiy-storchaka
    Copy link
    Member

    It would be better if you commit a patch which changes PyTuple_Size to PyTuple_GET_SIZE in separate commit.

    @serhiy-storchaka
    Copy link
    Member

    Excellent! I'm rewriting the zlib module and the code becomes much cleaner with this patch.

    There is one problem left -- Py_buffer doesn't support default value at all.

    Such code

    /[clinic input]
    zlib.compressobj
    zdict: Py_buffer = unspecified
    [clinic start generated code]
    /

    produces error:

    When you specify a named constant ('unspecified') as your default value,
    you MUST specify a valid c_default.

    But this code

    /[clinic input]
    zlib.compressobj
    zdict: Py_buffer(c_default="{NULL, NULL}") = unspecified
    [clinic start generated code]
    /

    produces error too:

    The only legal default value for Py_buffer is None.

    And specifying c_default=None returns first error.

    @larryhastings
    Copy link
    Contributor

    Py_buffer is currently inflexible. The only default value it accepts is None. If you want it to be required, give it no default value. If you want it to be optional, give it a default value of None.

    Do you have a use case for any other default value?

    @larryhastings
    Copy link
    Contributor

    Third patch, changes based on Georg's review. Thanks, Georg!

    @birkenfeld
    Copy link
    Member Author

    Rietveld doesn't like your new patch?

    @larryhastings
    Copy link
    Contributor

    Sorry, forgot to update trunk, here you go.

    @birkenfeld
    Copy link
    Member Author

    Thx! (without soundcheck)

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 16, 2014

    New changeset abf87e1fbc62 by Larry Hastings in branch 'default':
    Issue bpo-20226: Major improvements to Argument Clinic.
    http://hg.python.org/cpython/rev/abf87e1fbc62

    @larryhastings
    Copy link
    Contributor

    Done!

    Georg: Thanks for the suggestion, and for the reviews! Argument Clinic is improved this day.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 16, 2014

    New changeset cd3fdf21a6e4 by Larry Hastings in branch 'default':
    Issue bpo-20226: Added tests for new features and regressions.
    http://hg.python.org/cpython/rev/cd3fdf21a6e4

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    AA-Turner pushed a commit to AA-Turner/devguide that referenced this issue Sep 13, 2023
    * You may now specify an expression as the default value for a
      parameter!  Example: "sys.maxsize - 1".  This support is
      intentionally quite limited; you may only use values that
      can be represented as static C values.
    * Removed "doc_default", simplified support for "c_default"
      and "py_default".  (I'm not sure we still even need
      "py_default", but I'm leaving it in for now in case a
      use presents itself.)
    * Parameter lines support a trailing '\\' as a line
      continuation character, allowing you to break up long lines.
    * The argument parsing code generated when supporting optional
      groups now uses PyTuple_GET_SIZE instead of PyTuple_GetSize,
      leading to a 850% speedup in parsing.  (Just kidding, this
      is an unmeasurable difference.)
    * A bugfix for the recent regression where the generated
      prototype from pydoc for builtins would be littered with
      unreadable "=<object ...>"" default values for parameters
      that had no default value.
    * Converted some asserts into proper failure messages.
    * Many doc improvements and fixes.
    
    GitHub-Issue-Link: python/cpython#64425
    erlend-aasland pushed a commit to python/devguide that referenced this issue Sep 26, 2023
    * You may now specify an expression as the default value for a
      parameter!  Example: "sys.maxsize - 1".  This support is
      intentionally quite limited; you may only use values that
      can be represented as static C values.
    * Removed "doc_default", simplified support for "c_default"
      and "py_default".  (I'm not sure we still even need
      "py_default", but I'm leaving it in for now in case a
      use presents itself.)
    * Parameter lines support a trailing '\\' as a line
      continuation character, allowing you to break up long lines.
    * The argument parsing code generated when supporting optional
      groups now uses PyTuple_GET_SIZE instead of PyTuple_GetSize,
      leading to a 850% speedup in parsing.  (Just kidding, this
      is an unmeasurable difference.)
    * A bugfix for the recent regression where the generated
      prototype from pydoc for builtins would be littered with
      unreadable "=<object ...>"" default values for parameters
      that had no default value.
    * Converted some asserts into proper failure messages.
    * Many doc improvements and fixes.
    
    GitHub-Issue-Link: python/cpython#64425
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants