(original) (raw)

On Oct 3, 2017 00:02, "Guido van Rossum" <guido@python.org> wrote:
On Mon, Oct 2, 2017 at 10:13 AM, Koos Zevenhoven <k7hoven@gmail.com> wrote:
Hi all, It was suggested that I start a new thread, because the other thread drifted away from its original topic. So here, in case someone is interested:

On Oct 2, 2017 17:03, "Koos Zevenhoven <k7hoven@gmail.com> wrote:
On Mon, Oct 2, 2017 at 6:42 AM, Guido van Rossum <guido@python.org> wrote:
On Sun, Oct 1, 2017 at 1:52 PM, Koos Zevenhoven <k7hoven@gmail.com> wrote:
On Oct 1, 2017 19:26, "Guido van Rossum" <guido@python.org> wrote:
Your PEP is currently incomplete. If you don't finish it, it is not even a contender. But TBH it's not my favorite anyway, so you could also just withdraw it.

I can withdraw it if you ask me to, but I don't want to withdraw it without any reason. I haven't changed my mind about the big picture. OTOH, PEP 521 is elegant and could be used to implement PEP 555, but 521 is almost certainly less performant and has some problems regarding context manager wrappers that use composition instead of inheritance.

It is my understanding that PEP 521 (which proposes to add optional \_\_suspend\_\_ and \_\_resume\_\_ methods to the context manager protocol, to be called whenever a frame is suspended or resumed inside a \`with\` block) is no longer a contender because it would be way too slow. I haven't read it recently or thought about it, so I don't know what the second issue you mention is about (though it's presumably about the \`yield\` in a context manager implemented using a generator decorated with \`@contextlib.contextmanager\`).


​Well, it's not completely unrelated to that. The problem I'm talking about is perhaps most easily seen from a simple context manager wrapper that uses composition instead of inheritance:

class Wrapper:
def \_\_init\_\_(self):
self.\_wrapped = SomeContextManager()

def \_\_enter\_\_(self):
print("Entering context")
return self.\_wrapped.\_\_enter\_\_()

def \_\_exit\_\_(self):
self.\_wrapped.\_\_exit\_\_()
print("Exited context")

Now, if the wrapped contextmanager becomes a PEP 521 one with \_\_suspend\_\_ and \_\_resume\_\_, the Wrapper class is broken, because it does not respect \_\_suspend\_\_ and \_\_resume\_\_. So actually this is a backwards compatiblity issue.

Why is it backwards incompatible? I'd think that without PEP 521 it would be broken in exactly the same way because there's no \_\_suspend\_\_/\_\_resume\_\_ at all.

The wrapper is (would be) broken because it depends on the internal implementation of the wrapped CM.

Maybe the author of SomeContextManager wants to upgrade the CM to also work in coroutines and generators. But it could be a more subtle change in the CM implementation.

The problem becomes more serious and more obvious if you don't know which context manager you are wrapping:

class Wrapper:
def \_\_init\_\_(self, contextmanager):
self.\_wrapped = contextmanager

def \_\_enter\_\_(self):
print("Entering context")
return self.\_wrapped.\_\_enter\_\_()

def \_\_exit\_\_(self):
self.\_wrapped.\_\_exit\_\_()
print("Exited context")


The wrapper is (would be) broken because it does not work for all CMs anymore.


But if the wrapper is made using inheritance, the problem goes away:


class Wrapper(SomeContextManager):
def \_\_enter\_\_(self):
print("Entering context")
return super().\_\_enter\_\_()
def \_\_exit\_\_(self):
super().\_\_exit\_\_()
print("Exited context")


Now the wrapper cleanly inherits the new optional \_\_suspend\_\_ and \_\_resume\_\_ from the wrapped context manager type.

In any case this is completely academic because PEP 521 is not going to happen. Nathaniel himself has said so (I think in the context of discussing PEP 550).

I don't mind this (or Nathaniel ;-) being academic. The backwards incompatibility issue I've just described applies to any extension via composition, if the underlying type/protocol grows new members (like the CM protocol would have gained \_\_suspend\_\_ and \_\_resume\_\_ in PEP521).


-- Koos (mobile)