Add a ContextManager ABC and type

[brettcannon]

BPO	25609
Nosy	@gvanrossum, @brettcannon, @rhettinger, @ncoghlan, @vadmium, @jstasiak
Dependencies	bpo-25637: Move non-collections-related ABCs out of collections.abc bpo-26691: Update the typing module to match what's in github.com/python/typing
Files	contextmanagertype.diff contextmanagertype.diff: Address Nick's comments

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/brettcannon'
closed_at = <Date 2016-04-15.17:52:06.015>
created_at = <Date 2015-11-12.19:14:39.600>
labels = ['easy', 'library']
title = 'Add a ContextManager ABC and type'
updated_at = <Date 2016-04-15.17:52:06.014>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2016-04-15.17:52:06.014>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2016-04-15.17:52:06.015>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2015-11-12.19:14:39.600>
creator = 'brett.cannon'
dependencies = ['25637', '26691']
files = ['42209', '42279']
hgrepos = []
issue_num = 25609
keywords = ['patch', 'easy']
message_count = 26.0
messages = ['254546', '254588', '254696', '254743', '254745', '257371', '257428', '257556', '262002', '262044', '262056', '262371', '262870', '262872', '262873', '262874', '262876', '262877', '262880', '263037', '263038', '263059', '263104', '263121', '263152', '263511']
nosy_count = 9.0
nosy_names = ['gvanrossum', 'brett.cannon', 'rhettinger', 'ncoghlan', 'python-dev', 'martin.panter', 'jstasiak', 'Nan Wu', 'supriyanto maftuh']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25609'
versions = ['Python 3.5', 'Python 3.6']

[brettcannon]

There should probably be a context manager ABC in collections.abc that requires enter/exit and a matching entry in typing that also takes a type argument of what the enter method returns.

[NanWu]

Hi Brett, I'd like work on this feature. Your description here is clear. Besides that, could you give a use case of this context manager?

[rhettinger]

-0 on this one. This is slightly more useful than the other "one-trick-ponies" like Container, Hashable, and Iterable. Those each provide some "recognition" capabilities using isinstance() but don't provide any useful "mixin" capabilities.

In practice, I'm not seeing the "recognition" capabilities being used (i.e. no one is using "if isinstance(obj, Container)" in real code). So adding another one-trick pony that won't get used is a bit of a waste.

Also, this would add more clutter to the collections ABCs, obscuring the ones that actually have added value (i.e. MutableMapping has been a great success). The collections.abc module has become a dumping ground for ABCs that that don't really have anything to do with collections and aren't useful for day-to-day development (Awaitable, Coroutine, AsyncIterable, AsyncIterator, etc).

One idea is to have a separate module of generic recognition abcs that would be used in much the same way as the old types module. Otherwise, the collections.abc module will continue unabated growth away from its original purpose as a small group of powerful mixin classes (Sequence, MutableSequence, Set, MutableSet, Mapping, MutableMapping, and the dict views).

[brettcannon]

The reason I suggested the ABC for context managers is mostly to provide a way to help people make sure to implement enter() and to provide a default exit() which has the proper signature (I personally always forget how many arguments exit() takes and what the arguments are past the first one). The type part of this feature request is because I realized that a type hint of "context manager" isn't really useful, but "context manager returning ..." is since a with statement does introduce a new variable. IOW none of this has anything to do with isinstance() and it's all about easing the use of the context manager interface and proper type hints for the with statement.

As for a new module analogous to the types module just for ABCs, that's fine by me. I had the same reaction you did, Raymond, about putting it in collections.abc. I can open another issue for that idea and leave this open as it's somewhat orthogonal to what I'm proposing.

[brettcannon]

Depending on how issue bpo-25637 turns out, the ABC could go into contextlib or a new interfaces modules.

[vadmium]

What would your context manager base class do? I presume you supply a default __enter__() that does nothing, or perhaps just returns self.

You mentioned having a default __exit__() but I am having trouble seeing how it would be useful. A context manager is only really useful if it does something interesting in __exit__(), and for that, the programmer has to write their own version and remember its signature.

One option to simplify __exit__() that I used to use is a base class that defers to an abstract close() method with no parameters. But since ExitStack is now available, I am finding that is good enough instead.

[gvanrossum]

The abstract base class would be an ABC providing documentation and a template for implementations, and a way to test using isinstance(). This is how several other ABCs are used that have no particularly useful default implementation.

In addition, the presence of the ABC provides a reference for corresponding class in the typing module (used to express type hints using PEP-484) -- again this is just following the example of the other ABCs.

[brettcannon]

In issue bpo-25637, Guido said he didn't want to move stuff out of collections.abc, so the context manager ABC will go into contextlib.

To answer Martin's point, I could make __exit__ abstract to begin with to make people override it properly. The only reason I thought of providing a default implementation is that it inherently isn't required to be implemented to match the context manager interface/protocol. But as you pointed out, the usefulness of a context manager is derived from doing stuff in __exit__(), so I will make it abstract.

[brettcannon]

Here is an initial patch to add AbstractContextManager and ContextManagerType to contextlib. There are no tests yet as I want to make sure this looks okay first.

[ncoghlan]

• the ABC should have a structural __issubclass__ check looking for __enter__ and __exit__ methods

• I agree with making __exit__ abstract (if you don't define it, you don't need a context manager), but does __enter__ need to be abstract? The "return self" default is a good one in many cases where the resource is acquired in __new__ or __init__.

• the docs for the ABC are potentially confusing, as they describe what the default implementation does, without making it clear that it's only the default implementation and *permitted* return behaviour is more flexible than that.

• I'd prefer not to have contextlib depend on typing, so the generic type definition should probably be in the typing module (similar to the abc-vs-type split for collections). That should also make backporting easier - the ABC can go back via contextlib2, while the generic type can go back via the comment-based typehinting variant

[gvanrossum]

Agreed on all points with Nick. We can add ContextManager (not ContextManagerType) to typing.py in 3.5.2 (it's provisional).

[brettcannon]

Here is an updated patch implementing Nick's suggestions and is ready for 3.6 sans a What's New entry.

As for Python 3.5, I think I will copy __subclasshook__() from contextlib.AbstractContextManager and put it in typing.ContextManager so the isinstance() checks will work with the type structurally.

LMK if this all sounds/looks reasonable.

[gvanrossum]

A slight problem is that I'd really like to keep the source of typing.py identical in Python 3.5 and 3.6. So maybe the definition should be conditional on the presence of AbstractContextManager?

[gvanrossum]

(Everything else looks great BTW.)

[brettcannon]

When I have a chance I'll do up a new patch where the definition of typing.ContextManager is guarded, add a What's New entry, and commit it. My planned guard will be:

if hasattr(contextlib, 'AbstractContextManager'):
    class ContextManager(...): ...
    __all__.append('ContextManager')

[gvanrossum]

I have some significant changes to typing.py (and test_typing.py)
upstream in https://github.com/python/typing/. We should coordinate
our changes to stdlib (test_)typing.py.

[brettcannon]

Is there a tracking issue I can set as a dependency?

[gvanrossum]

No, but feel free to create one and assign it to me -- I will take
care of the rest then.

[brettcannon]

Tracker issue created and assigned.

[python-dev]

New changeset 841a263c0c56 by Brett Cannon in branch 'default':
Issue bpo-25609: Introduce contextlib.AbstractContextManager and
https://hg.python.org/cpython/rev/841a263c0c56

[brettcannon]

Thanks to everyone for the feedback! I will now go backport this to python/typing on GitHub.

[vadmium]

The docs buildbot is complaining:

http://buildbot.python.org/all/builders/Docs%203.x/builds/1156/steps/lint/logs/stdio
[2] whatsnew/3.6.rst:199: default role used

I guess this is complaining about the `__enter__()` syntax with back-ticks. Maybe it should be either :meth:`__enter__` or ``__enter__()``, etc.

[brettcannon]

The typing changes need to be backported to 3.5 as-is to keep the files in sync.

[python-dev]

New changeset 5c0e332988b2 by Martin Panter in branch 'default':
Issue bpo-25609: Double back-ticks to avoid “make check” buildbot failure
https://hg.python.org/cpython/rev/5c0e332988b2

[brettcannon]

Thanks for fixing the doc bug, Martin. Too much Markdown lately. :)

[python-dev]

New changeset 257dd57dd732 by Brett Cannon in branch '3.5':
Issue bpo-25609: Backport typing.ContextManager.
https://hg.python.org/cpython/rev/257dd57dd732

New changeset 14cf0011c5e9 by Brett Cannon in branch 'default':
Merge for issue bpo-25609
https://hg.python.org/cpython/rev/14cf0011c5e9