Skip to content

Conversation

@galmyk
Copy link
Contributor

@galmyk galmyk commented Oct 11, 2024

gh-125289: Update sample code in asyncio-task.rst

This will change coroutines sample code in the Awaitables section and make the example clearer.


📚 Documentation preview 📚: https://cpython-previews--125292.org.readthedocs.build/

This will change **coroutines** sample code in the **Awaitables** section and make the example clearer.
@ghost
Copy link

ghost commented Oct 11, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Member

@JacobCoffee JacobCoffee left a comment

Choose a reason for hiding this comment

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

this lgtm, but more broadly: i find this example weird in general that if i as a new user copy and paste it it just throws an error.

i think it would be nice to scour the documentation and "split" examples into working/non-working and denote them as such... @willingc thoughts?

@galmyk
Copy link
Contributor Author

galmyk commented Oct 11, 2024

@JacobCoffee I agree with splitting working and not working codes. But in this case, code works, just prints a warning:

/home/gmt/a.py:10: RuntimeWarning: coroutine 'nested' was never awaited
  nested()  # will raise a "RuntimeWarning".
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
42

I'm not native english speaker, So I fear my wording in the following line's comment is not correct:

ested()  # will raise a "RuntimeWarning".

It just prints the warning! If it is wrong, please propose a better comment.
Or is it better to remove that command?

@willingc
Copy link
Contributor

this lgtm, but more broadly: i find this example weird in general that if i as a new user copy and paste it it just throws an error.

i think it would be nice to scour the documentation and "split" examples into working/non-working and denote them as such... @willingc thoughts?

Hmm... there are a couple of concepts at work here: async/await, nesting coroutines, and example behavior

I think the use of main() provides a bit more confusion since it is an async function in the example to illustrate nesting. It may be better to use something other than main maybe outer().

Where print() is placed is a bit of a red herring, since it's the async/await nesting that governs behavior.

I do think it makes sense to improve this section of docs, but I want to make sure that we do it in a way that gets the point across clearly about nesting.

@galmyk
Copy link
Contributor Author

galmyk commented Oct 12, 2024

@willingc , @JacobCoffee
Isn't better to merge this with minimal corrections, and add a more complete change to backlog? I think we need to check other code examples, too.

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Let's revert the change in print and limit this PR to adding the runtime error comment.

galmyk and others added 2 commits October 12, 2024 22:23
Revert the added print

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Thanks @galmyk for the PR and @JacobCoffee for the initial review and discussion.

@willingc willingc added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 12, 2024
@willingc willingc merged commit fa52b82 into python:main Oct 12, 2024
27 checks passed
@miss-islington-app
Copy link

Thanks @galmyk for the PR, and @willingc for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 12, 2024
)

* Update sample code in asyncio-task.rst

This will change **coroutines** sample code in the **Awaitables** section and make the example clearer.

* Update Doc/library/asyncio-task.rst

Revert the added print

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>

* Update Doc/library/asyncio-task.rst

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>

---------

(cherry picked from commit fa52b82)

Co-authored-by: Ghorban M. Tavakoly <58617996+galmyk@users.noreply.github.com>
Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 12, 2024
)

* Update sample code in asyncio-task.rst

This will change **coroutines** sample code in the **Awaitables** section and make the example clearer.

* Update Doc/library/asyncio-task.rst

Revert the added print

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>

* Update Doc/library/asyncio-task.rst

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>

---------

(cherry picked from commit fa52b82)

Co-authored-by: Ghorban M. Tavakoly <58617996+galmyk@users.noreply.github.com>
Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 12, 2024

GH-125374 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 12, 2024
@bedevere-app
Copy link

bedevere-app bot commented Oct 12, 2024

GH-125375 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 12, 2024
willingc added a commit that referenced this pull request Oct 12, 2024
…H-125374)

gh-125289: Update sample code in asyncio-task.rst (GH-125292)

* Update sample code in asyncio-task.rst

This will change **coroutines** sample code in the **Awaitables** section and make the example clearer.

* Update Doc/library/asyncio-task.rst

Revert the added print



* Update Doc/library/asyncio-task.rst



---------

(cherry picked from commit fa52b82)

Co-authored-by: Ghorban M. Tavakoly <58617996+galmyk@users.noreply.github.com>
Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
willingc added a commit that referenced this pull request Oct 12, 2024
…H-125375)

gh-125289: Update sample code in asyncio-task.rst (GH-125292)

* Update sample code in asyncio-task.rst

This will change **coroutines** sample code in the **Awaitables** section and make the example clearer.

* Update Doc/library/asyncio-task.rst

Revert the added print



* Update Doc/library/asyncio-task.rst



---------

(cherry picked from commit fa52b82)

Co-authored-by: Ghorban M. Tavakoly <58617996+galmyk@users.noreply.github.com>
Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants