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.

Sign up for GitHub

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

Jump to bottom

contextlib.ExitStack abuses __self__ #77446

Closed
jdemeyer opened this issue Apr 12, 2018 · 5 comments
Closed

contextlib.ExitStack abuses __self__ #77446

jdemeyer opened this issue Apr 12, 2018 · 5 comments
Labels
3.8 only security fixes performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@jdemeyer
Copy link
Contributor

BPO 33265
Nosy @ncoghlan, @jdemeyer, @1st1
PRs
  • bpo-33265: use an actual method instead of a method-like function in ExitStack #6456
    		•

    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 2018-04-13.12:25:09.795>
created_at = <Date 2018-04-12.05:24:50.947>
labels = ['3.8', 'library', 'performance']
title = 'contextlib.ExitStack abuses __self__'
updated_at = <Date 2018-04-13.12:25:09.793>
user = 'https://github.com/jdemeyer'

    bugs.python.org fields:

    activity = <Date 2018-04-13.12:25:09.793>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2018-04-13.12:25:09.795>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2018-04-12.05:24:50.947>
creator = 'jdemeyer'
dependencies = []
files = []
hgrepos = []
issue_num = 33265
keywords = ['patch']
message_count = 5.0
messages = ['315212', '315221', '315239', '315243', '315244']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'jdemeyer', 'yselivanov']
pr_nums = ['6456']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue33265'
versions = ['Python 3.8']
    @jdemeyer
    Copy link
    Contributor Author

    In contextlib, there is code which roughly looks like

            def _exit_wrapper(exc_type, exc, tb):
            return cm_exit(cm, exc_type, exc, tb)
        _exit_wrapper.__self__ = cm

    This creates a new function _exit_wrapper from a given function cm_exit by prepending the __self__ attribute to *args. Now this is exactly what a method does too.

    It would be better to use an actual method for this: it's cleaner, faster and it doesn't abuse a double-underscore attribute. The latter will actually break with PEP-575, as __self__ will become a special name instead of an arbitrary attribute.

    @jdemeyer jdemeyer added the stdlib Python modules in the Lib dir label Apr 12, 2018
    @ncoghlan
    Copy link
    Contributor

    Yury, could you double check the async exit stack change in the PR? I think it's fine since the bound method just passes back the underlying coroutine and the tests all still pass, but a second opinion would be good :)

    @1st1
    Copy link
    Member

    1st1 commented Apr 13, 2018

    Yep, I think this is a good fix!

    @ncoghlan
    Copy link
    Contributor

    New changeset 23ab5ee by Nick Coghlan (jdemeyer) in branch 'master':
    bpo-33265: use an actual method instead of a method-like function in ExitStack (GH-6456)
    23ab5ee

    @ncoghlan
    Copy link
    Contributor

    Classifying this as a minor performance enhancement, since methods are much simpler objects than full Python level closures.

    Thanks!

    @ncoghlan ncoghlan added the 3.8 only security fixes label Apr 13, 2018
    @ncoghlan ncoghlan added the performance Performance or resource usage label Apr 13, 2018
    @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
    Assignees
    No one assigned
    Labels
    3.8 only security fixes performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @1st1 @ncoghlan @jdemeyer