Add contextlib.ExitStack

[nikratio]

BPO	13585
Nosy	@rhettinger, @ncoghlan, @pitrou, @giampaolo, @merwok, @meadori, @Julian, @ericsnowcurrently, @smarnach, @jdemeyer
Files	CleanupManager_patch_v1.diff CleanupManager_patch_v2.diff

^{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/ncoghlan'
closed_at = <Date 2012-05-21.12:54:58.701>
created_at = <Date 2011-12-12.01:49:50.244>
labels = ['type-feature', 'library']
title = 'Add contextlib.ExitStack'
updated_at = <Date 2018-04-12.05:25:34.535>
user = 'https://bugs.python.org/nikratio'

bugs.python.org fields:

activity = <Date 2018-04-12.05:25:34.535>
actor = 'jdemeyer'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2012-05-21.12:54:58.701>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2011-12-12.01:49:50.244>
creator = 'nikratio'
dependencies = []
files = ['23923', '23933']
hgrepos = []
issue_num = 13585
keywords = ['patch']
message_count = 34.0
messages = ['149268', '149269', '149271', '149272', '149273', '149303', '149352', '149353', '149354', '149366', '149372', '149373', '149376', '149377', '149378', '149380', '149384', '149393', '150011', '150012', '150017', '150028', '150057', '150063', '150065', '150071', '150080', '150090', '150092', '157386', '159754', '161271', '315201', '315213']
nosy_count = 12.0
nosy_names = ['rhettinger', 'ncoghlan', 'pitrou', 'giampaolo.rodola', 'eric.araujo', 'nikratio', 'meador.inge', 'Julian', 'python-dev', 'eric.snow', 'smarnach', 'jdemeyer']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue13585'
versions = ['Python 3.3', 'Python 3.4']

[nikratio]

I'd like to propose addding the CleanupManager class described in http://article.gmane.org/gmane.comp.python.ideas/12447 to the contextlib module. The idea is to add a general-purpose context manager to manage (python or non-python) resources that don't come with their own context manager.

Example code:

with CleanupManager() als mngr:
    tmpdir = tempfile.mkdtemp()
    mngr.register(shutil.rmtree(tmpdir))
    # do stuff with tmpdir

# shutil.rmtree() get's called automatically when the block is over

Note that mkdtemp() could of course also be changed to become its own context manager. The idea is to provide a general facility for this kind of problem, so it doesn't have to be reinvented whenever a module provides a ressource without its own context manager. Other possible uses are of course ressources that are completely external to Python,
e.g. anything allocated with a subprocess (think of subprocess.check_call('mount'))/

I'll be happy to make a proper patch with documentation and testcases from Jan's code. As a matter of fact, I'll probably start working out it right now, so please let me know quickly if this doesn't have a chance of getting accepted.

[nikratio]

Here's the first part of the patch with the implementation. I'll add tests and documentation as soon as someone confirms that the idea & API is okay.

[rhettinger]

I would like to see this posted as a recipe before being put in the standard library. It needs a chance to mature and to demonstrate that people will want to use it.

FWIW, the example is problematic in a couple of ways. The registration of shutil.rmtree(tmpdir) will run *before* mngr register is called.

Also, it doesn't take advantage of any of the with-statement features. It doesn't show any advantage over a standard try/finally which is arguably cleaner:

    tmpdir = tempfile.mkdtemp()
    try:
        # do stuff with tmpdir
    finally:
        shutil.rmtree()

Also, I suspect that the CleanupManager would be an error-prone construct because the registration occurs somewhere after the with-statement is set-up, possibly resulting in errors if there is an early, pre-registration failure.

[nikratio]

Not sure what you mean with "posted as a recipe" -- are you thinking of a specific website/mailing list?

Example: which one do you mean? The one in the issue or the one in the patch?

With statement: what advantages do you have in mind?

Try/finally: I think the patch and the discussion in python-ideas talk about the advantage over try/finally. IMO the two most important points are: (1) avoids deep and pointless indendation for multiple ressources, (2) keeps logically connected lines (allocation+cleanup) closely together in the source instead of splitting them far apart like try/finally.

error-prone: not sure if I understand you correctly. If there is an error prior to registration, the callback will not be called (that's a feature). To what kind of errors could that lead?

Sorry for basically asking you to re-explain every sentence, but I honestly don't understand most of your message.

[rhettinger]

'''
Example code:

with CleanupManager() als mngr:
    tmpdir = tempfile.mkdtemp()
    mngr.register(shutil.rmtree(tmpdir)) <-- this makes the call right away
    # do stuff with tmpdir
'''

The part of my note that should be clear is that the idea and code need to prove itself before being added to the standard library. So far, there has been zero demand for this and I've not seen code like it being used in the wild. AFAICT, it is not demonstrably better than a straight-forward try/finally.

[nikratio]

On 12/11/2011 10:17 PM, Raymond Hettinger wrote:

Raymond Hettinger <raymond.hettinger@gmail.com> added the comment:

'''
Example code:

with CleanupManager() als mngr:
tmpdir = tempfile.mkdtemp()
mngr.register(shutil.rmtree(tmpdir)) <-- this makes the call right away
# do stuff with tmpdir
'''

Oh, of course. That is fixed in the patch. I couldn't find a way to edit
the message in the tracker.

The part of my note that should be clear is that the idea and code need to prove itself before being added to the standard library. So far, there has been zero demand for this and I've not seen code like it being used in the wild. AFAICT, it is not demonstrably better than a straight-forward try/finally.

I think it has the same advantages over try...finally as any use of
'with' has. CleanupManager would allow to use 'with' instead of
'try..finally' in many cases where this is currently not possible, so
unless the introduction of 'with' itself was a mistake, I think this is
just taking a step along the path that has already been chosen.

Best,

-Nikolaus

[smarnach]

There is actually a second thread on python-ideas on a very similar topic, see

http://mail.python.org/pipermail/python-ideas/2011-December/013021.html

The two main advantages of the proposed CleanupManager over try/finally blocks are

1. You can add a clean-up function conditionally. In a try/finally block, you would need a flag, which would scatter the single idea more across the code. Example:

    with CleanupManager() als mngr:
        if f is None:
            f = open("some_file")
            mngr.register(f.close)
        # do something with f (possibly many lines of code)

seems much clearer to me than

    f_needs_closing = False
    if f is None:
        f = open("some_file")
        f_needs_closing = True
    try:
        # do something with f (possibly many lines of code)
    finally:
        if f_needs_closing:
            f.close()

The first version is also much more in the spirit of context managers. You state at the beginning "we open the file, and guarantee that it will be closed", and we know right from the start that we don't have to bother with it again.

2. CleanupManager could replace several nested try/finally blocks, which again might lead to more readable code.

On the other hand, people who never saw ContextManager before will have to look it up, which will impair readability for those people.

Adding this as a cookbook recipe first seems like a good idea.

[nikratio]

On 12/12/2011 03:31 PM, Sven Marnach wrote:

Adding this as a cookbook recipe first seems like a good idea.

This is the second time that this is mentioned, so I would be very
grateful if someone could elucidate what "adding as a cookbook recipe"
means :-). Is there an official Python cookbook somewhere?

Best,

-Nikolaus

[ericsnowcurrently]

Check out: http://code.activestate.com/recipes/

[Julian]

For reference, the implementation that I posted in the other thread is:

    @contextlib.contextmanager
    def maybe(got, contextfactory, *args, checkif=bool, **kwargs):
        if checkif(got):
            yield got
        else:
            with contextfactory(*args, **kwargs) as got:
                yield got

which produces code like:

def fn(input_file=None):
    with maybe(input_file, open, "/default/input/file/location/"):
        for line in input_file:
            print line

For the example given here in the OP, since rmtree isn't a contextmanager it'd require wrapping it in one first.

I could probably understand if there was desire for these to be a recipe instead. The advantage (if any) of the above over this class is that it keeps context managers sorta looking like context managers. Here if you wanted to write my example it'd require writing code like

with ContextManager() as mgr:
    foo = ctxtmgr()
    mgr.register(foo.__close__)

which doesn't look too great :/

The class though does have wider scope though, since it doesn't necessarily only need a context manager, it can do cleanup for anything, which I guess is nice.

[ncoghlan]

Given the existence of tempfile.TemporaryDirectory in recent Python versions, I suggest finding a new cleanup function example that doesn't duplicate native stdlib functionality :)

I do see value in the feature itself though - I believe the precedent of both the atexit module [1] and unittest.TestCase.addCleanup() [2] shows how it can be useful to have a standard way to accumulate a sequence of "undo" operations for invocation at a later time.

In particular, it's a much better fit than nested with statements are for *optional* resources - you can make the with statement unconditional (setting up the cleanup manager), optionally add the cleanup methods, and avoid needing to have two copies of your actual invocation code (one inside a with statement and one without) or having a delayed check in a finally block.

It codifies the fairly common "if this resource was acquired, make sure it is released" idiom in a way that with statements and try/finally just don't handle neatly. By using an incremental API, it also avoids the traps associated with the ultimately misguided "contextlib.nested()" design.

However, I suggest using an API that strictly follows the "register only" model employed by both of the existing mechanisms. In addition, any solution provided as part of contextlib should interoperate nicely with existing context managers - it should be trivial to rewrite a nested with statement to be based on CleanupManager instead.

Accordingly, I would give the manager the following public methods:

register_exit() (only accepts callbacks with the __exit__ signature)
register() (equivalent to TestCase.addCleanup)
enter_context() (accepts actual context managers)
close() (equivalent to TestCase.doCleanups)

register_exit() would be the base callback registration method. It would accept only callbacks with the same signature as __exit__ methods.

    def register_exit(self, exit):
        self._callbacks.append(exit)
        return exit # Allow use as a decorator

register() would wrap arbitrary callbacks to support the __exit__ method signature:

    def register(self, _cb, *args, **kwds):
        def _wrapper(exc_type, exc, tb):
            return _cb(*args, **kwds)
        return self.register_exit(_wrapper)

enter_context() would work as follows:

    def enter_context(self, cm):
        result = cm.__enter__()
        self.register_exit(cm.__exit__)
        return result

close() would look like:

    def close(self):
        self.__exit__(None, None, None)

And finally, __exit__() itself would be:

    def __exit__(self, *exc_details):
        def _invoke_next_callback(exc_details):
            # Callbacks are removed from the list in FIFO order
            # but are actually *invoked* in LIFO order
            cb = self._callbacks.pop(0)
            if not self._callbacks:
                # Innermost callback is invoked directly
                return cb(exc_type, exc, tb)
            # Use try-finally to ensure this callback still gets
            # invoked even if an inner one fails
            try:
                inner_result = _invoke_next_callback()
            except:
                cb_result = cb(*sys.exc_info())
                # Check if this cb suppressed the inner exception
                if not cb_result:
                    raise
            else:
                # Check if inner cb suppressed the original exception
                if inner_result:
                    exc_details = (None, None, None)
                cb_result = cb(*exc_details)
            return cb_result
        _invoke_next_callback(exc_details)

An example using a cleanup manager to handle multiple files, one of which is optional:

    with contextlib.CleanupManager() as cm:
        source = cm.enter_context(open(source_fname))
        if dest_fname is not None:
            dest = cm.enter_context(open(dest_fname))
            _write_to_dest = dest.write
        else:
            def _write_to_dest(line): pass
        for line in source:
            _write_to_dest(line)
            yield line

[1] http://docs.python.org/library/atexit
[2] http://docs.python.org/library/unittest#unittest.TestCase.addCleanup

[rhettinger]

I think you guys need to post your code somewhere (perhaps on PyPi or the ASPN Cookbook). It needs to mature beyond the stage of "I just whipped-up this code and think it would be great if everybody used it".

I've seen nothing like this being used in production code (code published on the net or at one of my clients). Context managers have been around for a while, so if this were a real need, we would expect to see people already using something like this (for example, namedtuples got introduced to the standard library upon seeing many, many reinventions of the concept and we were able to consolidate the best features from each).

Design of the a feature in the standard library should be driven by examples of real world code that would be improved with the new feature. The design should also be informed by the experience of teaching people how to use it and seeing what they do (lots of technically correct ideas get shot down because it turns out that incorrect usage is common (a bug factory like the % formatting operator) or that people have a hard time learning and remembering the feature.

The core problem is that it is easier to add things to the standard library than to take them out if they prove to be a bad idea. Accordingly, we need to be *really sure* that this is a good idea, that it will *improve* real world code, that people learn, understand, and remember it easily, and that is doesn't impair readability.

The example code from Nikolaus has a bug in it -- that is worrisome and may suggest that we are better-off without this being in the standard library.