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

Start should be a keyword argument of the built-in sum #75324

Closed
MarkCBell mannequin opened this issue Aug 8, 2017 · 10 comments
Closed

Start should be a keyword argument of the built-in sum #75324

MarkCBell mannequin opened this issue Aug 8, 2017 · 10 comments
Assignees
Labels
3.7 (EOL) end of life topic-ctypes type-feature A feature request or enhancement

Comments

@MarkCBell
Copy link
Mannequin

MarkCBell mannequin commented Aug 8, 2017

BPO 31141
Nosy @rhettinger, @vstinner, @stevendaprano, @serhiy-storchaka, @MarkCBell, @lisroach, @csabella
PRs
  • bpo-31141: Changed sum to take start as a keyword argument. #3022
  • Superseder
  • bpo-34637: Make start usable as a keyword argument for sum().
  • 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/lisroach'
    closed_at = <Date 2018-09-12.17:59:37.496>
    created_at = <Date 2017-08-08.11:52:48.120>
    labels = ['ctypes', 'type-feature', '3.7']
    title = 'Start should be a keyword argument of the built-in sum'
    updated_at = <Date 2018-09-13.06:20:51.911>
    user = 'https://github.com/MarkCBell'

    bugs.python.org fields:

    activity = <Date 2018-09-13.06:20:51.911>
    actor = 'vstinner'
    assignee = 'lisroach'
    closed = True
    closed_date = <Date 2018-09-12.17:59:37.496>
    closer = 'rhettinger'
    components = ['ctypes']
    creation = <Date 2017-08-08.11:52:48.120>
    creator = 'Mark.Bell'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 31141
    keywords = []
    message_count = 10.0
    messages = ['299908', '299957', '299968', '299973', '300356', '300420', '315790', '315791', '315793', '315794']
    nosy_count = 7.0
    nosy_names = ['rhettinger', 'vstinner', 'steven.daprano', 'serhiy.storchaka', 'Mark.Bell', 'lisroach', 'cheryl.sabella']
    pr_nums = ['3022']
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '34637'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue31141'
    versions = ['Python 3.7']

    @MarkCBell
    Copy link
    Mannequin Author

    MarkCBell mannequin commented Aug 8, 2017

    The built-in function sum takes an optional argument "start" to specify what value to start adding from (defaults to 0). This argument should be a keyword argument in order to match the other built-in functions such as:

    enumerate(range(10), start=5)
    

    This patch allows users to write:

    sum(range(10), start=5)
    

    which previously raised "TypeError: sum() takes no keyword arguments". Since the only change is making an optional positional argument into a keyword argument, this has no effect on any existing code using the current convention of:

    sum(range(10), 5)
    

    @MarkCBell MarkCBell mannequin added 3.7 (EOL) end of life topic-ctypes type-bug An unexpected behavior, bug, or error labels Aug 8, 2017
    @stevendaprano
    Copy link
    Member

    This seems like a reasonable enhancement to sum to me.

    Since 2.7 is in feature freeze, this can only apply to 3.7.

    @stevendaprano stevendaprano added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Aug 9, 2017
    @rhettinger
    Copy link
    Contributor

    Lisa, would you like to take this one?

    @serhiy-storchaka
    Copy link
    Member

    Adding this feature is so easy as moving '/' in Argument Clinic declaration one line up. I don't think it is worth to allow passing the first argument as a keyword argument.

    Check what performance effect of this change on simple calls sum(()), sum((), 0).

    @MarkCBell
    Copy link
    Mannequin Author

    MarkCBell mannequin commented Aug 16, 2017

    I ran some timing tests of the patch I submitted to compare it to the current build of Python. Using timit on the current master branch I got:

    python.exe -m timeit "sum(())"    .... 1.12 usec per loop
    python.exe -m timeit "sum((), 0)" .... 1.22 usec per loop
    

    And for the patched version:

    python.exe -m timeit "sum(())"    .... 1.46 usec per loop
    python.exe -m timeit "sum((), 0)" .... 1.57 usec per loop
    

    However my patch wasn't just the simple argument clinic change suggested by serhiy.storchaka, so maybe that would be more efficient and easier to understand.

    @serhiy-storchaka
    Copy link
    Member

    Your tests show that there is a performance regression of getting rid of Argument Clinic (in addition to increasing the maintenance cost of the code that was generated previously). Try to use the simple Argument Clinic change (it can has non-zero cost too, but I expect that its penalty is much smaller).

    @csabella
    Copy link
    Contributor

    Hi Mark,

    Are you able to make the Argument Clinic change the Serhiy suggested to come up with new benchmarks?

    Thanks!

    @serhiy-storchaka
    Copy link
    Member

    First than to allow this argument be passes by keyword, we mast choose its name. See the discussion "Start argument for itertools.accumulate()" on Python-ideas (https://mail.python.org/pipermail/python-ideas/2018-April/049649.html).

    @vstinner
    Copy link
    Member

    I don't think it is worth to allow passing the first argument as a keyword argument.

    I concur. Would you mind to add a test to make sure that passing the first argument as the "iterable" keyword doesn't work?

    "iterable" name comes from the Doc/library/functions.rst documentation and from the docstring.

    @serhiy-storchaka
    Copy link
    Member

    In 2.6 it was "sequence".

    @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
    3.7 (EOL) end of life topic-ctypes type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants