Add async capabilities to protocols

[thomascobb]

Description

An attempt at the tweaks outlined in #1501 to provoke discussion

Motivation and Context

So that asyncio can be used in ophyd.v2 and in preparation for bluesky/ophyd#1013

How Has This Been Tested?

Not tested yet, will be tested as part of ophyd.v2 when changes are agreed, but here as a draft for discussion

[thomascobb]

@callumforrester any comments?

[callumforrester]

Have left a few, @tacaswell and @danielballan feel free to correct me on any of them

[callumforrester]

@thomascobb Food for thought: Have we considered maintaining two sets of protocols rather than one set with doubled-up capabilities?

For example:

@runtime_checkable
class Movable(Protocol):
    def set(self, value: Any) -> Status:
        """Return a ``Status`` that is marked done when the device is done moving."""
        ...

@runtime_checkable
class AsyncMovable(Protocol):
    async def set(self, value: Any) -> AsyncStatus:
        """Return ``AsyncStatus`` that is marked done when the device is done moving."""
        ...

It allows for a clean break and doesn't change where the differentiating logic in the RE has to go. I can imagine there are disadvantages though, bloat for one. It might help to resolve some of the backwards-compatibility issues discussed here though.

[thomascobb]

@thomascobb Food for thought: Have we considered maintaining two sets of protocols rather than one set with doubled-up capabilities?

I started there, but it turns out runtime_checkable can't tell the difference between those two protocols, which means an additional inspect call as well as the isinstance to tell what protocols an object supports. At that point you might as well call it an see whether it returns an Awaitable or not. That's what led me to try and combine the two.

[untzag]

@thomascobb really basic question here---sorry I'm slow 😄

The protocols already support callbacks for subscription. Why do we need to change the protocols for ophyd v2? Couldn't asyncio be an implementation detail?

[thomascobb]

@thomascobb really basic question here---sorry I'm slow smile

The protocols already support callbacks for subscription. Why do we need to change the protocols for ophyd v2? Couldn't asyncio be an implementation detail?

This is driven by 2 things:

1. For asyncio, we would like callables to optionally be coroutines
2. It would appear that subscribe doesn't actually return what the run engine wants. At the moment it has to call read() each time the callback fires:

   bluesky/bluesky/bundlers.py

   Lines 273 to 285 in dc8b36f

   	def emit_event(*args, **kwargs):
   	# Ignore the inputs. Use this call as a signal to call read on the
   	# object, a crude way to be sure we get all the info we need.
   	data, timestamps = _rearrange_into_parallel_dicts(obj.read())
   	doc = dict(
   	descriptor=descriptor_uid,
   	time=ttime.time(),
   	data=data,
   	timestamps=timestamps,
   	seq_num=next(seq_num_counter),
   	uid=new_uid(),
   	)
   	self.emit_sync(DocumentNames.event, doc)

If we narrowed the protocol be:

Callback = Callable[[Dict[str, EventData]], SyncOrAsync[None]]

@runtime_checkable
class Subscribable(Protocol):
    def subscribe(self, function: Callback) -> None:
        ...
    def clear_sub(self, function: Callback) -> None:
        ...

Then:

1. Would probably be OK with current ophyd as the RE doesn't do sync callbacks
2. We could check in the RE what was passed to the callback function, and fallback to the current behaviour of calling read() if it didn't contain a Dict[str, EventData]

[untzag]

Thanks @thomascobb

It would appear that subscribe doesn't actually return what the run engine wants. At the moment it has to call read() each time the callback fires.

Wow, I missed this subtlety I see what you mean that needs to change.

For asyncio, we would like callables to optionally be coroutines.

As in passing awaitable callbacks into my subscribe method? Can you say way? You can write a synchronous function which launches coroutine(s), isn't that enough?

[thomascobb]

As in passing awaitable callbacks into my subscribe method? Can you say way? You can write a synchronous function which launches coroutine(s), isn't that enough?

That's a good point, I was thinking about this like read(), where the protocol is implemented by ophyd, so needs to be async compatible, so we can handle the complexity of the return value in the RE. For subscribe(), the callback is implemented only in the RE, so it makes sense to put the complexity there, and make the subscribe function as simple as possible, i.e. sync only.

I'll make some edits to the PR and resolve some of these threads...

[tacaswell]

A suggested name for this file: duckery.py 🦆 🦆

[coretl]

A suggested name for this file: duckery.py

I don't think typing.Protocol was the best choice of name, I would have gone with typing.Trait or typing.Interface, although maybe they should have tried typing.Duck? With that in mind we could go with:

1. protocols.py
2. duckery.py
3. traits.py
4. interfaces.py

@danielballan, @tacaswell, @untzag, @ksunden what do you reckon?

[coretl]

Remaining hasattrs and getattrs:

• read_attrs
• rewindable
• RealPosition
• dotted_name
• component_names
  Are any of these important enough to go in the protocols?

[untzag]

I have a fairly strong preference to follow the Python language standards where possible. Python chose the word "Protocol", I think we should be consistent.

[untzag]

I think we should escalate the concept of components to protocol in a future PR.

[coretl]

I think we should escalate the concept of components to protocol in a future PR.

At the moment I don't have the Component class in ophyd.v2, whether I'll need to introduce it in the future is a different matter. Apart from the use case of creating Signals (which I think I've covered), where else are they used?

[untzag]

I think nested hardware interfaces are common to many different control systems. Queueserver recently added support for dot syntax to address components. As more control systems become supported it would be useful to standardize this syntax because right now it's just a convention based on how ophyd did it (as far as I know).

[coretl]

• read_attrs - can get rid of this
• rewindable - @tacaswell will check if we need this
• RealPosition - thinking of pushing logic of merging Devices down to Ophyd so don't need this, and maybe not parent. Used so you can't scan Real and Pseudo together, also needed for CS motor requirements
• dotted_name - implementation detail of ophyd.v1, don't need this
• component_names: List[str] - these are child attribute names that are also Abilities, make new Protocol for StructuredDevice

[coretl]

• Add get_setpoint and get_readback and call it class SomethingMovable(Movable)

[coretl]

I think we should try and close this out sooner rather than later, so have raised #1517 to cover issues that don't have an immediately obvious solution, or need more discussion. They should all be additions rather than changes to this PR, so this is probably a good section to merge on its own.

[tacaswell]

rewindable (which is currently implemented as a Signal (😱 )) should be an attribute an anything triggerable or flyable.

It is a mechanism that a device can use to inform trigger and read that going back and naively re-triggering the device when recovering from an interruption (either due to manual intervention by the user or from suspenders).

The work also needs to be done for bluesky.plans.fly to actually use this value.

I think that changing this to be a normal attribute is on the table. Trying to re-construct what we were thinking I suspect we made it a signal so that we could put the value in read_configuration but I am not sure if I would still agree with that position.

[coretl]

I have squashed this and rebased on master in #1522