Is returning a value other than `self` in `__enter__` an anti-pattern?

user_3062183 0
Viewed 1169

Following this related question, while there are always examples of some library using a language feature in a unique way, I was wondering whether returning a value other than self in an __enter__ method should be considered an anti-pattern.

The main reason why this seems to me like a bad idea is that it makes wrapping context managers problematic. For example, in Java (also possible in C#), one can wrap an AutoCloseable class in another class which will take care of cleaning up after the inner class, like in the following code snippet:

try (BufferedReader reader = 
     new BufferedReader(new FileReader("src/main/resources/input.txt"))) {
  return readAllLines(reader);
}

Here, BufferedReader wraps FileReader, and calls FileReader's close() method inside its own close() method. However, if this was Python, and FileReader would've returned an object other than self in its __enter__ method, this would make such an arrangement significantly more complicated. The following issues would have to be addressed by the writer of BufferedReader:

  1. When I need to use FileReader for my own methods, do I use FileReader directly or the object returned by its __enter__ method? What methods are even supported by the returned object?
  2. In my __exit__ method, do I need to close only the FileReader object, or the object returned in the __enter__ method?
  3. What happens if __enter__ actually returns a different object on its call? Do I now need to keep a collection of all of the different objects returned by it in case someone calls __enter__ several times on me? How do I know which one to use when I need to use on of these objects?

And the list goes on. One semi-successful solution to all of these problems would be to simply avoid having one context manager class clean up after another context manager class. In my example, that would mean that we would need two nested with blocks - one for the FileReader, and one for the BufferedReader. However, this makes us write more boilerplate code, and seems significantly less elegant.

All in all, these issues lead me to believe that while Python does allow us to return something other than self in the __enter__ method, this behavior should simply be avoided. Is there some official or semi-official remarks about these issues? How should a responsible Python developer write code that addresses these issues?

python
1 Answers

TLDR: Returning something other than self from __enter__ is perfectly fine and not bad practice.

The introducing PEP 343 and Context Manager specification expressly list this as desired use cases.

An example of a context manager that returns a related object is the one returned by decimal.localcontext(). These managers set the active decimal context to a copy of the original decimal context and then return the copy. This allows changes to be made to the current decimal context in the body of the with statement without affecting code outside the with statement.

The standard library has several examples of returning something other than self from __enter__. Notably, much of contextlib matches this pattern.

The context manager protocol makes it clear what is the context manager, and who is responsible for cleanup. Most importantly, the return value of __enter__ is inconsequential for the protocol.

A rough paraphrasing of the protocol is this: When something runs cm.__enter__, it is responsible for running cm.__exit__. Notably, whatever code does that has access to cm (the context manager itself); the result of cm.__enter__ is not needed to call cm.__exit__.

In other words, a code that takes (and runs) a ContextManager must run it completely. Any other code does not have to care whether its value comes from a ContextManager or not.

# entering a context manager requires closing it…
def managing(cm: ContextManager):
    value = cm.__enter__()  # must clean up `cm` after this point
    try:
        yield from unmanaged(value)
    except BaseException as exc:
        if not cm.__exit__(type(exc), exc, exc.__traceback__):
           raise
    else:
        cm.__exit__(None, None, None)

# …other code does not need to know where its values come from
def unmanaged(smth: Any):
    yield smth

When context managers wrap others, the same rules apply: If the outer context manager calls the inner one's __enter__, it must call its __exit__ as well. If the outer context manager already has the entered inner context manager, it is not responsible for cleanup.

In some cases it is in fact bad practice to return self from __enter__. Returning self from __enter__ should only be done if self is fully initialised beforehand; if __enter__ runs any initialisation code, a separate object should be returned.

class BadContextManager:
      """
      Anti Pattern: Context manager is in inconsistent state before ``__enter__``
      """
      def __init__(self, path):
          self.path = path
          self._file = None  # BAD: initialisation not complete

      def read(self, n: int):
          return self._file.read(n)  # fails before the context is entered!

      def __enter__(self) -> 'BadContextManager':
          self._file = open(self.path)
          return self  # BAD: self was not valid before

      def __exit__(self, exc_type, exc_val, tb):
          self._file.close()

class GoodContext:
      def __init__(self, path):
          self.path = path
          self._file = None  # GOOD: Inconsistent state not visible/used

      def __enter__(self) -> TextIO:
          if self._file is not None:
             raise RuntimeError(f'{self.__class__.__name__} is not re-entrant')
          self._file = open(self.path)
          return self._file  # GOOD: value was not accessible before

      def __exit__(self, exc_type, exc_val, tb):
          self._file.close()

Notably, even though GoodContext returns a different object, it is still responsible to clean up. Another context manager wrapping GoodContext does not need to close the return value, it just has to call GoodContext.__exit__.

user_53499160
Related