contextlib.ExitStack abuses __self__

[jdemeyer]

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]

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.

[ncoghlan]

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]

Yep, I think this is a good fix!

[ncoghlan]

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]

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

Thanks!