classification
Title: Add a ContextManager ABC and type
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: 25637 26691 Superseder:
Assigned To: brett.cannon Nosy List: Nan Wu, brett.cannon, gvanrossum, jstasiak, martin.panter, ncoghlan, python-dev, rhettinger, supriyanto maftuh
Priority: normal Keywords: easy, patch

Created on 2015-11-12 19:14 by brett.cannon, last changed 2016-04-15 17:52 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
contextmanagertype.diff brett.cannon, 2016-03-18 22:49 review
contextmanagertype.diff brett.cannon, 2016-03-24 19:05 Address Nick's comments review
Messages (26)
msg254546 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-11-12 19:14
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.
msg254588 - (view) Author: Nan Wu (Nan Wu) * Date: 2015-11-13 04:44
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?
msg254696 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2015-11-15 21:11
-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).
msg254743 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-11-16 18:58
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.
msg254745 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-11-16 19:22
Depending on how issue #25637 turns out, the ABC could go into contextlib or a new interfaces modules.
msg257371 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-02 22:18
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.
msg257428 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-01-03 21:28
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.
msg257556 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-01-05 21:19
In issue #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.
msg262002 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-03-18 22:49
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.
msg262044 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-03-19 12:43
* 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
msg262056 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-03-19 17:39
Agreed on all points with Nick.  We can add ContextManager (not ContextManagerType) to typing.py in 3.5.2 (it's provisional).
msg262371 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-03-24 19:05
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.
msg262870 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-04 20:59
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?
msg262872 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-04 21:34
(Everything else looks great BTW.)
msg262873 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-04 22:02
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')
msg262874 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-04 22:26
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.
msg262876 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-04 22:50
Is there a tracking issue I can set as a dependency?
msg262877 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2016-04-04 22:57
No, but feel free to create one and assign it to me -- I will take
care of the rest then.
msg262880 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-04 23:28
Tracker issue created and assigned.
msg263037 - (view) Author: Roundup Robot (python-dev) Date: 2016-04-08 19:16
New changeset 841a263c0c56 by Brett Cannon in branch 'default':
Issue #25609: Introduce contextlib.AbstractContextManager and
https://hg.python.org/cpython/rev/841a263c0c56
msg263038 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-08 19:17
Thanks to everyone for the feedback! I will now go backport this to python/typing on GitHub.
msg263059 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-04-09 03:40
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.
msg263104 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-09 17:52
The typing changes need to be backported to 3.5 as-is to keep the files in sync.
msg263121 - (view) Author: Roundup Robot (python-dev) Date: 2016-04-10 02:44
New changeset 5c0e332988b2 by Martin Panter in branch 'default':
Issue #25609: Double back-ticks to avoid “make check” buildbot failure
https://hg.python.org/cpython/rev/5c0e332988b2
msg263152 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-04-10 21:20
Thanks for fixing the doc bug, Martin. Too much Markdown lately. :)
msg263511 - (view) Author: Roundup Robot (python-dev) Date: 2016-04-15 17:51
New changeset 257dd57dd732 by Brett Cannon in branch '3.5':
Issue #25609: Backport typing.ContextManager.
https://hg.python.org/cpython/rev/257dd57dd732

New changeset 14cf0011c5e9 by Brett Cannon in branch 'default':
Merge for issue #25609
https://hg.python.org/cpython/rev/14cf0011c5e9
History
Date User Action Args
2016-04-15 17:52:06brett.cannonsetstatus: open -> closed
2016-04-15 17:51:51python-devsetmessages: + msg263511
2016-04-12 14:59:14supriyantomaftuhsetnosy: + supriyanto maftuh
2016-04-10 21:20:19brett.cannonsetmessages: + msg263152
2016-04-10 02:44:27python-devsetmessages: + msg263121
2016-04-09 17:52:41brett.cannonsetstatus: closed -> open

messages: + msg263104
versions: + Python 3.5
2016-04-09 03:40:04martin.pantersetmessages: + msg263059
2016-04-08 19:17:15brett.cannonsetstatus: open -> closed
resolution: fixed
messages: + msg263038

stage: patch review -> resolved
2016-04-08 19:16:29python-devsetnosy: + python-dev
messages: + msg263037
2016-04-04 23:28:45brett.cannonsetmessages: + msg262880
2016-04-04 23:28:23brett.cannonsetdependencies: + Update the typing module to match what's in github.com/python/typing
2016-04-04 22:57:17gvanrossumsetmessages: + msg262877
2016-04-04 22:50:16brett.cannonsetmessages: + msg262876
2016-04-04 22:26:30gvanrossumsetmessages: + msg262874
2016-04-04 22:02:25brett.cannonsetmessages: + msg262873
2016-04-04 21:34:26gvanrossumsetmessages: + msg262872
2016-04-04 20:59:10gvanrossumsetmessages: + msg262870
2016-03-24 19:05:42brett.cannonsetfiles: + contextmanagertype.diff

messages: + msg262371
2016-03-19 17:39:02gvanrossumsetmessages: + msg262056
2016-03-19 12:43:37ncoghlansetmessages: + msg262044
2016-03-18 22:49:08brett.cannonsetfiles: + contextmanagertype.diff
keywords: + patch
messages: + msg262002

stage: needs patch -> patch review
2016-03-18 22:02:31jstasiaksetnosy: + jstasiak
2016-01-05 21:20:14brett.cannonsetassignee: brett.cannon
2016-01-05 21:19:53brett.cannonsetdependencies: + Move non-collections-related ABCs out of collections.abc
messages: + msg257556
2016-01-03 21:28:19gvanrossumsetmessages: + msg257428
2016-01-02 22:18:21martin.pantersetnosy: + martin.panter
messages: + msg257371
2015-12-29 03:46:51ncoghlansetnosy: + ncoghlan
2015-11-16 19:22:31brett.cannonsetmessages: + msg254745
2015-11-16 18:58:14brett.cannonsetmessages: + msg254743
2015-11-15 21:11:42rhettingersetmessages: + msg254696
2015-11-15 20:50:15rhettingersetnosy: + rhettinger
2015-11-13 04:44:52Nan Wusetnosy: + Nan Wu
messages: + msg254588
2015-11-12 19:14:39brett.cannoncreate