python / python/cpython

ContextDecorator documentation is unclear.

Open
#134,537 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

docs stdlib
Dominant language
Python
Stars
77.2k
Forks
35.9k
PR merge metrics
PR metrics pending

Description

Bug report

Bug description:
Overview:

ContextDecorator can not be safely used on functions that make recursive calls, or may be used with multithreading, even if those context managers support use in sequential with statements. The documentation suggests otherwise, and could be clarified. Additionally, this functionality could be added via a class method.

Detailed description

The documentation for ContextDecorator states:

This change is just syntactic sugar for any construct of the following form:

def f():
    with cm():
        # Do stuff

ContextDecorator lets you instead write:

@cm()
def f():
    # Do stuff

However, ContextDecorator is closer in functionality to the following:

cm = cm()
def f():
    with cm:
        # Do stuff

The documentation does contain the following warning. However, the wording could be more clear, especially given the syntactic sugar example.

Note As the decorated function must be able to be called multiple times, the underlying context manager must support use in multiple with statements. If this is not the case, then the original construct with the explicit with statement inside the function should be used.

Repro Code

This code demonstrates the issue:

import contextlib
import time
class timed(contextlib.ContextDecorator):
  def __enter__(self):
    self._start_time = time.monotonic()

  def __exit__(self, *exc):
    print(f"Execution took  {time.monotonic() - self._start_time:.0f} seconds")

@timed()
def my_func(recurse_once = False):
  time.sleep(1)
  if recurse_once:
    my_func()

my_func(recurse_once = True)

Expected output:

Execution took 1 seconds
Execution took 2 seconds

Actual output:

Execution took 1 seconds
Execution took 1 seconds

Potential fixes

Implementing one or more of these fixes could alleviate the issue.

1. Clarify the syntactic sugar section to show that all function invocations share a single instance of CM.

The "syntactic sugar" section could be changed to read:

ContextDecorator lets you instead write:

@cm()
def f():
    # Do stuff

Which is equivalent to:

cm = cm()
def f():
    with cm:
        # Do stuff
2. Clarify the warning note

The warning note could be changed to make it more clear that separate functions share state in the context manager.

Note The underlying context manager is instantiated once when the function definition is evaluated: this instance is shared between all calls to the function. As the decorated function must be able to be called multiple times, the underlying context manager must support use in multiple with statements. If this is not the case, then the original construct with the explicit with statement inside the function should be used.

3. Provide a method for wrapping functions with stateful context managers

For example:

def wrap_with_context(cm_factory, *cm_args, **cm_kwargs):
  """Wraps the decorated function with a context created by cm_factory."""

  def wrap(func):
    @functools.wraps(func)
    def inner(*func_args, **func_kwargs):
      with cm_factory(*cm_args, **cm_kwargs):
        return func(*func_args, **func_kwargs)
    return inner
  return wrap

The previous example now works as expected:

@wrap_with_context(timed)
def my_func2(recurse_once = False):
  time.sleep(1)
  if recurse_once:
    my_func2()

my_func2(recurse_once = True)

Actual output:

Execution took 1 seconds
Execution took 2 seconds

4. Add class method to ContextDecorator that acts as a decorator and a factory.
class ContextDecorator:
...
  @classmethod
  def wrap(cls, *cls_args, **cls_kwargs):
    def enclose(func):
      @functools.wraps(func)
      def inner(*args, **kwds):
        with cls(*cls_args, **cls_kwargs):
          return func(*args, **kwds)
      return inner
    return enclose

This allows usage that is similar to existing, but instantiates a separate CM for each function invocation:

@timed.wrap()
def my_func2(recurse_once = False):
  time.sleep(1)
  if recurse_once:
    my_func2()
CPython versions tested on:

3.13

Operating systems tested on:

macOS

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the ContextDecorator section in the linked Python documentation, especially the syntactic-sugar example and the warning note. Clarify that one underlying context-manager instance is shared across function calls and explain when an explicit with statement is needed; the revised documentation should accurately describe recursive and concurrent use.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
documentation
Issue type
Documentation
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.