Issue 5251: contextlib.nested inconsistent with, well, nested with statements due exceptions raised in enter (original) (raw)

Created on 2009-02-13 18:09 by jwp, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
nested_issue.py jwp,2009-02-13 18:09 shows the variations of nested context managers and their results
pep377_v2.diff ncoghlan,2009-03-15 03:04 Second draft review
pep377_v3.diff ncoghlan,2009-03-15 12:29 Functionally complete (except compiler package) review
pep377_bench.py ncoghlan,2009-03-15 12:31 Micro benchmark for with statement speed
outer_sees_no_exc.py jwp,2009-03-16 02:34
Messages (20)
msg81957 - (view) Author: James William Pye (jwp) Date: 2009-02-13 18:09
Basically, nested() doesn't seem to be consistent with explicitly nested with-statements when an exception is thrown in a CM's __enter__. Consider a pair of nested CMs, the inner __enter__ raises an exception trapped by the outer. In the situation of explicitly nested with-statements, the inner's block will not be ran as an exception was raised prior to the block. The outer traps the exception and code continues to flow after the outer's block because the exception was *not* raised. Currently, if contextlib.nested() is used in such a situation, it blows up with a RuntimeError("generator didn't yield"). See the attached file for a set of naive variations and their resulting exceptions or lack thereof in the situation of explicitly nested CMs. Despite the RuntimeError raised by nested(), I'm not sure it's *currently* possible for an implementation of nested() to be *totally* consistent with explicitly nested with-statements. It would seem that an additional facility(AbortBlock exception?) would be needed to communicate that the block should not actually be ran, and that no exception should be raised as it was consumed by an "outer" CM. If this is considered to be the intended behavior, I would think the doc-string on contextlib.nested should be updated to document the inconsistency as it currently states that nested() and nested with statements are equivalent. Based on the results of the attached file in Python 3.0, they are clearly not. Cheers folks; lovin' CMs. And, of course, apologies if this has already been discussed. :P
msg82701 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-02-25 11:36
That's an interesting problem... since one of the things I like about CMs is the ability to turn pretty much any block of code into a context manager by substituting a yield statement at the appropriate point, not having the ability to skip the yield is actually something of a hole in the design. This will need a bit of python-dev discussion, and may even turn into a PEP (assuming we do go ahead with adding a SkipBlock exception and modifying the with statement to check for it being raised by __enter__() methods).
msg82886 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-02-27 23:56
I'll be working on a PEP for 2.7/3.1 to try to get the semantics of with statement changed as suggested. http://mail.python.org/pipermail/python-dev/2009-February/086470.html
msg83297 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-08 04:14
PEP 377 (http://www.python.org/dev/peps/pep-0377/) has now been submitted.
msg83304 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-08 05:44
Attached simple benchmark script to check for any slowdowns introduced by the planned with statement changes.
msg83588 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-14 15:08
First draft of patch attached. Limitations of this version of the patch: - includes some unrelated marshal.c changes (improved error messages) - no tests of the new functionality - contextlib not changed yet - no documentation changes The new semantics are in place though, and the existing test_with and test_contextlib tests all still pass.
msg83615 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 02:50
Removed first draft of patch - it was fundamentally flawed (it didn't clean up the stack properly when the statement body was skipped)
msg83616 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 03:04
Second draft attached, this time with tests and sans segmentation faults :) Limitations of this version of the patch: - still includes some unrelated marshal.c changes (improved error messages) - no documentation changes yet
msg83617 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 04:04
Note that the semantics in the current patch aren't quite correct, since the __exit__ attribute is retrieved inside the scope of the outer try/except block. Updated patch with the correct semantics coming soon.
msg83621 - (view) Author: James William Pye (jwp) Date: 2009-03-15 06:46
Just downloaded v2 and tried it out against Python 2.7a0 (trunk:70381M, Mar 14 2009, 23:12:51). output of the "nested_issue.py" script with patch: jwp@torch[]:org/python/trunk 0% /src/build/py/bin/python ./nested_issue.py () [try_with_nested] Skipping statement body () SUCCESS! i guess.. () [try_with_nested_class] Skipping statement body () SUCCESS! i guess.. () [try_with_nested_statements] () SUCCESS! i guess.. () [try_with_nested_statements_class] () SUCCESS! i guess.. I'm going to play with it a bit more, but it looks pretty solid already...
msg83623 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 07:20
To see what is wrong with v2, try something like: class Bizarre(): def __getattr__(self, attr): raise SkipStatement with Bizarre(): print "Not gonna happen, but no exception either!"
msg83626 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 12:29
Version 3 of patch attached - now functionally complete with the correct semantics - compiler package not updated yet - documentation not updated yet Unfortunately, getting the semantics right involves moving context.__enter__ and context.__exit__ to temporary variables instead of being able to leave them on the stack - that makes for something of a speed hit.
msg83627 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 12:31
Updated micro benchmark to also give numbers for direct try/finally execution
msg83628 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-15 12:36
I'm definitely not happy with the effect the current patch has on with statement execution speed - I think I'll try to come up with a patch that adds a new SETUP_WITH opcode and see how that performs.
msg83658 - (view) Author: James William Pye (jwp) Date: 2009-03-16 02:34
I tested the attached script against v2. It further identifies consistencies between nested with-statements and nested() that should exist(and do in v2). It answers the question: what is the effect of a SkipStatement exception on another, outer CM? with nested(outer(), trap(), fail()): ... That is, validate that outer()'s __exit__ invocation is not given an exception. This is probably an obvious effect of the patch, but I think it merited a test.
msg83924 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-03-21 08:58
Reclassifying as a documentation bug, since PEP 377 was rejected by Guido.
msg95324 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2009-11-16 07:00
Lowering priority due to implementation of explicit syntax for multiple context managers in a single with statement
msg119516 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2010-10-24 14:01
R.I.P contextlib.nested (it is gone in 3.2 following the deprecation in 3.1). The issue is obscure enough that I don't see much value in updating the documentation for the versions that still contain it in deprecated form.
msg120497 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2010-11-05 14:27
Reopening as a reminder to myself that I have a new PEP I want to write in this area. The idea is essentially a lighter weight alternative to PEP 377 that adds an optional __entered__ method to the context management protocol along the following lines: _v = cm.__enter__() try: if hasattr(cm, "__entered__"): VAL = cm.__entered__(_v) else: VAL = _v # do stuff finally: cm.__exit__(*exception_status) Providing a second, optional method that is executed *inside* the body will let CMs do things they can't do now (like skip the body of the with statement) without significantly affecting the behaviour of normal CMs. Notably, GeneratorContextManager will be able to use this to more gracefully handle the case where the generator doesn't yield a value. I plan to flesh this out into a python-ideas post (and likely a subsequent PEP) at some point in the next few months.
msg160139 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2012-05-07 11:40
Superseded by issue 13585 (which will add an improved dynamic context management API)
History
Date User Action Args
2022-04-11 14:56:45 admin set github: 49501
2012-05-07 11:40:43 ncoghlan set status: open -> closedsuperseder: Add contextlib.ExitStackresolution: postponed -> out of datemessages: +
2010-11-05 14:27:53 ncoghlan set status: closed -> openversions: + Python 3.3, - Python 3.1, Python 2.7, Python 3.2messages: + components: + Interpreter Core, - Documentationresolution: out of date -> postponed
2010-10-24 14:01:25 ncoghlan set status: open -> closedresolution: out of datemessages: +
2010-08-04 23:47:53 terry.reedy set versions: - Python 2.6, Python 2.5
2010-04-28 15:08:05 ncoghlan set versions: + Python 3.2, - Python 3.0
2009-11-16 07:00:26 ncoghlan set priority: lowmessages: +
2009-03-21 08:58:50 ncoghlan set messages: + components: + Documentation, - None
2009-03-16 02:34:35 jwp set files: + outer_sees_no_exc.pymessages: +
2009-03-15 12:57:03 ncoghlan set files: - pep377_bench.py
2009-03-15 12:36:57 ncoghlan set messages: +
2009-03-15 12:31:43 ncoghlan set files: + pep377_bench.pymessages: +
2009-03-15 12:29:54 ncoghlan set files: + pep377_v3.diffmessages: +
2009-03-15 07:20:21 ncoghlan set messages: +
2009-03-15 06:46:45 jwp set messages: +
2009-03-15 04:04:44 ncoghlan set messages: +
2009-03-15 03:04:54 ncoghlan set files: + pep377_v2.diffmessages: +
2009-03-15 02:50:31 ncoghlan set messages: +
2009-03-15 02:49:28 ncoghlan set files: - pep377_v1.diff
2009-03-14 15:08:15 ncoghlan set files: + pep377_v1.diffkeywords: + patchmessages: +
2009-03-08 05:44:40 ncoghlan set files: + pep377_bench.pymessages: +
2009-03-08 04:14:58 ncoghlan set messages: +
2009-02-27 23:56:42 ncoghlan set messages: +
2009-02-25 11:37:01 ncoghlan set messages: +
2009-02-17 01:57:24 benjamin.peterson set assignee: ncoghlannosy: + ncoghlan
2009-02-13 18:09:29 jwp create