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

Demark behavioural differences between plans and stubs, move orphaned plans into dodal. #793

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

DiamondJoseph
Copy link
Contributor

Bluesky distinguishes between plans: complete experimental proceedures, which open and close data collection runs, which may be part of a larger plan that collect data multiple times, but that might also be run alone to collect data, and plan_stubs: which do not create & complete data collection runs and are either isolated behaviours or building blocks for plans.

In order to make it clearer when a MsgGenerator can be safely used without considering the enclosing run, when it is required to manage a run and when running a procedure will create data documents, I think we should adopt this standard.

This change also adds the count and spec_scan plan previously from dls_bluesky_core, as remove this as a requirement for blueapi is long overdue and this is the last remnants of that module. count is a basic collection of a set of devices, equivalent to Mapping GDA's StaticScan. spec_scan is analogous to GDA's mscan command, constructing a generic N-dimensional trajectory for scanning.

Checks for reviewer

  • Would the PR title make sense to a scientist on a set of release notes
  • If a new device has been added does it follow the standards
  • If changing the API for a pre-existing device, ensure that any beamlines using this device have updated their Bluesky plans accordingly
  • Have the connection tests for the relevant beamline(s) been run via dodal connect ${BEAMLINE}

),
],
spec: Annotated[
Spec[Movable], # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@coretl @callumforrester do you have any idea why this is currently required? Previously we had a Spec[str] in blueapi and this did not cause issues- here in dodal, it seems we cannot have a Spec[str] or Spec[Movable].

"__getitem__" method not defined on type "type"Pylance[reportIndexIssue](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportIndexIssue)

This is an issue in Pylance in the IDE and also in the type-checking step of the CI job.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was due to the discriminated_union decorator's type hinting not being adequate.
This change is therefore blocked until ScanSpec 0.7.3 can be released, which is blocked until time is spent to handle dataclasses.

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.

1 participant