Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-94597: add asyncio.EventLoop #110723

Merged
merged 9 commits into from
Oct 12, 2023
Merged

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Oct 11, 2023

@graingert
Copy link
Contributor Author

graingert commented Oct 11, 2023

This allows running asyncio coroutines without interacting with the policy system eg:

asyncio.run(main(), loop_factory=asyncio.EventLoop)
# no need to call asyncio.set_event_loop_policy(None) !

this allows code to run without interfering with a policy that has been set in one location especially one that expects asyncio.set_event_loop() to be called after every invocation of asyncio.new_event_loop()

@graingert graingert changed the title GH-94597 add asyncio.EventLoop gh-94597: add asyncio.EventLoop Oct 11, 2023
@graingert graingert marked this pull request as ready for review October 11, 2023 16:07
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Do you intend this to be merged before or after the deprecations PR?

Comment on lines 1728 to 1729
Uses the most efficient available event loop for the given
platform.
Copy link
Member

Choose a reason for hiding this comment

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

I think this ought to explain that it's an alias for one of the above classes, and that it's the one used by default, and that it's selected the way described above (L1689-90). "Uses" doesn't even explain that it's a subclass of AbstractEventLoop.

Maybe we should be less specific about how the default is selected, and probably whatever we say about it should be only said once, but it should be made clear that this class is the default, and the default is this class.

…PC8t.rst

Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
Comment on lines 1728 to 1729
Uses the most efficient available event loop for the given
platform.
Copy link
Contributor Author

@graingert graingert Oct 12, 2023

Choose a reason for hiding this comment

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

maybe something like this? The other classes don't explain that they are subclasses of AbstractEventLoop, should that be explained in ProactorEventLoop and SelectorEventLoop?

Suggested change
Uses the most efficient available event loop for the given
platform.
Uses the most efficient available event loop for the given
platform.
It is an alias for :class:`SelectorEventLoop` on Unix and :class:`ProactorEventLoop` on Windows.

Copy link
Member

Choose a reason for hiding this comment

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

Not quite. I don't like any sentence describing a class object starting with "Uses". Classes are nouns, and the description should explain what the class is, not what it does. Also, the current algorithm for choosing a default is now specified in two places. I think it should be specified only here (and the earlier text should be changed to claim that EventLoop is the class used by default).

It's also a good idea to clarify that both concrete classes are subclasses of AbstractEventLoop (but if you're tired of going around in review you can skip that).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok I've pushed a change with some doc tweaks

@graingert
Copy link
Contributor Author

graingert commented Oct 12, 2023

Do you intend this to be merged before or after the deprecations PR?

I don't mind what order the PRs go in - I plan a cleanup PR for the CPython test suite removing redundant calls to set_event_loop_policy(None) once asyncio.EventLoop is available for use in asyncio.run calls

@kumaraditya303 kumaraditya303 self-assigned this Oct 12, 2023
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks! Merging now.

@gvanrossum gvanrossum merged commit 8c6c14b into python:main Oct 12, 2023
24 of 26 checks passed
@graingert graingert deleted the add-asyncio-event-loop branch October 12, 2023 14:14
@vstinner
Copy link
Member

It is an alias to :class:SelectorEventLoop on Unix and :class:ProactorEventLoop on Windows.

Well. In the early days of tulip/asyncio, there were more choices, and we didn't know how third-party projects will inherit/override these classes. Now I agree that asyncio.EventLoop is the way to go, it's easier to remind, it's more explicit, and it's easy to guess what is it (SelectorEventLoop or ProactorEventLoop).

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
This is needed to pave the way for deprecating and eventually killing the event loop policy system (which is over-engineered and rarely used).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants