Move CentreEllipseMethod to a more common location#1942
Move CentreEllipseMethod to a more common location#1942noemifrisina wants to merge 7 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1942 +/- ##
=======================================
Coverage 99.09% 99.10%
=======================================
Files 317 317
Lines 11995 12000 +5
=======================================
+ Hits 11887 11892 +5
Misses 108 108 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
DominicOram
left a comment
There was a problem hiding this comment.
Thanks, minor comment. Could you also add a test that explicitly checks that if you specify a large ROI it gives you the full image? It may be covered by test_get_roi_creates_correct_image_array, if it is then just a comment on one of those cases would be good.
| ellipse_fit = self._fit_ellipse(roi_binary) | ||
| roi_centre_x = ellipse_fit[0][0] | ||
| roi_centre_y = ellipse_fit[0][1] | ||
| LOGGER.info(f"Beam centre founf at ({roi_centre_x}, {roi_centre_y})") |
There was a problem hiding this comment.
Nit:
| LOGGER.info(f"Beam centre founf at ({roi_centre_x}, {roi_centre_y})") | |
| LOGGER.info(f"Beam centre found at ({roi_centre_x}, {roi_centre_y})") |
There was a problem hiding this comment.
Also, this is the centre in the ROI co-ordinates, I think the full image co-ordinates would be more expected (or specify in the log that they're ROI)
Needed for i19-bluesky#122
Instructions to reviewer on how to test:
Checks for reviewer
dodal connect ${BEAMLINE}