Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Use new params in rotation scan plan #1326

Merged
merged 5 commits into from
May 13, 2024
Merged

Conversation

d-perl
Copy link
Contributor

@d-perl d-perl commented Apr 22, 2024

Contributes towards #1275

Uses the new param model throughout rotation scan plan
adds a little move chi/phi setup plan because this was expected behaviour our tests missed due to the default position being 0

To test:

  1. Run tests

@d-perl d-perl force-pushed the 1275_use_new_params_in_rotation branch 2 times, most recently from 0638225 to 0c16545 Compare April 22, 2024 16:06
@d-perl d-perl force-pushed the 1275_use_new_params_in_rotation branch from 0c16545 to bd9b695 Compare April 23, 2024 10:40
Copy link
Collaborator

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

Other than a couple of comments this looks good.

@@ -253,62 +248,61 @@ def cleanup_plan(composite: RotationScanComposite, max_vel: float, **kwargs):

def rotation_scan(
composite: RotationScanComposite,
parameters: Union[RotationScan, RotationInternalParameters, Any],
parameters: RotationScan | Any,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could: Why does this need to accept Any?

Copy link
Collaborator

@DominicOram DominicOram Apr 25, 2024

Choose a reason for hiding this comment

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

Must: This needs to be a Union, until we have a release with DiamondLightSource/blueapi#432

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually doesn't need to accept Any, I guess it used to need it with the old params for some blueAPI reason I didn't understand & I cargo-culted it in.

"y_start_um": 2.0,
"z_start_um": 3.0,
"ispyb_extras": {
"microns_per_pixel_x": 1.0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to remove microns_per_pixel and upper_left once rebased

Copy link
Collaborator

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@d-perl d-perl force-pushed the 1275_use_new_params_in_rotation branch from cb6a0b4 to ab24a61 Compare May 13, 2024 07:55
@d-perl d-perl merged commit 31e86c8 into main May 13, 2024
4 checks passed
@d-perl d-perl deleted the 1275_use_new_params_in_rotation branch May 13, 2024 15:39
olliesilvester pushed a commit to olliesilvester/mx-bluesky that referenced this pull request Aug 23, 2024
…Source/1275_use_new_params_in_rotation

Use new params in rotation scan plan
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants