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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2009-03-15 12:31 |
Updated micro benchmark to also give numbers for direct try/finally execution |
|
|
msg83628 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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) *  |
Date: 2009-03-21 08:58 |
Reclassifying as a documentation bug, since PEP 377 was rejected by Guido. |
|
|
msg95324 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
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) *  |
Date: 2012-05-07 11:40 |
Superseded by issue 13585 (which will add an improved dynamic context management API) |
|
|