Skip to content

bpo-30241: implement contextlib.AbstractAsyncContextManager - #1412

Merged
1st1 merged 6 commits into
python:masterfrom
JelleZijlstra:aacm
Dec 14, 2017
Merged

bpo-30241: implement contextlib.AbstractAsyncContextManager#1412
1st1 merged 6 commits into
python:masterfrom
JelleZijlstra:aacm

Conversation

@JelleZijlstra

@JelleZijlstraJelleZijlstra commented May 3, 2017•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@mention-bot

mention-bot commented May 3, 2017

Copy link
Copy Markdown

@JelleZijlstra, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ncoghlan, @birkenfeld and @rhettinger to be potential reviewers.

ilevkivskyi
ilevkivskyi reviewed May 3, 2017
View reviewed changes
Comment threadLib/contextlib.py Outdated
def __subclasshook__(cls, C):
if cls is AbstractAsyncContextManager:
if (any("__aenter__" in B.__dict__ for B in C.__mro__) and
any("__aexit__" in B.__dict__ for B in C.__mro__)):

@ilevkivskyiilevkivskyiMay 3, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand you just copied this from above, but I think both should be updated to support the explicit "anti-registration" __enter__ = None pattern, like all classes in collections.abc, see _check_methods function in _collections_abc.py.

@JelleZijlstraJelleZijlstraMay 3, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that should be a separate issue? AbstractContextManager should arguably be fixed even in 3.6, and this PR only targets 3.7. I can fix AbstractAsyncContextManager though.

@ilevkivskyiilevkivskyiMay 3, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, a separate PR for 3.6 is needed. But I think it makes sense to already fix the async version here.

@JelleZijlstraJelleZijlstraMay 4, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed http://bugs.python.org/issue30266 to track that.

@gvanrossum

gvanrossum commented Jun 9, 2017

Copy link
Copy Markdown
Member

Wait, isn't this a new feature that should go into 3.7?

@ilevkivskyi

ilevkivskyi commented Jun 9, 2017

Copy link
Copy Markdown
Member

Wait, isn't this a new feature that should go into 3.7?

Oh yes, sorry, this one is not urgent then.

@JelleZijlstraJelleZijlstra mentioned this pull request Jun 9, 2017
Merged
JelleZijlstra added a commit to JelleZijlstra/typeshed that referenced this pull request Jun 24, 2017
@JelleZijlstra
add typing.AsyncContextManager and contextlib.asynccontextmanager
32977e0
Implements:
- python/typing#438
- python/cpython#360python/cpython#1412, which adds
contextlib.AbstractAsyncContextManager, has not yet been merged.
@JelleZijlstraJelleZijlstra mentioned this pull request Jun 24, 2017
Merged
gvanrossum pushed a commit to python/typeshed that referenced this pull request Jun 27, 2017
@JelleZijlstra@gvanrossum
add typing.AsyncContextManager and contextlib.asynccontextmanager (#1432
22f47fd
)
Implements:
- python/typing#438
- python/cpython#360
Note that python/cpython#1412, which adds
contextlib.AbstractAsyncContextManager, has not yet been merged.
@1st1

1st1 commented Oct 10, 2017

Copy link
Copy Markdown
Member

Please add a NEWS entry and I'll merge this in.

@JelleZijlstra

JelleZijlstra commented Oct 11, 2017

Copy link
Copy Markdown
MemberAuthor

Thanks, just pushed a NEWS entry.

1st1
1st1 reviewed Oct 11, 2017
View reviewed changes
Comment threadDoc/library/contextlib.rst Outdated

.. class:: AbstractAsyncContextManager

An :term:`abstract base class` similar to

@1st11st1Oct 11, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we could just copy the paragraph from AbstractContextManager doc. "similar to" isn't easy to read here.

@Kentzo

Kentzo commented Dec 11, 2017•
edited
Loading

Copy link
Copy Markdown
Contributor

The PR #4790 adds AsyncExitStack subclass. It should inherit from AbstractAsyncContextManager.

@ilevkivskyi

ilevkivskyi commented Dec 13, 2017

Copy link
Copy Markdown
Member

@JelleZijlstra Could you please fix the merge conflict and update the docs as @1st1 proposed? I would like to have this merged soon.

@1st1
1st1 merged commit 176baa3 into python:masterDec 14, 2017
@1st1

1st1 commented Dec 14, 2017

Copy link
Copy Markdown
Member

Thanks @JelleZijlstra!

@JelleZijlstra
JelleZijlstra deleted the aacm branch December 14, 2017 01:28
@JelleZijlstra

JelleZijlstra commented Dec 14, 2017

Copy link
Copy Markdown
MemberAuthor

Thanks, and sorry for dropping the ball on making the docs change!

@1st1

1st1 commented Dec 14, 2017

Copy link
Copy Markdown
Member

NP ;)

@ilevkivskyi

ilevkivskyi commented Dec 14, 2017

Copy link
Copy Markdown
Member

Thank you @JelleZijlstra and @1st1 !

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st11st11st1 approved these changes
@ilevkivskyiilevkivskyiilevkivskyi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@JelleZijlstra@mention-bot@gvanrossum@ilevkivskyi@1st1@Kentzo@the-knights-who-say-ni@bedevere-bot