add typing.ContextManager on all supported Python versions #422
Conversation
So that there is a way to type hint for a context manager even on language versions before 3.6. This is just copied from contextlib.AbstractContextManager in 3.6. Fixes python#274
src/typing.py
Outdated
| def __subclasshook__(cls, C): | ||
| if cls is ContextManager: | ||
| if (any("__enter__" in B.__dict__ for B in C.__mro__) and | ||
| any("__exit__" in B.__dict__ for B in C.__mro__)): |
There was a problem hiding this comment.
I understand this is just copied from contextlib, but I think both should be updated to support __enter__ = None pattern, see my other comment.
There was a problem hiding this comment.
I don't think that makes sense here, because this class will only be used in Python < 3.6, and the = None pattern became generally supported only in 3.6. The collections ABCs don't support anti-registration either before 3.6.
There was a problem hiding this comment.
I don't think that makes sense here, because this class will only be used in Python < 3.6, and the
= Nonepattern became generally supported only in 3.6. The collections ABCs don't support anti-registration either before 3.6.
OK, if @gvanrossum agrees with this, then I think this can be merged as is, I don't have any other comments.
There was a problem hiding this comment.
OK, but this is worth a comment in the code.
python2/typing.py
Outdated
| def __subclasshook__(cls, C): | ||
| if cls is ContextManager: | ||
| if (any("__enter__" in B.__dict__ for B in C.__mro__) and | ||
| any("__exit__" in B.__dict__ for B in C.__mro__)): |
There was a problem hiding this comment.
This is a cringe-worthy indentation (due to a stupid flake rule), since the two any() calls are actually siblings. I propose to stop contorting our code and instead disable that specific flake error. @ambv thoughts?
There was a problem hiding this comment.
I'm pushing a commit to do that.
There was a problem hiding this comment.
Changing this required more changes in the flake8 configuration than I would have liked. To keep this PR focused, maybe we should keep the cringeworthy formatting here for now, then change the flake8 configuration in a separate PR?
src/typing.py
Outdated
| def __subclasshook__(cls, C): | ||
| if cls is ContextManager: | ||
| if (any("__enter__" in B.__dict__ for B in C.__mro__) and | ||
| any("__exit__" in B.__dict__ for B in C.__mro__)): |
There was a problem hiding this comment.
OK, but this is worth a comment in the code.
src/typing.py
Outdated
|
|
||
| @abc.abstractmethod | ||
| def __exit__(self, exc_type, exc_value, traceback): | ||
| """Raise any exception triggered within the runtime context.""" |
There was a problem hiding this comment.
This docstring confused me. Maybe the code is better off without it?
|
PEP 484 already lists ContextManager in its laundry list of ABCs provided by typing, so I think no change is needed there. |
|
Oh I didn't know that, let me try it. |
|
OK, much better! (Also would be nice to get rid of the separate .flake8-tests but that seems a ways away -- if I run flake8 on the test files with the config from tox.ini I get a ton of errors.) |
So that there is a way to type hint for a context manager even on
language versions before 3.6.
This is just copied from contextlib.AbstractContextManager in 3.6.
Fixes #274