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 support for partial keyword arguments in extension functions #70470

Closed
serhiy-storchaka opened this issue Feb 3, 2016 · 10 comments
Closed
Assignees
Labels
expert-argument-clinic interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Feb 3, 2016

BPO 26282
Nosy @rhettinger, @larryhastings, @vadmium, @serhiy-storchaka
Files
  • pyarg_parse_positional_only_and_keywords.patch
  • pyarg_parse_positional_only_and_keywords_2.patch
  • zlib_compress_positional_only_data.patch
  • pyarg_parse_positional_only_and_keywords_3.patch
  • 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/serhiy-storchaka'
    closed_at = <Date 2016-06-10.18:55:30.859>
    created_at = <Date 2016-02-03.21:44:16.888>
    labels = ['interpreter-core', 'type-feature', 'expert-argument-clinic']
    title = 'Add support for partial keyword arguments in extension functions'
    updated_at = <Date 2016-06-10.18:55:30.858>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2016-06-10.18:55:30.858>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2016-06-10.18:55:30.859>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core', 'Argument Clinic']
    creation = <Date 2016-02-03.21:44:16.888>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['41868', '42681', '42682', '42779']
    hgrepos = []
    issue_num = 26282
    keywords = ['patch']
    message_count = 10.0
    messages = ['259521', '259525', '259773', '259933', '264639', '264640', '264972', '265120', '265143', '268141']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'larry', 'python-dev', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue26282'
    versions = ['Python 3.6']

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented Feb 3, 2016

    Currently extension functions either accept only positional-only arguments (PyArg_ParseTuple), or keyword arguments (PyArg_ParseTupleAndKeywords). While adding support passing by keywords looks good for some arguments, for other arguments it doesn't make much sense. For example "encoding" and "errors" arguments for str or "base" argument for int are examples of good keyword arguments, but it is hard to choose good name for the first argument.

    I suggest to allow to add the support of keyword arguments only for the part of arguments, while left other arguments positional-only. This issue consists from two stages:

    1. Allow PyArg_ParseTupleAndKeywords to accept empty string "" as keywords and interpret this as positional-only argument.

    2. Make Argument Clinic to generate code for partial keyword arguments. The syntax already supports this: "/" separates positional-only arguments from keyword arguments.

    @serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Feb 3, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Feb 4, 2016

    For the examples you gave, they already seem to support keywords for the first parameter: int(x=0), str(object=""), bytes(source=b""). Maybe it is a bit unfortunate that they are inconsistent, but these names seem reasonable when considered separately.

    But I agree that in those cases having the option of a keyword would rarely be useful. This proposal sounds like it could give a small benefit, and I guess it wouldn’t be costly to implement.

    @rhettinger
    Copy link
    Contributor

    rhettinger commented Feb 7, 2016

    +1 for this suggestion. There are a number of places where we've been trapped between having no keyword arguments or having to use keywords for all arguments even when it doesn't make sense to have all arguments be keywords.

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented Feb 9, 2016

    Proposed patch adds support for positional-only parameters in PyArg_ParseTupleAndKeywords(). It is simple, the most complex part is generating detailed error message for the case of calling with insufficient number of positional arguments.

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented May 2, 2016

    Removed development code from tests (thanks Martin for catching this) and adds support in Argument Clinic. Now the patch is complete and ready for final review.

    @serhiy-storchaka serhiy-storchaka added interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) expert-argument-clinic labels May 2, 2016
    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented May 2, 2016

    For example here is a patch that partially reverts bpo-26243 by making "data" positional parameter, but keeping "level" positional-or-keyword parameter.

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented May 6, 2016

    If there are no objections I'm inclined to commit pyarg_parse_positional_only_and_keywords_2.patch in short time.

    @serhiy-storchaka serhiy-storchaka self-assigned this May 6, 2016
    @vadmium
    Copy link
    Member

    vadmium commented May 8, 2016

    I made some suggestions in the documentation. I only skimmed over the C and arg clinic code, but there is nothing obviously wrong.

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented May 8, 2016

    On new patch applied Martin's suggestions, added entities in What's New and NEWS, added a reference to the glossary, and improved error message.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 10, 2016

    New changeset 69c0aa8a8185 by Serhiy Storchaka in branch 'default':
    Issue bpo-26282: PyArg_ParseTupleAndKeywords() and Argument Clinic now support
    https://hg.python.org/cpython/rev/69c0aa8a8185

    @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
    expert-argument-clinic interpreter-core Interpreter core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants