Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC: annotate bluesky.plans signatures #1600

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tangkong
Copy link

Description

  • annotates plan signatures with type hints
  • touches up and standardizes docstrings a bit

Motivation and Context

bluesky_queueserver's annotation collection mechanisms require annotations to collect.

The annotations are restricted to Python base types, NoneType, and imports from the typing module. This means common arguments like ophyd.Device -> Any

Some questions:

  • It's not obvious from the top-level plan definitions, but are these generators are also sent Msg objects? I omitted the return type but suppose it would be plan() -> Generator[Msg, Msg, None]?
  • should spiral_square's x_num and y_num be int rather than float?

How Has This Been Tested?

Test suite passes locally. Also some fiddling with the queueserver will take place after this

@tangkong
Copy link
Author

FWIW the numpy drop schedule has:

On Apr 14, 2023 drop support for Python 3.8 (initially released on Oct 14, 2019)

@tangkong
Copy link
Author

Turns out that the type hint I applied to most detector list arguments (typing.List[Any]) will result in bluesky-queueserver not converting the provided arg into an actual device.

To be compatible with the current state of the queueserver, we either need to delete this hint or use the parameter_annotation_decorator to specify it is a __DEVICE__

At this point, the changes in this PR are maybe less than useful, since they don't actually enable bluesky-queueserver integration. The best we get are the occasional float annotation.

@dmgav
Copy link
Contributor

dmgav commented Jul 31, 2023

By default, the queue server attempts to convert all string passed as values of each parameter without type annotation to a device or a plan object with matching name. The idea was that 'lasy' user may use queue server with plans that have no type annotations. This seems to work in simple cases, but it is not very efficient and may fail if certain cases. In my experience, once I mention that type annotation is optional, users don't want to listen any further. Specifying type as Any adds type annotation to the parameter and disables conversion.

Moving the annotation decorator into Bluesky package may be challenging. I am thinking if there is an alternative way of telling the queue server that the parameter value is a device. For example, the names such as ophyd.Device or ophyd.OphydObject could be automatically replaced by __DEVICE__. It was mentioned that the devices do not have to be inherited from ophyd.Device. The queue server is using protocols.Readable and protocols.Flyable to decide if an object is a device: https://github.com/bluesky/bluesky-queueserver/blob/a6d9b489b51589f2c2e9509b3125329c80274d18/bluesky_queueserver/manager/profile_ops.py#L701 I am not sure if using typing extensions in annotations makes sense.

@dmgav
Copy link
Contributor

dmgav commented Aug 18, 2023

I pushed a few commits. The Queue Server (in 'main' branch) should be able to properly handle the updated annotations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants