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

functools.partial: don't copy keywoard arguments in partial_call()? #72027

Closed
vstinner opened this issue Aug 23, 2016 · 6 comments
Closed

functools.partial: don't copy keywoard arguments in partial_call()? #72027

vstinner opened this issue Aug 23, 2016 · 6 comments
Labels
performance Performance or resource usage

Comments

@vstinner
Copy link
Member

BPO 27840
Nosy @rhettinger, @pitrou, @vstinner, @berkerpeksag, @serhiy-storchaka
PRs
  • Document why functools.partial() must copy kwargs #253
  • Files
  • partial_call_kwargs.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 = <Date 2017-02-27.12:36:56.749>
    created_at = <Date 2016-08-23.14:34:03.378>
    labels = ['performance']
    title = "functools.partial: don't copy keywoard arguments in partial_call()?"
    updated_at = <Date 2017-02-27.12:36:56.748>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-02-27.12:36:56.748>
    actor = 'berker.peksag'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-02-27.12:36:56.749>
    closer = 'berker.peksag'
    components = []
    creation = <Date 2016-08-23.14:34:03.378>
    creator = 'vstinner'
    dependencies = []
    files = ['44199']
    hgrepos = []
    issue_num = 27840
    keywords = ['patch']
    message_count = 6.0
    messages = ['273456', '273592', '273691', '273697', '288454', '288639']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'pitrou', 'vstinner', 'berker.peksag', 'serhiy.storchaka']
    pr_nums = ['253']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue27840'
    versions = ['Python 3.6']

    @vstinner
    Copy link
    Member Author

    I justed optimized partial_call() for positional arguments in the change c1a698edfa1b to avoid the creation of a temporary tuple when possible.

    I noticed that keyword parameters from partial() constructor are always copied. Is it mandatory? Can't we avoid copying them?

    Example:
    ---

    import functools
    hello = functools.partial(print, "Hello World", end='!\n')
    hello()

    hello keyword arguments are {'end'; '!\n'}.

    Attached patch avoids copying keyword arguments when the partial objects is not called with new keyword arguments.

    Tests pass, but I don't know if there is a risk that some strange function modify keyword arguments in-place?

    @vstinner vstinner added the performance Performance or resource usage label Aug 23, 2016
    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2016

    Modifying the keyword arguments dict is quite common so this change would be a heavy compatibility breaker.

    A well-known idiom:

      def some_function(..., **kwargs):
          some_option = kwargs.pop('some_option', None)
          # further process kwargs

    @rhettinger
    Copy link
    Contributor

    I concur with Antoine.

    @vstinner
    Copy link
    Member Author

    Ah yes sorry, I misunderstood the link between kwargs from a C function and
    a PY function. The link is simple, it's the same object :-) This issue can
    be closed.

    @vstinner
    Copy link
    Member Author

    I reopen the issue to propose to add a comment explaining why the dictionary must always be copied:
    #253

    @vstinner vstinner reopened this Feb 23, 2017
    @berkerpeksag
    Copy link
    Member

    PR 253 has been merged.

    @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
    performance Performance or resource usage
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants