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

(DiamondLightSource/mx-bluesky#475) Method to allow access to full zocalo results #814

Closed
wants to merge 6 commits into from

Conversation

rtuck99
Copy link
Contributor

@rtuck99 rtuck99 commented Oct 4, 2024

Extends ZocaloResults to get access to everything instead of just the position and bounding box

Fixes DiamondLightSource/mx-bluesky#475

Copy link

codecov bot commented Oct 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.19%. Comparing base (523cef8) to head (1dfa59a).
Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #814      +/-   ##
==========================================
- Coverage   95.20%   95.19%   -0.01%     
==========================================
  Files         120      120              
  Lines        4985     4997      +12     
==========================================
+ Hits         4746     4757      +11     
- Misses        239      240       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@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.

The consensus in ophyd_async is to move away from arbitrary types on signals. I have made bluesky/ophyd-async#601 to actually try and advertise this fact. We should probably refactor this to use individual signals rather than XRCResults, like we did for #769.

Additionally, I think after this PR get_processing_result is no longer used, can we remove it?

@rtuck99 rtuck99 marked this pull request as draft October 11, 2024 09:12
@rtuck99 rtuck99 force-pushed the mx-bluesky_475_multi_centre_plan branch from 97e21f2 to 65bf6a3 Compare October 15, 2024 13:56
@rtuck99 rtuck99 marked this pull request as ready for review October 15, 2024 13:56
@rtuck99 rtuck99 force-pushed the mx-bluesky_475_multi_centre_plan branch from 65bf6a3 to 303869f Compare October 22, 2024 13:41
@dperl-dls dperl-dls dismissed DominicOram’s stale review October 23, 2024 12:26

Requested changes were adressed

Copy link
Collaborator

@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.

Looks good, approving, not merging until DiamondLightSource/mx-bluesky#553 is ready

@rtuck99
Copy link
Contributor Author

rtuck99 commented Oct 30, 2024

This PR now obsoleted by #858

@rtuck99 rtuck99 marked this pull request as draft October 30, 2024 13:58
@DominicOram
Copy link
Contributor

With #858 merged I think this is now redundant so closing

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.

Make a plan that will do an XRC and data collection in multiple places
3 participants