Conversation
cclauss
left a comment
There was a problem hiding this comment.
Nice! LGTM.
Some OPTIONAL microoptimizations for your consideration.
|
astral-sh/ruff#20122 Was merged into the just released ˋruff` v0.12.12. |
7a8fc35 to
2903da1
Compare
| await drone.connect(system_address="udpin://0.0.0.0:14540") | ||
|
|
||
| tasks = [ | ||
| _tasks = [ |
There was a problem hiding this comment.
It is unusual to assemble these tasks but not await them. Are we certain that they will be run at some point?
There was a problem hiding this comment.
Well, so this code was working fine but ruff complained that the tasks were dangling: https://docs.astral.sh/ruff/rules/asyncio-dangling-task/.
So, the advice was to create a strong reference to the tasks. However, now ruff complains that tasks are assigned but never used. It feels like you can't win.
There was a problem hiding this comment.
@cclauss you need to actually do a read&write conversation, not just write only please.
There was a problem hiding this comment.
I was wrong... This usage is correct.
>>> import asyncio
>>> async def print_char(char: str) -> None:
... print(char)
...
>>> async def run(chars: str) -> None:
... _tasks = [asyncio.create_task(print_char(char)) for char in chars]
...
>>> asyncio.run(run("abcdefg"))
a
b
c
d
e
f
g
.github/workflows/main.yml
Outdated
| run: | | ||
| pipx run pycodestyle examples/* | ||
| pipx run ruff check --select=ASYNC,RUF006 | ||
| pipx run ruff check |
There was a problem hiding this comment.
| pipx run ruff check | |
| pipx run ruff check # default rules | |
| pipx run ruff check --select=ASYNC,RUF006 # Special rules for asyncio |
This changes things. Normally, ASYNC and RUF are off by default, so this change removes those rules and adds the default rules. https://docs.astral.sh/ruff/rules Let's run both just to be safe.
There was a problem hiding this comment.
Oh, I see. Thanks.
| _tasks = [ | ||
| asyncio.create_task(observe_current_settings(drone)), | ||
| asyncio.create_task(observe_camera_mode(drone)), | ||
| asyncio.create_task(observe_possible_setting_options(drone)), | ||
| ] |
There was a problem hiding this comment.
This syntax would delete tasks as they are completed, as discussed in ruff rule RUF006.
| _tasks = [ | |
| asyncio.create_task(observe_current_settings(drone)), | |
| asyncio.create_task(observe_camera_mode(drone)), | |
| asyncio.create_task(observe_possible_setting_options(drone)), | |
| ] | |
| tasks = { | |
| asyncio.create_task(observe_current_settings(drone)), | |
| asyncio.create_task(observe_camera_mode(drone)), | |
| asyncio.create_task(observe_possible_setting_options(drone)), | |
| } | |
| for task in tasks: | |
| task.add_done_callback(tasks.discard) |
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
e002b4a to
c428076
Compare
julianoes
left a comment
There was a problem hiding this comment.
I think that's an improvement, let's get it in before I have to fix more conflicts.
Closes #771.
@cclauss