bluesky / ophyd-async

Hardware abstraction for bluesky written using asyncio
https://blueskyproject.io/ophyd-async
BSD 3-Clause "New" or "Revised" License
7 stars 21 forks source link

`AsyncStatus` takes an `Awaitable` according to type signature but rejects anything that isn't a coroutine #346

Open callumforrester opened 1 month ago

callumforrester commented 1 month ago

If a gather is passed, for example:

>>> from ophyd_async.core import AsyncStatus
>>> import asyncio
>>> operation = asyncio.gather(asyncio.sleep(1), asyncio.sleep(2))
>>> async def foo():
...     AsyncStatus(operation)
... 
>>> 
>>> asyncio.run(foo())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib64/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    return future.result()
  File "<stdin>", line 2, in foo
  File "/home/vid18871/projects/bluesky/ophyd-async/src/ophyd_async/core/async_status.py", line 33, in __init__
    self.task = asyncio.create_task(awaitable)
  File "/usr/lib64/python3.10/asyncio/tasks.py", line 337, in create_task
    task = loop.create_task(coro)
  File "/usr/lib64/python3.10/asyncio/base_events.py", line 438, in create_task
    task = tasks.Task(coro, loop=self, name=name)
TypeError: a coroutine was expected, got <_GatheringFuture pending>
coretl commented 1 month ago

Is there a usecase for this? Now we have AsyncStatus.wrap taking arbitrary kwargs then we are mostly getting either a Task or a coroutine, so I propose to ban all other alternatives...

callumforrester commented 4 weeks ago

I have no strong preference whether the type signature is changed or things that aren't coroutines are supported, as long as one of those things happens. The only convenient use case I can think of is:

def do_thing() -> AsyncStatus:
    return AsyncStatus(asyncio.gather(task_1(), task_2(), task_3()))
coretl commented 4 weeks ago

I have no strong preference whether the type signature is changed or things that aren't coroutines are supported, as long as one of those things happens. The only convenient use case I can think of is:

def do_thing() -> AsyncStatus:
    return AsyncStatus(asyncio.gather(task_1(), task_2(), task_3()))

As we can do this:

@AsyncStatus.wrap
async def do_thing():
    return await asyncio.gather(task_1(), task_2(), task_3())

I think we should change it to only accept coroutines and Tasks

coretl commented 2 days ago

So this should change the type signature and docs to say that it only takes a Coroutine or a Task, not an Awaitable