-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
0638225
to
0c16545
Compare
0c16545
to
bd9b695
Compare
There was a problem hiding this 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, |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
cb6a0b4
to
ab24a61
Compare
…Source/1275_use_new_params_in_rotation Use new params in rotation scan plan
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: