Skip to content

accept keyword arguments on most base type methods and builtins #52952

Closed as not planned

Description

@gpshead
BPO 8706
Nosy @rhettinger, @gpshead, @ezio-melotti, @merwok, @briancurtin, @methane, @ericsnowcurrently, @berkerpeksag, @vadmium, @serhiy-storchaka, @nchammas
Files
  • 8706.patch
  • 8706_2.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 = None
    closed_at = None
    created_at = <Date 2010-05-13.19:10:53.581>
    labels = ['extension-modules', 'interpreter-core', 'type-feature', '3.7']
    title = 'accept keyword arguments on most base type methods and builtins'
    updated_at = <Date 2017-01-21.07:13:37.955>
    user = 'https://github.com/gpshead'

    bugs.python.org fields:

    activity = <Date 2017-01-21.07:13:37.955>
    actor = 'methane'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'Interpreter Core']
    creation = <Date 2010-05-13.19:10:53.581>
    creator = 'gregory.p.smith'
    dependencies = []
    files = ['24644', '24650']
    hgrepos = []
    issue_num = 8706
    keywords = ['patch']
    message_count = 25.0
    messages = ['105652', '154340', '154379', '154491', '154493', '154505', '154506', '154507', '154509', '154510', '154522', '154524', '154528', '154534', '154725', '154743', '154745', '154748', '154749', '223570', '241589', '241606', '241610', '285924', '285942']
    nosy_count = 16.0
    nosy_names = ['rhettinger', 'gregory.p.smith', 'ezio.melotti', 'eric.araujo', 'Arfrever', 'brian.curtin', 'gruszczy', 'methane', 'meatballhat', 'eric.snow', 'Ramchandra Apte', 'berker.peksag', 'martin.panter', 'serhiy.storchaka', 'Julien.Palard', 'nchammas']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue8706'
    versions = ['Python 3.7']

    Activity

    1. gpshead commented on May 13, 2010

      @gpshead
      MemberAuthor

      C Python has a real wart in that standard types and library functions that are implemented in C do not always accept keyword arguments:

      >>> 'xxxxxx'.find('xx', 4)
      4
      >>> 'xxxxxx'.find('xx', start=4)
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      TypeError: find() takes no keyword arguments
      >>> 

      While other things do accept keywords:

      sorted(s, key=bla)

      We should clean this up. It is not well documented anywhere and I suspect other python implementations (haven't tested this) may accept keywords on these where C Python doesn't.

      In string.find()'s case it looks like this is because it is an old style C method declaration that only gets an args tuple, no keyword args dict.

    2. gruszczy commented on Feb 26, 2012

      gruszczymannequin
      Mannequin

      I don't know if this is exactly what you want, but this is an early patch.

    3. gruszczy commented on Feb 26, 2012

      gruszczymannequin
      Mannequin

      With tests.

    4. gpshead commented on Feb 27, 2012

      @gpshead
      MemberAuthor

      At a quick glance, I think you've got the right idea. Fixing this involves a lot of PyArg_ParseTuple -> PyArg_ParseTupleAndKeywords upgrades all over the place.

      Obviously there are are a wide range of things that can use updating for this so in order to keep things sane, it makes sense to do them in stages with individual patches for particular libraries.

      I like that your 8706_2.patch chose to attack stringlib to start with as that covers a lot of ground with one piece of code and that you updated the tests to confirm that ordered args and keyword args both work.

      This issue as filed doesn't have a well defined "end goal" to know when it is complete. Another good thing would be to come up with a final goal. Perhaps an audit of the stdlib and types to list which things do not properly accept keyword arguments? That way there is a list of things to check off. And if desired we could file sub-issues for various components rather than letting this one get huge.

    5. gruszczy commented on Feb 27, 2012

      gruszczymannequin
      Mannequin

      Do you think I should put everything into a single patch or rather slowly add new patches with different methods or method groups?

      I would rather split it into several patches, I think it is easier to manage them, especially that this one is quite huge already.

    6. gpshead commented on Feb 27, 2012

      @gpshead
      MemberAuthor

      Several patches for sure! and give the patch files useful names
      indicating which things they touch.

    7. ezio-melotti commented on Feb 27, 2012

      @ezio-melotti
      Member

      On one hand I agree that it would be nice to get rid of these implementation details that prevent some C functions/methods to accept keyword args, but on the other hand I'm not sure that changing them all is the right thing to do.
      For some functions/methods being able to pass keyword args make the code more readable/flexible, but for some other there's no real gain.
      It also seems to me that since the arguments where only positional, not much thought went into choosing an appropriate name for these arguments.
      For example str.join() is documented as str.join(iterable), and the C function calls the argument 'data'.
      If we use those names, we won't have a chance to fix them later, so we should be careful before doing a mass-update.

    8. briancurtin commented on Feb 27, 2012

      @briancurtin
      Member

      For some functions/methods being able to pass keyword args make the code more readable/flexible, but for some other there's no real gain.

      I know what you're saying with the last part, but I think everyone becomes a winner in the consistency game if we expose kwargs regardless of level of improvement compared to other places.

    9. ezio-melotti commented on Feb 27, 2012

      @ezio-melotti
      Member

      FWIW PyPy doesn't seem to support keyword args for e.g. str.join() (so that's extra work for them too), and I don't see what would be the advantage of being able to do '-'.join(iterable=a_list). Even if I also don't see a valid reason why that shouldn't work and realize that it might be surprising for someone, I'm not sure it's worth changing it just for the sake of being consistent everywhere.

    10. gruszczy commented on Feb 27, 2012

      gruszczymannequin
      Mannequin

      I would stay away from methods that accept just a single argument. For those that accept more, I think it's reasonable to allow keyword args.

    11. rhettinger commented on Feb 28, 2012

      @rhettinger
      Contributor

      For many builtins, this would be a total waste, slowing down the calls, and supplying keyword names that would be meaningless or just weird. For example, chr(i=65) is just a waste.

      Keyword args should only be added where they would add clarity to something that would otherwise be ambiguous.

      FWIW, that is the reason that sorted() required a keyword argument. It prevents confusion between key-functions and cmp-functions.

    12. RamchandraApte commented on Feb 28, 2012

      RamchandraAptemannequin
      Mannequin

      See also bpo-14081 which got fixed.

    13. RamchandraApte commented on Feb 28, 2012

      RamchandraAptemannequin
      Mannequin

      Can we have keyword arguments to range([start],stop,[step])?

    14. rhettinger commented on Feb 28, 2012

      @rhettinger
      Contributor

      range() has been around 20+ years and there has been zero need for keyword arguments for it.

      FWIW, the maxsplit keyword argument was a case where a keyword argument added clarity, and there may be a handful of other cases that are also warranted. Greg Smith's idea for "start" for str.find() may be one of those.

      People are welcome to propose individual suggestions in separate tracker items; however, if someone wants a wholesale set of changes to core builtins, then they should discuss it on python-ideas. It would also be worthwhile for someone to benchmark the performance cost of switching from "METH_O" to "METH_VARARGS | METH_KEYWORDS".

    15. 6 remaining items

    16. JulienPalard commented on Jul 21, 2014

      @JulienPalard
      Member

      I think for some builtins it may be usefull to have keyword arguments, in the case they take more than one parameter.

      Typically, it's impossible to write:

          self.drop_elements(partial(isinstance, type(lxml.etree.Comment)))

      Because isinstance take its argument in the "other" order, we may bypass this using keywords arguments:

          self.drop_elements(partial(isinstance, type=type(lxml.etree.Comment)))

      But isinstance refuses keyword arguments, so there is no way to write this without a lambda:

          self.drop_elements(lambda x: isinstance(x,
                             type(lxml.etree.Comment)))

      With is cool and work, I agree, it's just an example to explicitly show why keywords argument may be cool: functools.partial.

    17. vadmium commented on Apr 20, 2015

      @vadmium
      Member

      See also bpo-23738, which identifies some functions whose documentation already suggests they accept keywords. Perhaps these functions could be prioritized.

      Also, I think “version changed” notices should be added in the documentation when a function grows support for keyword arguments.

    18. serhiy-storchaka commented on Apr 20, 2015

      @serhiy-storchaka
      Member

      Supporting keyword arguments has performance loss. For fast builtins it make be significant. We should defer adding keyword arguments support until more efficient parsing will implemented. Note that it is easier to implement efficient argument parsing for functions with positional-only arguments (see for example bpo-23867).

    19. gpshead commented on Apr 20, 2015

      @gpshead
      MemberAuthor

      I wouldn't make an efficiency argument against it without trying it and
      showing reproducible degradation in the hg.python.org/benchmarks suite.

      On Sun, Apr 19, 2015, 10:31 PM Serhiy Storchaka <[email protected]>
      wrote:

      Serhiy Storchaka added the comment:

      Supporting keyword arguments has performance loss. For fast builtins it
      make be significant. We should defer adding keyword arguments support until
      more efficient parsing will implemented. Note that it is easier to
      implement efficient argument parsing for functions with positional-only
      arguments (see for example bpo-23867).

      ----------
      nosy: +serhiy.storchaka


      Python tracker <[email protected]>
      <http://bugs.python.org/issue8706\>


    20. serhiy-storchaka commented on Jan 20, 2017

      @serhiy-storchaka
      Member

      Most overhead of supporting keyword arguments when pass positional arguments was removed in bpo-29029. But still there is an overhead of passing argument by keywords. It can be decreased when convert functions to Argument Clinic or use new private argument parsing API. This is not very easy issue.

    21. methane commented on Jan 21, 2017

      @methane
      Member

      TL;DR

      Changing positional argument name doesn't break backward compatibility.
      After start accepting keyword argument, it's name is part of API and should be stable.

      For example, document says str.startswith(prefix[, start[, end]]).
      But this patch seems using sub instead of prefix.
      https://docs.python.org/3.5/library/stdtypes.html#str.startswith

      Keyword name should be chosen carefully, like choosing method name.
      So I'm -1 to adding keyword argument support to many method in one issue.

    22. transferred this issue fromon Apr 10, 2022
    23. added
      3.13only security fixes
      and removed on Apr 29, 2023
    24. arhadthedev commented on Apr 29, 2023

      @arhadthedev
      Member

      Is this idea still worth implementation? It seems that the only methods touched by the patches and still unported to Argument Clinic are thin wrappers around _Py_bytes_* and alike.

    25. gpshead commented on Apr 29, 2023

      @gpshead
      MemberAuthor

      I'll just close it. In the 13 years since filing the idea argument clinic was invented and a lot has moved to that. points made above and in other discussions about the keyword argument remain true, and in 3.8 we got https://peps.python.org/pep-0570/ for positional only arguments from the Python side via the / token. So a request to do it everywhere makes even less sense.

      Performance wise, I expect the existing faster cpython work may ultimately change the shape of what does and doesn't perform well. It could, for example, specialize around differences in argument passing styles in hot paths if that winds up being worthwhile.

    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

      3.13only security fixesextension-modulesC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions