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

Update mx-bluesky references to ApertureScatterguard #430

Merged
merged 6 commits into from
Sep 3, 2024

Conversation

DiamondJoseph
Copy link
Contributor

For dodal change DiamondLightSource/dodal#769

Required for dodal pydantic 2/ophyd_async 0.5.0 update

Ensure values match previous values
Copy link
Contributor

@dperl-dls dperl-dls left a comment

Choose a reason for hiding this comment

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

lgtm, just one update for that test

@DiamondJoseph DiamondJoseph merged commit 6ca9e1f into main Sep 3, 2024
17 checks passed
@DiamondJoseph DiamondJoseph deleted the aperture_scatterguard branch September 3, 2024 14:01
@@ -153,7 +151,7 @@ def _handle_ispyb_transmission_flux_read(self, doc) -> Sequence[ScanDataInfo]:
hwscan_data_collection_info, None, self.params
)
ISPYB_LOGGER.info("Updating ispyb data collection after flux read.")
self.append_to_comment(f"Aperture: {aperture_size.name}. ")
self.append_to_comment(f"Aperture: {aperture}. ")
Copy link
Contributor

@DominicOram DominicOram Sep 3, 2024

Choose a reason for hiding this comment

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

Now that we've dropped the user friendly names in the device this now means that the comment in ispyb is Aperture: SMALL_APERTURE rather than Aperture: Small, right? I think this might be ok, just noting the change in behaviour

Copy link
Contributor Author

@DiamondJoseph DiamondJoseph Sep 3, 2024

Choose a reason for hiding this comment

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

I just checked, it's Aperture: ApertureValue.SMALL: ApertureValue.SMALL.value == SMALL_APERTURE. We could ammend the __str__ of the ApertureValue if we want to get just Small?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be exact it's Aperture: ApertureValue.SMALL. with stop and space

Copy link
Contributor

Choose a reason for hiding this comment

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

Yh, that's quite ugly. Sorry, could we fix it to be Small etc. and add a test? If you want to move on to other things feel free to just add a new issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make an issue, I'm trying to finish the Pydantic update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

3 participants