Fix and improve asyncio.run() docs

[aeros]

This PR is based on the suggestions I made in #16337 (comment), which were approved of by @asvetlov.

[aeros]

@1st1 I think it's useful to explicitly mention that a ValueError is raised when something other than a coroutine is passed to coro, but you are are correct that the RuntimeError scenario is already mentioned in a previous section, so that can definitely be removed.

My main concern is just addressing the typo though, so if you want to remove the ValueError as well that's fine. Let me know either way.

[1st1]

changes in runners.py appear to be musindented

[aeros]

@1st1 Done. I fixed the indent in the docs earlier, but forgot to do it in runners.py, good catch.

[aeros]

I also replaced the header of the docstring "Run a coroutine" with "Execute the coroutine coro and return the result". I would argue that "Run a coroutine" could be gathered from the name of the function and parameter alone. OTH, "Execute the coroutine coro and return the result" provides a bit more information while still remaining succinct.

I can remove this change if needed, but I think it provides a decent improvement and keeps the docstring in sync with the docs.

[aeros]

@1st1 The recommended changes have been applied. Thanks for the in-depth review. I'd much rather see this go through multiple revisions and be as high quality as possible, instead of having the PR merged more quickly. The former provides far more value to the users of asyncio.

[aeros]

On second thought, I'll remove coro from the docstring. Since name of the parameter is technically main within runners.py, it wouldn't make sense have coro there.

[asvetlov]

Looks good.
Would you consider renaming coro to main in rst?
I think it makes sense for emphasizing asyncio.run() audience (plus reflects the implementation better).

Please double-check how :term:`coroutine` is rendered. IIRC it was Coroutines or something like this, we used :term:`coroutine <coroutine>` notation for some reason.

[1st1]

Would you consider renaming coro to main in rst?

Would maincoro work?

[aeros]

@1st1:

Would maincoro work?

I think this name would make more sense, but it would be a bit odd if the .rst wasn't consistent with the actual name of the parameter in runners.py (which was @asvetlov's main concern I believe).

Are we able to change the actual parameter name in runners.py without causing backwards compatibility issues? Since we didn't specify the "/" positional only symbol from PEP 570, users could technically call asyncio.run() with asyncio.run(main=main()) , which would no longer work if main were to be changed to maincoro (I can't imagine why they would though).

However, that might not be an issue if the parameter name main was never officially documented in the .rst, therefore making it not a part of the public API.

[aeros]

@asvetlov:

Please double-check how :term:coroutine is rendered. IIRC it was Coroutines or something like this, we used :term:coroutine <coroutine> notation for some reason.

No problem, I'll build the html locally to make sure the link works properly.

asyncio.run() docs:

Clicking the link correctly leads to the glossary entry:

I believe the situation you were referring to was probably where the word was "coroutines", so you had to use:

:term:`coroutines <coroutine>`

Something like this

:term:`coroutines`

wouldn't work because there is not a literal term in the glossary for "coroutines".

That's a pretty commonly occurring issue with Sphinx, since the link target is very literal.

[aeros]

Looking at the glossary entry made me realize there's a minor typo in it that I hadn't noticed before. "Coroutines is a more generalized form of subroutines" should be "Coroutines are a more generalized form of subroutines". I'll add the correction to this PR, since it's just fixing a single word. That wouldn't be worth it's own PR, but might as well be fixed.

[aeros]

@1st1 Is there anything left to do with this PR? It's currently blocking #16360 since I'll need to resolve a merge conflict. Neither are particularly time sensitive by any means, I just wanted to wrap these up before we start working on the new streaming API for 3.9.

Edit: The only part left was resolving the maincoro idea (#16403 (comment)).

[miss-islington]

Thanks @aeros167 for the PR, and @1st1 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

[bedevere-bot]

GH-16504 is a backport of this pull request to the 3.8 branch.

[1st1]

@1st1 Is there anything left to do with this PR?

And... merged. Thank you, Kyle!

[bedevere-bot]

GH-16505 is a backport of this pull request to the 3.7 branch.

[aeros]

Thanks!