From e750129de2270dd69d457539b74b163c39a7f068 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 18 Feb 2026 17:55:23 +0000 Subject: [PATCH 01/17] Initial commit, splitting params into focus, capture, stack --- .../things/autofocus.py | 88 +++++++++++++++++-- .../things/scan_workflows.py | 88 +++++++++++++------ tests/unit_tests/test_scan_workflows.py | 4 +- tests/unit_tests/test_stack.py | 46 ++-------- 4 files changed, 150 insertions(+), 76 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 9001601b..f62416aa 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -33,15 +33,25 @@ class NotStreamingError(RuntimeError): """No images captured from stream. The camera is almost certainly not streaming.""" -class SmartStackParams(BaseModel): - """A class for holding for smart stack parameters, and returning computed ones.""" +class AutofocusParams(BaseModel): + """A class for running autofocus routines.""" + + dz: int + sharpness_method: str = "jpeg" + + +class CaptureParams(BaseModel): + """A class for capturing at least a single image.""" + + images_dir: str + save_resolution: tuple[int, int] + + +class StackParams(BaseModel): + """A class for holding stack parameters, and returning computed ones.""" stack_dz: int images_to_save: int - min_images_to_test: int - autofocus_dz: int - images_dir: str - save_resolution: tuple[int, int] # Using docstrings under variables as this is how pdoc would expect # attributed to be documented @@ -49,6 +59,12 @@ class SmartStackParams(BaseModel): settling_time: float = 0.3 """Time (in seconds) between moving and capturing an image""" + +class SmartStackParams(StackParams): + """A class for holding smart stack parameters, and returning computed ones.""" + + min_images_to_test: int + backlash_correction: int = 250 """ Distance (in steps) to overshoot a move and then undo, to account for backlash @@ -457,6 +473,8 @@ class AutofocusThing(lt.Thing): def run_smart_stack( self, stack_parameters: SmartStackParams, + capture_parameters: CaptureParams, + autofocus_parameters: AutofocusParams, save_on_failure: bool = False, check_turning_points: bool = True, ) -> tuple[bool, int]: @@ -498,7 +516,7 @@ class AutofocusThing(lt.Thing): initial_z_pos = captures[0].position["z"] # If a stack is not successful, move to the start and autofocus try: - self.reset_stack(initial_z_pos, stack_parameters.autofocus_dz) + self.reset_stack(initial_z_pos, autofocus_parameters.dz) except NoFocusFoundError: break @@ -510,6 +528,7 @@ class AutofocusThing(lt.Thing): sharpest_id=sharpest_id, captures=captures, stack_parameters=stack_parameters, + capture_parameters=capture_parameters, ) return success, _get_capture_by_id(captures, sharpest_id).position["z"] @@ -535,6 +554,7 @@ class AutofocusThing(lt.Thing): sharpest_id: int, captures: list[CaptureInfo], stack_parameters: SmartStackParams, + capture_parameters: CaptureParams, ) -> int: """Save the required captures to disk. @@ -552,8 +572,8 @@ class AutofocusThing(lt.Thing): # Loop through the range, saving each capture to disk for capture in captures[slice_to_save]: self._cam.save_from_memory( - jpeg_path=os.path.join(stack_parameters.images_dir, capture.filename), - save_resolution=stack_parameters.save_resolution, + jpeg_path=os.path.join(capture_parameters.images_dir, capture.filename), + save_resolution=capture_parameters.save_resolution, buffer_id=capture.buffer_id, ) self._cam.clear_buffers() @@ -712,6 +732,56 @@ class AutofocusThing(lt.Thing): return "success", capture_id + @lt.action + def run_basic_stack( + self, + stack_parameters: StackParams, + capture_parameters: CaptureParams, + ) -> tuple[int, list[int]]: + """Capture a simple z-stack with no focus testing or fitting. + + This performs a fixed stack of images spaced by `stack_dz`, + saving all images captured. No sharpness testing, restart + logic, or autofocus is performed. + + :param stack_parameters: SmartStackParams defining stack spacing, + image count, directory, and save resolution. + + :returns: + - Final z position + - List of z positions captured + """ + captures: list[CaptureInfo] = [] + z_positions: list[int] = [] + + # Capture images_to_save images + for _ in range(stack_parameters.images_to_save): + time.sleep(stack_parameters.settling_time) + + capture = self.capture_stack_image( + buffer_max=stack_parameters.images_to_save + ) + captures.append(capture) + z_positions.append(capture.position["z"]) + + self._stage.move_relative(z=stack_parameters.stack_dz) + + # Save all captures + for capture in captures: + self._cam.save_from_memory( + jpeg_path=os.path.join( + capture_parameters.images_dir, + capture.filename, + ), + save_resolution=capture_parameters.save_resolution, + buffer_id=capture.buffer_id, + ) + + self._cam.clear_buffers() + + final_z = self._stage.position["z"] + return final_z, z_positions + class NotAPeakError(lt.exceptions.InvocationError): """The data to fit isn't a peak.""" diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 85198a9e..42f11a18 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -31,7 +31,9 @@ from openflexure_microscope_server.stitching import ( from openflexure_microscope_server.things.autofocus import ( MAX_TEST_IMAGE_COUNT, MIN_TEST_IMAGE_COUNT, + AutofocusParams, AutofocusThing, + CaptureParams, SmartStackParams, ) from openflexure_microscope_server.things.background_detect import ( @@ -101,7 +103,7 @@ class ScanWorkflow(Generic[SettingModelType], lt.Thing): is returned if it is not possible to stitch the scan. """ raise NotImplementedError( - "Each specific ScanWorkflow must implement a `all_settings`. method." + "Each specific ScanWorkflow must implement a `all_settings` method." ) def pre_scan_routine(self, settings: SettingModelType) -> None: @@ -239,6 +241,36 @@ class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType] correlation_resize=STITCHING_RESOLUTION[0] / self.save_resolution[0], ) + def all_settings( + self, images_dir: str + ) -> tuple[SettingModelType, Optional[StitchingSettings]]: + """Return the scan settings and the stitching settings. + + - `images_dir` is used to create the CaptureParams stored in the settings model. + - The returned SettingModelType now contains param objects: + * capture_params: CaptureParams + * autofocus_params: AutofocusParams + * stack_params: StackParams (if relevant) + """ + stitching_settings = self._get_stitching_settings_model() + dx, dy = self._calc_displacement_from_overlap(self.overlap) + + capture_params = CaptureParams( + images_dir=images_dir, save_resolution=self.save_resolution + ) + + autofocus_params = AutofocusParams(dz=self.autofocus_dz) + + scan_settings = self._settings_model( + overlap=self.overlap, + dx=dx, + dy=dy, + capture_params=capture_params, + autofocus_params=autofocus_params, + ) + + return scan_settings, stitching_settings + @lt.property def ready(self) -> bool: """Whether this scanworkflow is ready to start.""" @@ -257,6 +289,8 @@ class HistoScanSettingsModel(BaseModel): dy: int max_dist: int skip_background: bool + capture_params: CaptureParams + autofocus_params: AutofocusParams smart_stack_params: SmartStackParams @@ -361,7 +395,7 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): def all_settings( self, images_dir: str ) -> tuple[HistoScanSettingsModel, StitchingSettings]: - """Return the workflow and stitching settings. + """Return the workflow settings and stitching settings. :param images_dir: The directory that images are to be written to. :return: A tuple containing the settings model for this workflow and the @@ -370,11 +404,12 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): stitching_settings = self._get_stitching_settings_model() dx, dy = self._calc_displacement_from_overlap(self.overlap) - smart_stack_params = self.create_smart_stack_params( + capture_params = CaptureParams( images_dir=images_dir, - autofocus_dz=self.autofocus_dz, save_resolution=self.save_resolution, ) + autofocus_params = AutofocusParams(dz=self.autofocus_dz) + smart_stack_params = self.create_smart_stack_params() scan_settings = HistoScanSettingsModel( overlap=self.overlap, @@ -382,6 +417,8 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): dx=dx, dy=dy, skip_background=self.skip_background, + capture_params=capture_params, + autofocus_params=autofocus_params, smart_stack_params=smart_stack_params, ) @@ -389,15 +426,8 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): def create_smart_stack_params( self, - images_dir: str, - autofocus_dz: int, - save_resolution: tuple[int, int], ) -> SmartStackParams: - """Set up the parameters used for all stacks in a scan. - - :param images_dir: the folder to save all images - :param autofocus_dz: the range to autofocus over if a stack fails - :param save_resolution: The resolution to save the captures to disk with + """Set up the parameters used for all smart stacks in a scan. :returns: A StackSmartParams object with the required parameters. """ @@ -452,9 +482,6 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): stack_dz=self.stack_dz, images_to_save=self.stack_images_to_save, min_images_to_test=self.stack_min_images_to_test, - autofocus_dz=autofocus_dz, - images_dir=images_dir, - save_resolution=save_resolution, ) def pre_scan_routine(self, settings: HistoScanSettingsModel) -> None: @@ -463,7 +490,7 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): :param settings: The settings for this scan as a HistoScanSettingsModel """ self._autofocus.looping_autofocus( - dz=settings.smart_stack_params.autofocus_dz, start="centre" + dz=settings.autofocus_params.dz, start="centre" ) def new_scan_planner( @@ -518,6 +545,8 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): focus_height: Optional[int] focused, focus_height = self._autofocus.run_smart_stack( stack_parameters=settings.smart_stack_params, + capture_parameters=settings.capture_params, + autofocus_parameters=settings.autofocus_params, save_on_failure=save_on_failure, ) # An image was captured if we are focussed or we are not skipping background. @@ -575,9 +604,8 @@ class RegularGridSettingsModel(BaseModel): x_count: int y_count: int style: Literal["snake", "raster"] - images_dir: str - autofocus_dz: int - save_resolution: tuple[int, int] + capture_params: CaptureParams + autofocus_params: AutofocusParams class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): @@ -604,6 +632,13 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): stitching_settings = self._get_stitching_settings_model() dx, dy = self._calc_displacement_from_overlap(self.overlap) + capture_params = CaptureParams( + images_dir=images_dir, + save_resolution=self.save_resolution, + ) + + autofocus_params = AutofocusParams(dz=self.autofocus_dz) + scan_settings = self._settings_model( overlap=self.overlap, dx=dx, @@ -611,9 +646,8 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): x_count=self.x_count, y_count=self.y_count, style=self._grid_style, - images_dir=images_dir, - autofocus_dz=self.autofocus_dz, - save_resolution=self.save_resolution, + capture_params=capture_params, + autofocus_params=autofocus_params, ) return scan_settings, stitching_settings @@ -625,7 +659,9 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): :param settings: The settings for this scan as as the relevant SettingsModel type. """ - self._autofocus.looping_autofocus(dz=settings.autofocus_dz, start="centre") + self._autofocus.looping_autofocus( + dz=settings.autofocus_params.dz, start="centre" + ) def new_scan_planner( self, settings: RegularGridSettingsModel, position: Mapping[str, int] @@ -659,9 +695,9 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): """ return self._autofocus_and_capture( xyz_pos=xyz_pos, - dz=settings.autofocus_dz, - images_dir=settings.images_dir, - save_resolution=settings.save_resolution, + dz=settings.autofocus_params.dz, + images_dir=settings.capture_params.images_dir, + save_resolution=settings.capture_params.save_resolution, ) @lt.property diff --git a/tests/unit_tests/test_scan_workflows.py b/tests/unit_tests/test_scan_workflows.py index 38ca6d61..40e62cc9 100644 --- a/tests/unit_tests/test_scan_workflows.py +++ b/tests/unit_tests/test_scan_workflows.py @@ -130,7 +130,7 @@ def test_histo_workflow_settings_generation(histo_workflow, mocker): assert workflow_settings.dx == 123 assert workflow_settings.dy == 456 # And that the input image dir is passed to stack the stack parameter for saving - assert workflow_settings.smart_stack_params.images_dir == "/this/img_dir" + assert workflow_settings.capture_params.images_dir == "/this/img_dir" # A CSM that is "normal" changing from camera maxtrix coordinates (y,x) to normal @@ -196,7 +196,7 @@ def test_histo_pre_scan_routine(histo_workflow, mocker): # Rather than create a whole Setting class, just create a mock with the value # we need set mock_settings = mocker.Mock() - mock_settings.smart_stack_params.autofocus_dz = 1234 + mock_settings.autofocus_params.dz = 1234 # Run the function histo_workflow.pre_scan_routine(mock_settings) # Check the autofocus was run using the mocked slot. diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 7cc4fe7e..fe1a4017 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -66,12 +66,7 @@ def test_stack_params_validation(save_ims, extra_ims): # to do automatically in hypothesis. This clamps the number between 3 and 9. min_images_to_test = max(min(save_ims + extra_ims, 9), 3) SmartStackParams( - stack_dz=50, - images_to_save=save_ims, - min_images_to_test=min_images_to_test, - autofocus_dz=2000, - images_dir="/this/is/fake", - save_resolution=(1640, 1232), + stack_dz=50, images_to_save=save_ims, min_images_to_test=min_images_to_test ) @@ -96,9 +91,6 @@ def test_stack_params_not_enough_test_images(save_ims, extra_ims): stack_dz=50, images_to_save=save_ims, min_images_to_test=save_ims + extra_ims, - autofocus_dz=2000, - images_dir="/this/is/fake", - save_resolution=(1640, 1232), ) @@ -121,9 +113,6 @@ def test_stack_params_negative_images_to_save(save_ims, extra_ims): stack_dz=50, images_to_save=save_ims, min_images_to_test=save_ims + extra_ims, - autofocus_dz=2000, - images_dir="/this/is/fake", - save_resolution=(1640, 1232), ) @@ -147,9 +136,6 @@ def test_even_min_images_to_test(save_ims, extra_ims): stack_dz=50, images_to_save=save_ims, min_images_to_test=save_ims + extra_ims, - autofocus_dz=2000, - images_dir="/this/is/fake", - save_resolution=(1640, 1232), ) @@ -171,9 +157,6 @@ def test_even_images_to_save(save_ims, extra_ims): stack_dz=50, images_to_save=save_ims, min_images_to_test=save_ims + extra_ims, - autofocus_dz=2000, - images_dir="/this/is/fake", - save_resolution=(1640, 1232), ) @@ -183,12 +166,7 @@ def test_computed_stack_params(): Not using hypothesis or we will just copy in the same formulas. """ stack_parameters = SmartStackParams( - stack_dz=50, - images_to_save=5, - min_images_to_test=9, - autofocus_dz=2000, - images_dir="/this/is/fake", - save_resolution=(1640, 1232), + stack_dz=50, images_to_save=5, min_images_to_test=9 ) assert stack_parameters.stack_z_range == 8 * 50 @@ -279,9 +257,7 @@ def test_create_stack(histo_scan_workflow, caplog): initial_min_images_to_test = histo_scan_workflow.stack_min_images_to_test initial_images_to_save = histo_scan_workflow.stack_images_to_save with caplog.at_level(logging.INFO): - stack_params = histo_scan_workflow.create_smart_stack_params( - autofocus_dz=2000, images_dir="/this/is/fake", save_resolution=(1640, 1232) - ) + stack_params = histo_scan_workflow.create_smart_stack_params() assert len(caplog.records) == 0 assert histo_scan_workflow.stack_min_images_to_test == initial_min_images_to_test @@ -308,9 +284,7 @@ def test_coercing_stack_test_ims( histo_scan_workflow.stack_min_images_to_test = initial_test_ims with caplog.at_level(logging.WARNING): - stack_params = histo_scan_workflow.create_smart_stack_params( - autofocus_dz=2000, images_dir="/this/is/fake", save_resolution=(1640, 1232) - ) + stack_params = histo_scan_workflow.create_smart_stack_params() assert len(caplog.records) == 1 assert str(caplog.records[0].msg).startswith(expected_log_start) @@ -338,9 +312,7 @@ def test_coercing_stack_save_ims( histo_scan_workflow.stack_images_to_save = initial_save_ims with caplog.at_level(logging.WARNING): - stack_params = histo_scan_workflow.create_smart_stack_params( - autofocus_dz=2000, images_dir="/this/is/fake", save_resolution=(1640, 1232) - ) + stack_params = histo_scan_workflow.create_smart_stack_params() assert len(caplog.records) == 1 assert str(caplog.records[0].msg).startswith(expected_log_start) @@ -353,9 +325,7 @@ def test_coercing_stack_save_ims( @pytest.mark.parametrize("pass_on", [1, 2, 3, 4]) def test_run_smart_stack(pass_on, histo_scan_workflow, autofocus_thing, mocker): """Test Running smart stack with the stack passing on different attempts.""" - stack_params = histo_scan_workflow.create_smart_stack_params( - autofocus_dz=2000, images_dir="/this/is/fake", save_resolution=(1640, 1232) - ) + stack_params = histo_scan_workflow.create_smart_stack_params() assert stack_params.max_attempts == 3 # Set up returns from z-stack @@ -417,9 +387,7 @@ def setup_and_run_smart_z_stack( is a list, it will be set as a side effect (and should be a list of tuples of results). If it a tuple (or anything else), it is set as a return value. """ - stack_params = histo_scan_workflow.create_smart_stack_params( - autofocus_dz=2000, images_dir="/this/is/fake", save_resolution=(1640, 1232) - ) + stack_params = histo_scan_workflow.create_smart_stack_params() stack_params.settling_time = 0 # Don't settle or tests take forever. autofocus_thing.capture_stack_image = mocker.Mock() From 67e43c90df6d5703ba2297cb025bf0995c544281 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 19 Feb 2026 12:09:10 +0000 Subject: [PATCH 02/17] Update histo_scan fixture with dummy settings --- tests/unit_tests/test_stack.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index fe1a4017..c4154c92 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -249,7 +249,17 @@ def autofocus_thing(): @pytest.fixture def histo_scan_workflow(): """Return an autofocus thing connected to a server.""" - return create_thing_without_server(HistoScanWorkflow, mock_all_slots=True) + workflow = create_thing_without_server( + HistoScanWorkflow, + mock_all_slots=True, + ) + + # Minimal CSM setup so all_settings() works + workflow._csm.image_resolution = (1000, 1000) + workflow._csm.calibration_required = False + workflow._csm.convert_image_to_stage_coordinates = lambda x, y: {"x": x, "y": y} + + return workflow def test_create_stack(histo_scan_workflow, caplog): @@ -325,8 +335,8 @@ def test_coercing_stack_save_ims( @pytest.mark.parametrize("pass_on", [1, 2, 3, 4]) def test_run_smart_stack(pass_on, histo_scan_workflow, autofocus_thing, mocker): """Test Running smart stack with the stack passing on different attempts.""" - stack_params = histo_scan_workflow.create_smart_stack_params() - assert stack_params.max_attempts == 3 + scan_settings, _ = histo_scan_workflow.all_settings(images_dir="dummy") + assert scan_settings.smart_stack_params.max_attempts == 3 # Set up returns from z-stack fake_captures = [ @@ -351,19 +361,21 @@ def test_run_smart_stack(pass_on, histo_scan_workflow, autofocus_thing, mocker): # Run it success, final_z = autofocus_thing.run_smart_stack( - stack_parameters=stack_params, + stack_parameters=scan_settings.smart_stack_params, + capture_parameters=scan_settings.capture_params, + autofocus_parameters=scan_settings.autofocus_params, save_on_failure=False, check_turning_points=True, ) # Only passes if the attempt it passes on is less than max attempts - assert success == (pass_on <= stack_params.max_attempts) + assert success == (pass_on <= scan_settings.smart_stack_params.max_attempts) # Final z is the one from the id returned by the stack "pick_me" assert final_z == 555 # smart_z_stack should run up until the time it passes. Running no more than # max_attempts - n_stacks = min(pass_on, stack_params.max_attempts) + n_stacks = min(pass_on, scan_settings.smart_stack_params.max_attempts) assert autofocus_thing.smart_z_stack.call_count == n_stacks # Move absolute should be 1 less time that the number of times z_stack_run assert autofocus_thing._stage.move_absolute.call_count == n_stacks - 1 From 37e345067018902a9a1cdac078c4e7782d56dd44 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 19 Feb 2026 12:19:31 +0000 Subject: [PATCH 03/17] Move CaptureParams into camera from autofocus --- src/openflexure_microscope_server/things/autofocus.py | 9 +-------- .../things/camera/__init__.py | 8 ++++++++ .../things/scan_workflows.py | 3 +-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index f62416aa..9bb5bb54 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -20,7 +20,7 @@ from pydantic import BaseModel, computed_field, field_validator, model_validator import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray -from .camera import BaseCamera +from .camera import BaseCamera, CaptureParams from .stage import BaseStage LOGGER = logging.getLogger(__name__) @@ -40,13 +40,6 @@ class AutofocusParams(BaseModel): sharpness_method: str = "jpeg" -class CaptureParams(BaseModel): - """A class for capturing at least a single image.""" - - images_dir: str - save_resolution: tuple[int, int] - - class StackParams(BaseModel): """A class for holding stack parameters, and returning computed ones.""" diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index cb599bdc..a9b28811 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -20,6 +20,7 @@ from typing import Any, Literal, Mapping, Optional, Self, Tuple import numpy as np import piexif from PIL import Image +from pydantic import BaseModel import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray @@ -47,6 +48,13 @@ class CaptureError(RuntimeError): """An error trying to capture from a CameraThing.""" +class CaptureParams(BaseModel): + """A class for capturing at least a single image.""" + + images_dir: str + save_resolution: tuple[int, int] + + class NoImageInMemoryError(RuntimeError): """An error called if no image is in memory when accessed.""" diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 42f11a18..4acbbe88 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -33,13 +33,12 @@ from openflexure_microscope_server.things.autofocus import ( MIN_TEST_IMAGE_COUNT, AutofocusParams, AutofocusThing, - CaptureParams, SmartStackParams, ) from openflexure_microscope_server.things.background_detect import ( ChannelDeviationLUV, ) -from openflexure_microscope_server.things.camera import BaseCamera +from openflexure_microscope_server.things.camera import BaseCamera, CaptureParams from openflexure_microscope_server.things.camera_stage_mapping import CameraStageMapper from openflexure_microscope_server.things.stage import BaseStage from openflexure_microscope_server.ui import PropertyControl, property_control_for From 331ceb705d4e170d03b777d1d696ba49bd22cdcf Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 19 Feb 2026 15:05:37 +0000 Subject: [PATCH 04/17] Changes based on review, mostly enums and moved stack params --- .../things/autofocus.py | 67 ++++++++++++++----- .../things/scan_workflows.py | 6 +- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 9bb5bb54..fb542a71 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -11,6 +11,7 @@ import logging import os import time from dataclasses import dataclass +from enum import Enum from types import TracebackType from typing import Literal, Mapping, Optional, Self, Sequence @@ -33,11 +34,25 @@ class NotStreamingError(RuntimeError): """No images captured from stream. The camera is almost certainly not streaming.""" +class SharpnessMethod(str, Enum): + """The possible SharpnessMethods for autofocus.""" + + jpeg = "jpeg" + + class AutofocusParams(BaseModel): """A class for running autofocus routines.""" dz: int - sharpness_method: str = "jpeg" + sharpness_method: SharpnessMethod = SharpnessMethod.jpeg + + +class StackOrigin(str, Enum): + """The position of the current location in the stack.""" + + START = "start" + CENTER = "center" + END = "end" class StackParams(BaseModel): @@ -52,15 +67,28 @@ class StackParams(BaseModel): settling_time: float = 0.3 """Time (in seconds) between moving and capturing an image""" + backlash_correction: int = 250 + """ + Distance (in steps) to overshoot a move and then undo, to account for backlash + """ + + origin: StackOrigin = StackOrigin.START + """Where the stack is positioned relative to the current z position.""" + class SmartStackParams(StackParams): """A class for holding smart stack parameters, and returning computed ones.""" min_images_to_test: int - backlash_correction: int = 250 + save_on_failure: bool = False + """Whether to save an image even if no focus was found.""" + + check_turning_points: bool = True """ - Distance (in steps) to overshoot a move and then undo, to account for backlash + Whether to check the number of turning points in + the sharpnesses of the images in the stack is exactly 1. + (May fail with thick samples) """ stack_height_limit: int = 15 @@ -468,8 +496,6 @@ class AutofocusThing(lt.Thing): stack_parameters: SmartStackParams, capture_parameters: CaptureParams, autofocus_parameters: AutofocusParams, - save_on_failure: bool = False, - check_turning_points: bool = True, ) -> tuple[bool, int]: """Run a smart stack. @@ -481,10 +507,6 @@ class AutofocusThing(lt.Thing): :param stack_parameters: A SmartStackParams object containing the required parameters to run a stack. - :param save_on_failure: Whether to save an image even if no focus was found. - :param check_turning_points: Whether to check the number of turning points in - the sharpnesses of the images in the stack is exactly 1. (May fail with - thick samples) :returns: A tuple containing: @@ -496,13 +518,10 @@ class AutofocusThing(lt.Thing): attempt += 1 success, captures, sharpest_id = self.smart_z_stack( stack_parameters=stack_parameters, - check_turning_points=check_turning_points, + check_turning_points=stack_parameters.check_turning_points, ) - if success: - break - - if attempt >= stack_parameters.max_attempts: + if success or attempt >= stack_parameters.max_attempts: break # The z position of the first images in the previous attempt. @@ -516,7 +535,7 @@ class AutofocusThing(lt.Thing): # Save stack_parameters.image_to_save images centred on the sharpest capture. # If the smart_stack failed the exact number of images saved may not be # stack_parameters.image_to_save - if success or save_on_failure: + if success or stack_parameters.save_on_failure: self.save_stack( sharpest_id=sharpest_id, captures=captures, @@ -747,6 +766,24 @@ class AutofocusThing(lt.Thing): captures: list[CaptureInfo] = [] z_positions: list[int] = [] + total_range = stack_parameters.stack_dz * (stack_parameters.images_to_save - 1) + + # Determine starting offset based on stack origin + if stack_parameters.origin == StackOrigin.CENTER: + target_offset = -total_range // 2 + elif stack_parameters.origin == StackOrigin.END: + target_offset = -total_range + else: + target_offset = 0 + + # Apply backlash correction: overshoot and move back + # Overshoot past the starting position + self._stage.move_relative( + z=target_offset - stack_parameters.backlash_correction + ) + # Move back to the exact starting point + self._stage.move_relative(z=stack_parameters.backlash_correction) + # Capture images_to_save images for _ in range(stack_parameters.images_to_save): time.sleep(stack_parameters.settling_time) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 4acbbe88..f473a99d 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -481,6 +481,7 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): stack_dz=self.stack_dz, images_to_save=self.stack_images_to_save, min_images_to_test=self.stack_min_images_to_test, + save_on_failure=not self.skip_background, ) def pre_scan_routine(self, settings: HistoScanSettingsModel) -> None: @@ -539,17 +540,14 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): self.logger.info(msg) return False, None - save_on_failure = not settings.skip_background - focus_height: Optional[int] focused, focus_height = self._autofocus.run_smart_stack( stack_parameters=settings.smart_stack_params, capture_parameters=settings.capture_params, autofocus_parameters=settings.autofocus_params, - save_on_failure=save_on_failure, ) # An image was captured if we are focussed or we are not skipping background. - imaged = focused or save_on_failure + imaged = focused or settings.smart_stack_params.save_on_failure if not imaged: msg = f"Stack failed at {xyz_pos}. Treating as background." From a5b77cc2a34a81886eeacb31080ede74ce6e38a8 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 19 Feb 2026 15:29:04 +0000 Subject: [PATCH 05/17] Add settings to test_scan_workflows --- tests/unit_tests/test_scan_workflows.py | 14 ++++++++++++++ tests/unit_tests/test_stack.py | 2 -- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_scan_workflows.py b/tests/unit_tests/test_scan_workflows.py index 40e62cc9..50c27cdc 100644 --- a/tests/unit_tests/test_scan_workflows.py +++ b/tests/unit_tests/test_scan_workflows.py @@ -258,6 +258,13 @@ def test_histo_acquisition_not_skipping_background(histo_workflow, mocker, caplo # Mocking for settings as above mock_settings = mocker.Mock() mock_settings.skip_background = False + mock_settings.smart_stack_params = SmartStackParams( + stack_dz=5, + images_to_save=3, + min_images_to_test=3, + save_on_failure=True, + check_turning_points=True, + ) # Set up background detector to say it is empty, this shouldn't stop stacking. histo_workflow._background_detector.image_is_sample.return_value = ( @@ -296,6 +303,13 @@ def test_histo_acquisition_on_sample(histo_workflow, mocker, caplog): # Mocking for settings as above mock_settings = mocker.Mock() mock_settings.skip_background = True + mock_settings.smart_stack_params = SmartStackParams( + stack_dz=5, + images_to_save=3, + min_images_to_test=3, + save_on_failure=False, + check_turning_points=True, + ) # Set up background detector to say there is sample. histo_workflow._background_detector.image_is_sample.return_value = (True, None) diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index c4154c92..4f959bcf 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -364,8 +364,6 @@ def test_run_smart_stack(pass_on, histo_scan_workflow, autofocus_thing, mocker): stack_parameters=scan_settings.smart_stack_params, capture_parameters=scan_settings.capture_params, autofocus_parameters=scan_settings.autofocus_params, - save_on_failure=False, - check_turning_points=True, ) # Only passes if the attempt it passes on is less than max attempts From fb40af915a499ee79eddf53608a3823cf951a325 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 19 Feb 2026 16:38:29 +0000 Subject: [PATCH 06/17] Switch to auto enumerating enums --- .../things/autofocus.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index fb542a71..651226db 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -7,11 +7,11 @@ of images (a 'z-stack'). See repository root for licensing information. """ +import enum import logging import os import time from dataclasses import dataclass -from enum import Enum from types import TracebackType from typing import Literal, Mapping, Optional, Self, Sequence @@ -34,25 +34,25 @@ class NotStreamingError(RuntimeError): """No images captured from stream. The camera is almost certainly not streaming.""" -class SharpnessMethod(str, Enum): +class SharpnessMethod(enum.Enum): """The possible SharpnessMethods for autofocus.""" - jpeg = "jpeg" + JPEG = enum.auto() class AutofocusParams(BaseModel): """A class for running autofocus routines.""" dz: int - sharpness_method: SharpnessMethod = SharpnessMethod.jpeg + sharpness_method: SharpnessMethod = SharpnessMethod.JPEG -class StackOrigin(str, Enum): +class StackOrigin(enum.Enum): """The position of the current location in the stack.""" - START = "start" - CENTER = "center" - END = "end" + START = enum.auto() + CENTER = enum.auto() + END = enum.auto() class StackParams(BaseModel): From 294dd33bc01b5ce919c7d11af7a2d67fbcb46da8 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Mon, 23 Feb 2026 19:27:41 +0000 Subject: [PATCH 07/17] Add tests for basicstack --- tests/unit_tests/test_stack.py | 151 +++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 4f959bcf..8bb40b0a 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -20,6 +20,8 @@ from openflexure_microscope_server.things.autofocus import ( CaptureInfo, NotAPeakError, SmartStackParams, + StackParams, + StackOrigin, _count_turning_points, _get_capture_by_id, _get_capture_index_by_id, @@ -658,3 +660,152 @@ def test_count_turning_points(): assert _count_turning_points(np.array([1, 2, 3, 4, 2, 4, 3, 2, 1])) == 3 # But only one if the dip isn't prominent assert _count_turning_points(np.array([1, 2, 3, 4, 3.8, 4, 3, 2, 1])) == 1 + +@pytest.fixture +def fake_capture(autofocus_thing): + def _fake_capture(*args, **kwargs): + z = autofocus_thing._stage.position["z"] + return CaptureInfo( + buffer_id=f"id_{z}", + position={"x": 0, "y": 0, "z": z}, + sharpness=1, + ) + return _fake_capture + + +@pytest.fixture +def fake_move_relative(autofocus_thing): + def _fake_move_relative(z, **kwargs): + autofocus_thing._stage.position["z"] += z + return _fake_move_relative + +def test_run_basic_stack_simple(autofocus_thing, mocker, fake_capture, fake_move_relative): + """Basic stack captures correct number of images and saves them.""" + stack_params = StackParams( + stack_dz=10, + images_to_save=3, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.START, + ) + + capture_params = mocker.Mock() + capture_params.images_dir = "dummy" + capture_params.save_resolution = (100, 100) + + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + + autofocus_thing.capture_stack_image = mocker.Mock( + side_effect=fake_capture + ) + + autofocus_thing._stage.move_relative = mocker.Mock( + side_effect=fake_move_relative + ) + + autofocus_thing._cam.save_from_memory = mocker.Mock() + autofocus_thing._cam.clear_buffers = mocker.Mock() + + final_z, z_positions = autofocus_thing.run_basic_stack( + stack_parameters=stack_params, + capture_parameters=capture_params, + ) + + assert z_positions == [0, 10, 20] + assert final_z == 30 + +def test_run_basic_stack_center_origin(autofocus_thing, mocker): + """Stack should shift start position when origin is CENTER.""" + + stack_params = StackParams( + stack_dz=10, + images_to_save=5, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.CENTER, + ) + + capture_params = mocker.Mock() + capture_params.images_dir = "dummy" + capture_params.save_resolution = (100, 100) + + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 100} + + autofocus_thing.capture_stack_image = mocker.Mock( + return_value=CaptureInfo("id", {"x": 0, "y": 0, "z": 0}, 1) + ) + + autofocus_thing._cam.save_from_memory = mocker.Mock() + autofocus_thing._cam.clear_buffers = mocker.Mock() + autofocus_thing._stage.move_relative = mocker.Mock() + + autofocus_thing.run_basic_stack(stack_params, capture_params) + + total_range = 10 * (5 - 1) # 40 + expected_offset = -total_range // 2 # -20 + + # First move should be to CENTER offset (with backlash logic) + first_call = autofocus_thing._stage.move_relative.call_args_list[0] + assert first_call.kwargs["z"] == expected_offset + +def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture): + """Backlash correction should overshoot and move back.""" + + stack_params = StackParams( + stack_dz=10, + images_to_save=3, + settling_time=0, + backlash_correction=50, + origin=StackOrigin.START, + ) + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + capture_params = mocker.Mock() + capture_params.images_dir = "dummy" + capture_params.save_resolution = (100, 100) + + autofocus_thing.capture_stack_image = mocker.Mock( + side_effect=fake_capture + ) + + autofocus_thing._stage.move_relative = mocker.Mock() + autofocus_thing._cam.save_from_memory = mocker.Mock() + autofocus_thing._cam.clear_buffers = mocker.Mock() + + autofocus_thing.run_basic_stack(stack_params, capture_params) + + calls = autofocus_thing._stage.move_relative.call_args_list + + # First call = overshoot + assert calls[0].kwargs["z"] == -50 + # Second call = move back + assert calls[1].kwargs["z"] == 50 + +def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): + """END origin should shift stack fully negative before starting.""" + + stack_params = StackParams( + stack_dz=10, + images_to_save=4, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.END, + ) + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + capture_params = mocker.Mock() + capture_params.images_dir = "dummy" + capture_params.save_resolution = (100, 100) + + autofocus_thing.capture_stack_image = mocker.Mock( + side_effect=fake_capture + ) + + autofocus_thing._stage.move_relative = mocker.Mock() + autofocus_thing._cam.save_from_memory = mocker.Mock() + autofocus_thing._cam.clear_buffers = mocker.Mock() + + autofocus_thing.run_basic_stack(stack_params, capture_params) + + total_range = 10 * (4 - 1) # 30 + + first_call = autofocus_thing._stage.move_relative.call_args_list[0] + assert first_call.kwargs["z"] == -total_range \ No newline at end of file From c28afe30ab4cabbfa17badb8b24c33d023bbce9d Mon Sep 17 00:00:00 2001 From: jaknapper Date: Tue, 24 Feb 2026 11:46:19 +0000 Subject: [PATCH 08/17] Optional args, kwargs in pytest fixtures --- tests/unit_tests/test_stack.py | 42 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 8bb40b0a..349424fb 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -20,8 +20,8 @@ from openflexure_microscope_server.things.autofocus import ( CaptureInfo, NotAPeakError, SmartStackParams, - StackParams, StackOrigin, + StackParams, _count_turning_points, _get_capture_by_id, _get_capture_index_by_id, @@ -661,25 +661,35 @@ def test_count_turning_points(): # But only one if the dip isn't prominent assert _count_turning_points(np.array([1, 2, 3, 4, 3.8, 4, 3, 2, 1])) == 1 + @pytest.fixture def fake_capture(autofocus_thing): - def _fake_capture(*args, **kwargs): + """Return a fake capture function using the current stage Z position.""" + + def _fake_capture(*_args, **_kwargs): z = autofocus_thing._stage.position["z"] return CaptureInfo( buffer_id=f"id_{z}", position={"x": 0, "y": 0, "z": z}, sharpness=1, ) + return _fake_capture @pytest.fixture def fake_move_relative(autofocus_thing): - def _fake_move_relative(z, **kwargs): + """Return a fake move_relative function updating the stage Z position.""" + + def _fake_move_relative(z, **_kwargs): autofocus_thing._stage.position["z"] += z + return _fake_move_relative -def test_run_basic_stack_simple(autofocus_thing, mocker, fake_capture, fake_move_relative): + +def test_run_basic_stack_simple( + autofocus_thing, mocker, fake_capture, fake_move_relative +): """Basic stack captures correct number of images and saves them.""" stack_params = StackParams( stack_dz=10, @@ -695,13 +705,9 @@ def test_run_basic_stack_simple(autofocus_thing, mocker, fake_capture, fake_move autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} - autofocus_thing.capture_stack_image = mocker.Mock( - side_effect=fake_capture - ) + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock( - side_effect=fake_move_relative - ) + autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() @@ -714,9 +720,9 @@ def test_run_basic_stack_simple(autofocus_thing, mocker, fake_capture, fake_move assert z_positions == [0, 10, 20] assert final_z == 30 + def test_run_basic_stack_center_origin(autofocus_thing, mocker): """Stack should shift start position when origin is CENTER.""" - stack_params = StackParams( stack_dz=10, images_to_save=5, @@ -748,9 +754,9 @@ def test_run_basic_stack_center_origin(autofocus_thing, mocker): first_call = autofocus_thing._stage.move_relative.call_args_list[0] assert first_call.kwargs["z"] == expected_offset + def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture): """Backlash correction should overshoot and move back.""" - stack_params = StackParams( stack_dz=10, images_to_save=3, @@ -763,9 +769,7 @@ def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture) capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) - autofocus_thing.capture_stack_image = mocker.Mock( - side_effect=fake_capture - ) + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) autofocus_thing._stage.move_relative = mocker.Mock() autofocus_thing._cam.save_from_memory = mocker.Mock() @@ -780,9 +784,9 @@ def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture) # Second call = move back assert calls[1].kwargs["z"] == 50 + def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): """END origin should shift stack fully negative before starting.""" - stack_params = StackParams( stack_dz=10, images_to_save=4, @@ -795,9 +799,7 @@ def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) - autofocus_thing.capture_stack_image = mocker.Mock( - side_effect=fake_capture - ) + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) autofocus_thing._stage.move_relative = mocker.Mock() autofocus_thing._cam.save_from_memory = mocker.Mock() @@ -808,4 +810,4 @@ def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): total_range = 10 * (4 - 1) # 30 first_call = autofocus_thing._stage.move_relative.call_args_list[0] - assert first_call.kwargs["z"] == -total_range \ No newline at end of file + assert first_call.kwargs["z"] == -total_range From 565204da59ce3788d94293e2b982ab452e05dedc Mon Sep 17 00:00:00 2001 From: jaknapper Date: Tue, 24 Feb 2026 12:35:41 +0000 Subject: [PATCH 09/17] Remove unneeded moves from basic stack Update tests with better comments Test a stack with both backlash compensation and offset --- .../things/autofocus.py | 19 ++- tests/unit_tests/test_stack.py | 145 ++++++++++++++---- 2 files changed, 129 insertions(+), 35 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 651226db..01328a90 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -777,15 +777,16 @@ class AutofocusThing(lt.Thing): target_offset = 0 # Apply backlash correction: overshoot and move back - # Overshoot past the starting position - self._stage.move_relative( - z=target_offset - stack_parameters.backlash_correction - ) - # Move back to the exact starting point - self._stage.move_relative(z=stack_parameters.backlash_correction) + overshoot = target_offset - stack_parameters.backlash_correction + if overshoot != 0: + self._stage.move_relative(z=overshoot) + + # Move back to starting point if needed + if stack_parameters.backlash_correction != 0: + self._stage.move_relative(z=stack_parameters.backlash_correction) # Capture images_to_save images - for _ in range(stack_parameters.images_to_save): + for move_count in range(stack_parameters.images_to_save): time.sleep(stack_parameters.settling_time) capture = self.capture_stack_image( @@ -794,7 +795,9 @@ class AutofocusThing(lt.Thing): captures.append(capture) z_positions.append(capture.position["z"]) - self._stage.move_relative(z=stack_parameters.stack_dz) + # Only move stage if not on the last image + if move_count < stack_parameters.images_to_save - 1: + self._stage.move_relative(z=stack_parameters.stack_dz) # Save all captures for capture in captures: diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 349424fb..0c5869f5 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -690,7 +690,8 @@ def fake_move_relative(autofocus_thing): def test_run_basic_stack_simple( autofocus_thing, mocker, fake_capture, fake_move_relative ): - """Basic stack captures correct number of images and saves them.""" + """Basic stack captures the correct number of images at correct Z positions.""" + # Stack parameters: small 3-image stack, 10-step spacing stack_params = StackParams( stack_dz=10, images_to_save=3, @@ -699,16 +700,17 @@ def test_run_basic_stack_simple( origin=StackOrigin.START, ) + # Capture parameters for the test capture_params = mocker.Mock() capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) + # Reset stage position autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + # Patch capture and stage movement autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) - autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() @@ -717,12 +719,36 @@ def test_run_basic_stack_simple( capture_parameters=capture_params, ) - assert z_positions == [0, 10, 20] - assert final_z == 30 + # Expect 3 captures from the stack at Z = 0, 10, 20 + assert z_positions == [0, 10, 20], ( + "Z positions captured do not match expected values" + ) + assert final_z == 20, "Final Z position is incorrect" + + # Check that capture_stack_image was called exactly 3 times + assert autofocus_thing.capture_stack_image.call_count == 3, ( + "Incorrect number of captures" + ) + + # Check that stage moved correctly (should match z_positions relative increments) + moves = [ + call.kwargs["z"] for call in autofocus_thing._stage.move_relative.call_args_list + ] + # Two moves in the stack, no other movement as stack began from the starting position + expected_moves = [10, 10] + assert moves == expected_moves, ( + "Stage move_relative calls do not match expected increments" + ) -def test_run_basic_stack_center_origin(autofocus_thing, mocker): - """Stack should shift start position when origin is CENTER.""" +def test_run_basic_stack_center_origin( + autofocus_thing, mocker, fake_capture, fake_move_relative +): + """Stack should shift start position when origin is CENTER. + + CENTER should cause the stage to move down by half the z range before + the stack begins. + """ stack_params = StackParams( stack_dz=10, images_to_save=5, @@ -735,28 +761,39 @@ def test_run_basic_stack_center_origin(autofocus_thing, mocker): capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 100} + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} - autofocus_thing.capture_stack_image = mocker.Mock( - return_value=CaptureInfo("id", {"x": 0, "y": 0, "z": 0}, 1) - ) + # Patch capture and stage movement + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) + autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() - autofocus_thing._stage.move_relative = mocker.Mock() - autofocus_thing.run_basic_stack(stack_params, capture_params) + # Run stack + final_z, z_positions = autofocus_thing.run_basic_stack( + stack_parameters=stack_params, + capture_parameters=capture_params, + ) - total_range = 10 * (5 - 1) # 40 - expected_offset = -total_range // 2 # -20 + # Calculate expected starting offset - moving down by half the stack height + total_range = 10 * (5 - 1) + expected_offset = -total_range // 2 - # First move should be to CENTER offset (with backlash logic) + # First move should apply center offset first_call = autofocus_thing._stage.move_relative.call_args_list[0] - assert first_call.kwargs["z"] == expected_offset + assert first_call.kwargs["z"] == expected_offset, ( + "Center origin offset not applied correctly" + ) + # Check 5 images captured, at expected heights + assert z_positions == [-20, -10, 0, 10, 20], ( + "Z positions captured do not match expected values" + ) + assert final_z == 20, "Final Z position is incorrect" def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture): - """Backlash correction should overshoot and move back.""" + """Backlash correction should overshoot first, then move back before starting stack.""" stack_params = StackParams( stack_dz=10, images_to_save=3, @@ -765,12 +802,12 @@ def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture) origin=StackOrigin.START, ) autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + capture_params = mocker.Mock() capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock() autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() @@ -779,14 +816,54 @@ def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture) calls = autofocus_thing._stage.move_relative.call_args_list - # First call = overshoot - assert calls[0].kwargs["z"] == -50 - # Second call = move back - assert calls[1].kwargs["z"] == 50 + # First move is overshoot for backlash + assert calls[0].kwargs["z"] == -50, "Backlash overshoot not applied correctly" + + # Second move corrects back to start + assert calls[1].kwargs["z"] == 50, "Backlash correction move not applied correctly" + + +def test_run_basic_stack_backlash_and_offset(autofocus_thing, mocker, fake_capture): + """Backlash correction and stack offset both applied. + + Stack should begin by moving down by half the height of the stack, + plus backlash correction, then move up by backlash correction, then run the basic stack. + """ + stack_params = StackParams( + stack_dz=100, + images_to_save=3, + settling_time=0, + backlash_correction=50, + origin=StackOrigin.CENTER, + ) + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + + capture_params = mocker.Mock() + capture_params.images_dir = "dummy" + capture_params.save_resolution = (100, 100) + + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) + autofocus_thing._stage.move_relative = mocker.Mock() + autofocus_thing._cam.save_from_memory = mocker.Mock() + autofocus_thing._cam.clear_buffers = mocker.Mock() + + # Run stack + autofocus_thing.run_basic_stack(stack_params, capture_params) + + calls = autofocus_thing._stage.move_relative.call_args_list + + # First move is overshoot for backlash and stack offset + assert calls[0].kwargs["z"] == -150, "Combined overshoot not applied correctly" + + # Second move corrects back to base of stack + assert calls[1].kwargs["z"] == 50, "Backlash correction move not applied correctly" + + # Third move is the first move in the stack + assert calls[2].kwargs["z"] == 100, "Backlash correction move not applied correctly" def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): - """END origin should shift stack fully negative before starting.""" + """END origin should shift stack down by full stack height before starting.""" stack_params = StackParams( stack_dz=10, images_to_save=4, @@ -795,19 +872,33 @@ def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): origin=StackOrigin.END, ) autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + capture_params = mocker.Mock() capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock() autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() autofocus_thing.run_basic_stack(stack_params, capture_params) - total_range = 10 * (4 - 1) # 30 + # Calculate the stack offset based on StackOrigin.END - moving down by the full stack height + total_range = 10 * (4 - 1) + # First move should shift fully down first_call = autofocus_thing._stage.move_relative.call_args_list[0] - assert first_call.kwargs["z"] == -total_range + assert first_call.kwargs["z"] == -total_range, ( + "End origin offset not applied correctly" + ) + + # Check number of captures + assert autofocus_thing.capture_stack_image.call_count == 4, ( + "Incorrect number of captures for END origin" + ) + + # Check final Z is equal to starting Z, as StackOrigin.END should finish at the starting height + final_z = autofocus_thing._stage.position["z"] + expected_final_z = 0 + assert final_z == expected_final_z, "Final Z position for END origin incorrect" From 78b407fb43d74f3d0bbe4d9f0e595f2ffda57386 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Tue, 24 Feb 2026 13:46:24 +0000 Subject: [PATCH 10/17] Apply suggestions from code review of branch scan-params Co-authored-by: Beth Probert --- tests/unit_tests/test_stack.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 0c5869f5..70ea6077 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -735,7 +735,7 @@ def test_run_basic_stack_simple( call.kwargs["z"] for call in autofocus_thing._stage.move_relative.call_args_list ] # Two moves in the stack, no other movement as stack began from the starting position - expected_moves = [10, 10] + expected_moves = [stack_params.stack_dz, stack_params.stack_dz] assert moves == expected_moves, ( "Stage move_relative calls do not match expected increments" ) @@ -777,7 +777,7 @@ def test_run_basic_stack_center_origin( ) # Calculate expected starting offset - moving down by half the stack height - total_range = 10 * (5 - 1) + total_range = stack_params.stack_dz * (stack_params.images_to_save - 1) expected_offset = -total_range // 2 # First move should apply center offset @@ -885,7 +885,7 @@ def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): autofocus_thing.run_basic_stack(stack_params, capture_params) # Calculate the stack offset based on StackOrigin.END - moving down by the full stack height - total_range = 10 * (4 - 1) + total_range = stack_params.stack_dz * (stack_params.images_to_save - 1) # First move should shift fully down first_call = autofocus_thing._stage.move_relative.call_args_list[0] From b211cfd4c5e766c2faa5947aaa914ac2ad076e71 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Tue, 24 Feb 2026 14:55:46 +0000 Subject: [PATCH 11/17] Validate and test stack inputs Update picamera hardware test --- .../things/autofocus.py | 33 ++- .../things/camera/__init__.py | 31 +++ tests/unit_tests/test_stack.py | 207 ++++++++++++++---- 3 files changed, 223 insertions(+), 48 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 01328a90..f0c4fce4 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -21,7 +21,7 @@ from pydantic import BaseModel, computed_field, field_validator, model_validator import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray -from .camera import BaseCamera, CaptureParams +from .camera import BaseCamera, CaptureParams, validate_capture_params from .stage import BaseStage LOGGER = logging.getLogger(__name__) @@ -756,13 +756,19 @@ class AutofocusThing(lt.Thing): saving all images captured. No sharpness testing, restart logic, or autofocus is performed. - :param stack_parameters: SmartStackParams defining stack spacing, - image count, directory, and save resolution. + :param stack_parameters: StackParams defining stack spacing, + image count and backlash correction. + :param capture_parameters: CaptureParams defining save + resolution, images directory. :returns: - Final z position - List of z positions captured """ + # Validate parameters and raise Exception if unsuitable + validate_stack_params(stack_parameters=stack_parameters) + validate_capture_params(capture_parameters=capture_parameters) + captures: list[CaptureInfo] = [] z_positions: list[int] = [] @@ -863,3 +869,24 @@ def _count_turning_points(sharpnesses: np.ndarray) -> int: d_sharpnesses = d_sharpnesses[prominent] # count the sign changes return int(np.sum(d_sharpnesses[1:] * d_sharpnesses[:-1] < 0)) + + +def validate_stack_params(stack_parameters: StackParams) -> None: + """Validate stack parameters for a z-stack acquisition. + + Ensures that the parameters allow a physical stack, without negative or zero + values where they would cause crashes. + + :param stack_parameters: StackParams object containing stacking settings. + + :raises ValueError: If any stack_parameters are found to be unusable. + """ + if stack_parameters.images_to_save <= 0: + raise ValueError( + f"Invalid number of images to save: {stack_parameters.images_to_save}. Must be > 0." + ) + + if stack_parameters.settling_time < 0: + raise ValueError( + f"Invalid settling time: {stack_parameters.settling_time}. Must be positive or 0." + ) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index a9b28811..a3cea358 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -55,6 +55,37 @@ class CaptureParams(BaseModel): save_resolution: tuple[int, int] +def validate_capture_params(capture_parameters: CaptureParams) -> None: + """Validate capture parameters for a capture. + + Ensures that the save resolution is a tuple of two positive integers + and that the images directory is a non-empty string. + + :param capture_parameters: CaptureParams object containing acquisition settings. + + :raises ValueError: If `save_resolution` is not a tuple of two positive integers, + or if `images_dir` is not a non-empty string. + """ + if ( + not isinstance(capture_parameters.save_resolution, tuple) + or len(capture_parameters.save_resolution) != 2 + or not all( + isinstance(x, int) and x > 0 for x in capture_parameters.save_resolution + ) + ): + raise ValueError( + f"Invalid save_resolution: {capture_parameters.save_resolution}. Must be a tuple of two positive integers." + ) + + if ( + not isinstance(capture_parameters.images_dir, str) + or not capture_parameters.images_dir + ): + raise ValueError( + f"Invalid images_dir: {capture_parameters.images_dir}. Must be a non-empty string." + ) + + class NoImageInMemoryError(RuntimeError): """An error called if no image is in memory when accessed.""" diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 70ea6077..40db949d 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -706,7 +706,8 @@ def test_run_basic_stack_simple( capture_params.save_resolution = (100, 100) # Reset stage position - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + start_z = 0 + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} # Patch capture and stage movement autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) @@ -719,23 +720,28 @@ def test_run_basic_stack_simple( capture_parameters=capture_params, ) - # Expect 3 captures from the stack at Z = 0, 10, 20 - assert z_positions == [0, 10, 20], ( + # Expected Z positions for the stack + expected_z_positions = [ + start_z + i * stack_params.stack_dz for i in range(stack_params.images_to_save) + ] + assert z_positions == expected_z_positions, ( "Z positions captured do not match expected values" ) - assert final_z == 20, "Final Z position is incorrect" - # Check that capture_stack_image was called exactly 3 times - assert autofocus_thing.capture_stack_image.call_count == 3, ( - "Incorrect number of captures" - ) + # Final Z should be the last captured Z + expected_final_z = expected_z_positions[-1] + assert final_z == expected_final_z, "Final Z position is incorrect" - # Check that stage moved correctly (should match z_positions relative increments) + # Check that capture_stack_image was called exactly images_to_save times + assert ( + autofocus_thing.capture_stack_image.call_count == stack_params.images_to_save + ), "Incorrect number of captures" + + # Check that stage moved correctly (should match relative increments) moves = [ call.kwargs["z"] for call in autofocus_thing._stage.move_relative.call_args_list ] - # Two moves in the stack, no other movement as stack began from the starting position - expected_moves = [stack_params.stack_dz, stack_params.stack_dz] + expected_moves = [stack_params.stack_dz] * (stack_params.images_to_save - 1) assert moves == expected_moves, ( "Stage move_relative calls do not match expected increments" ) @@ -761,12 +767,12 @@ def test_run_basic_stack_center_origin( capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} + start_z = 0 + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} # Patch capture and stage movement autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) - autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() @@ -776,7 +782,7 @@ def test_run_basic_stack_center_origin( capture_parameters=capture_params, ) - # Calculate expected starting offset - moving down by half the stack height + # Calculate expected starting offset - half of stack range total_range = stack_params.stack_dz * (stack_params.images_to_save - 1) expected_offset = -total_range // 2 @@ -785,14 +791,24 @@ def test_run_basic_stack_center_origin( assert first_call.kwargs["z"] == expected_offset, ( "Center origin offset not applied correctly" ) - # Check 5 images captured, at expected heights - assert z_positions == [-20, -10, 0, 10, 20], ( + + # Expected Z positions after CENTER offset + expected_z_positions = [ + expected_offset + i * stack_params.stack_dz + for i in range(stack_params.images_to_save) + ] + assert z_positions == expected_z_positions, ( "Z positions captured do not match expected values" ) - assert final_z == 20, "Final Z position is incorrect" + + # Final Z should be last captured Z + expected_final_z = expected_z_positions[-1] + assert final_z == expected_final_z, "Final Z position is incorrect" -def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture): +def test_run_basic_stack_backlash_applied( + autofocus_thing, mocker, fake_capture, fake_move_relative +): """Backlash correction should overshoot first, then move back before starting stack.""" stack_params = StackParams( stack_dz=10, @@ -801,14 +817,16 @@ def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture) backlash_correction=50, origin=StackOrigin.START, ) - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} capture_params = mocker.Mock() capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) + start_z = 0 + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock() + autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() @@ -817,13 +835,19 @@ def test_run_basic_stack_backlash_applied(autofocus_thing, mocker, fake_capture) calls = autofocus_thing._stage.move_relative.call_args_list # First move is overshoot for backlash - assert calls[0].kwargs["z"] == -50, "Backlash overshoot not applied correctly" + assert calls[0].kwargs["z"] == -stack_params.backlash_correction, ( + "Backlash overshoot not applied correctly" + ) - # Second move corrects back to start - assert calls[1].kwargs["z"] == 50, "Backlash correction move not applied correctly" + # Second move corrects back to starting point + assert calls[1].kwargs["z"] == stack_params.backlash_correction, ( + "Backlash correction move not applied correctly" + ) -def test_run_basic_stack_backlash_and_offset(autofocus_thing, mocker, fake_capture): +def test_run_basic_stack_backlash_and_offset( + autofocus_thing, mocker, fake_capture, fake_move_relative +): """Backlash correction and stack offset both applied. Stack should begin by moving down by half the height of the stack, @@ -836,33 +860,46 @@ def test_run_basic_stack_backlash_and_offset(autofocus_thing, mocker, fake_captu backlash_correction=50, origin=StackOrigin.CENTER, ) - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} capture_params = mocker.Mock() capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) + start_z = 0 + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock() + autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() - # Run stack autofocus_thing.run_basic_stack(stack_params, capture_params) calls = autofocus_thing._stage.move_relative.call_args_list - # First move is overshoot for backlash and stack offset - assert calls[0].kwargs["z"] == -150, "Combined overshoot not applied correctly" + # Combined initial offset + backlash overshoot + total_stack_range = stack_params.stack_dz * (stack_params.images_to_save - 1) + expected_first_move = -(total_stack_range // 2 + stack_params.backlash_correction) + assert calls[0].kwargs["z"] == expected_first_move, ( + "Combined overshoot not applied correctly" + ) - # Second move corrects back to base of stack - assert calls[1].kwargs["z"] == 50, "Backlash correction move not applied correctly" + # Backlash correction back to base of stack + assert calls[1].kwargs["z"] == stack_params.backlash_correction, ( + "Backlash correction move not applied correctly" + ) - # Third move is the first move in the stack - assert calls[2].kwargs["z"] == 100, "Backlash correction move not applied correctly" + # Subsequent moves in stack + expected_stack_moves = [stack_params.stack_dz] * (stack_params.images_to_save - 1) + actual_stack_moves = [c.kwargs["z"] for c in calls[2:]] + assert actual_stack_moves == expected_stack_moves, ( + "Stack moves after backlash/offset not correct" + ) -def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): +def test_run_basic_stack_end_origin( + autofocus_thing, mocker, fake_capture, fake_move_relative +): """END origin should shift stack down by full stack height before starting.""" stack_params = StackParams( stack_dz=10, @@ -871,34 +908,114 @@ def test_run_basic_stack_end_origin(autofocus_thing, mocker, fake_capture): backlash_correction=0, origin=StackOrigin.END, ) - autofocus_thing._stage.position = {"x": 0, "y": 0, "z": 0} capture_params = mocker.Mock() capture_params.images_dir = "dummy" capture_params.save_resolution = (100, 100) + start_z = 0 + autofocus_thing._stage.position = {"x": 0, "y": 0, "z": start_z} + autofocus_thing.capture_stack_image = mocker.Mock(side_effect=fake_capture) - autofocus_thing._stage.move_relative = mocker.Mock() + autofocus_thing._stage.move_relative = mocker.Mock(side_effect=fake_move_relative) autofocus_thing._cam.save_from_memory = mocker.Mock() autofocus_thing._cam.clear_buffers = mocker.Mock() autofocus_thing.run_basic_stack(stack_params, capture_params) - # Calculate the stack offset based on StackOrigin.END - moving down by the full stack height + # Calculate the stack offset based on StackOrigin.END total_range = stack_params.stack_dz * (stack_params.images_to_save - 1) - - # First move should shift fully down + expected_first_move = -total_range first_call = autofocus_thing._stage.move_relative.call_args_list[0] - assert first_call.kwargs["z"] == -total_range, ( + assert first_call.kwargs["z"] == expected_first_move, ( "End origin offset not applied correctly" ) # Check number of captures - assert autofocus_thing.capture_stack_image.call_count == 4, ( - "Incorrect number of captures for END origin" - ) + assert ( + autofocus_thing.capture_stack_image.call_count == stack_params.images_to_save + ), "Incorrect number of captures for END origin" - # Check final Z is equal to starting Z, as StackOrigin.END should finish at the starting height + # Check final Z is equal to starting Z final_z = autofocus_thing._stage.position["z"] - expected_final_z = 0 + expected_final_z = start_z assert final_z == expected_final_z, "Final Z position for END origin incorrect" + + +def test_invalid_stack_images_raises(autofocus_thing, mocker): + """Test basic stack raises expected error for negative or zero image count.""" + for capture_count in [-3, 0]: + stack_params = StackParams( + stack_dz=10, + images_to_save=capture_count, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.START, + ) + capture_params = mocker.Mock() + + with pytest.raises(ValueError, match="Invalid number of images to save"): + autofocus_thing.run_basic_stack(stack_params, capture_params) + + +def test_invalid_stack_settling_raises(autofocus_thing, mocker): + """Test basic stack raises expected error for negative settling time.""" + stack_params = StackParams( + stack_dz=10, + images_to_save=1, + settling_time=-10, + backlash_correction=0, + origin=StackOrigin.START, + ) + capture_params = mocker.Mock() + + with pytest.raises(ValueError, match="Invalid settling time"): + autofocus_thing.run_basic_stack(stack_params, capture_params) + + +def test_invalid_capture_dir_raises(autofocus_thing, mocker): + """Test basic stack raises expected error for bad image dir paths.""" + for bad_path in ["", None, 67, ["path"]]: + stack_params = StackParams( + stack_dz=10, + images_to_save=1, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.START, + ) + capture_params = mocker.Mock() + capture_params.images_dir = bad_path + capture_params.save_resolution = (100, 100) + mocker.patch.object(autofocus_thing._cam, "save_from_memory") + mocker.patch.object(autofocus_thing._cam, "clear_buffers") + + with pytest.raises(ValueError, match="Invalid images_dir"): + autofocus_thing.run_basic_stack(stack_params, capture_params) + + +def test_invalid_capture_res_raises(autofocus_thing, mocker): + """Test basic stack raises expected error for invalid save resolutions.""" + for bad_res in [ + "", + None, + 67, + ["path"], + (-100, 50), + (20, 0), + (20, 20, 20), + [30, 30], + ]: + stack_params = StackParams( + stack_dz=10, + images_to_save=1, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.START, + ) + capture_params = mocker.Mock() + capture_params.images_dir = "dummy" + capture_params.save_resolution = bad_res + autofocus_thing.capture_stack_image = mocker.Mock() + + with pytest.raises(ValueError, match="Invalid save_resolution:"): + autofocus_thing.run_basic_stack(stack_params, capture_params) From 80760a7d8fcb78acaf26cc7ac243de60aac3b1ab Mon Sep 17 00:00:00 2001 From: jaknapper Date: Tue, 24 Feb 2026 16:49:43 +0000 Subject: [PATCH 12/17] Fix docstring --- .../things/scan_workflows.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index f473a99d..20a1cc65 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -245,11 +245,9 @@ class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType] ) -> tuple[SettingModelType, Optional[StitchingSettings]]: """Return the scan settings and the stitching settings. - - `images_dir` is used to create the CaptureParams stored in the settings model. - - The returned SettingModelType now contains param objects: - * capture_params: CaptureParams - * autofocus_params: AutofocusParams - * stack_params: StackParams (if relevant) + :param images_dir: The directory that images are to be written to. + :return: A tuple containing the settings model for this workflow and the + settings model for stitching. """ stitching_settings = self._get_stitching_settings_model() dx, dy = self._calc_displacement_from_overlap(self.overlap) From 43fa793698290d2e731a228b5a3921000a6a8b67 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 25 Feb 2026 11:51:54 +0000 Subject: [PATCH 13/17] Apply suggestions from code review of branch scan-params Co-authored-by: Julian Stirling --- src/openflexure_microscope_server/things/camera/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index a3cea358..e9dc75bc 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -74,7 +74,8 @@ def validate_capture_params(capture_parameters: CaptureParams) -> None: ) ): raise ValueError( - f"Invalid save_resolution: {capture_parameters.save_resolution}. Must be a tuple of two positive integers." + f"Invalid save_resolution: {capture_parameters.save_resolution}. Must " + "be a tuple of two positive integers." ) if ( From 63338f8ea0c0359a26a624066b146f751d04fef2 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 25 Feb 2026 13:19:38 +0000 Subject: [PATCH 14/17] From feedback --- .../things/autofocus.py | 43 +++---- .../things/camera/__init__.py | 53 ++++---- .../things/scan_workflows.py | 90 ++++---------- tests/unit_tests/test_stack.py | 116 ++++++++---------- 4 files changed, 111 insertions(+), 191 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index f0c4fce4..5d3c68ec 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -21,7 +21,7 @@ from pydantic import BaseModel, computed_field, field_validator, model_validator import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray -from .camera import BaseCamera, CaptureParams, validate_capture_params +from .camera import BaseCamera, CaptureParams from .stage import BaseStage LOGGER = logging.getLogger(__name__) @@ -75,6 +75,22 @@ class StackParams(BaseModel): origin: StackOrigin = StackOrigin.START """Where the stack is positioned relative to the current z position.""" + @field_validator("images_to_save") + @classmethod + def validate_images_to_save(cls, value: int) -> int: + """Validate that the number of images to save is greater than zero.""" + if value <= 0: + raise ValueError(f"Invalid number of images to save: {value}. Must be > 0.") + return value + + @field_validator("settling_time") + @classmethod + def validate_settling_time(cls, value: float) -> float: + """Validate that settling time is zero or positive.""" + if value < 0: + raise ValueError(f"Invalid settling time: {value}. Must be positive or 0.") + return value + class SmartStackParams(StackParams): """A class for holding smart stack parameters, and returning computed ones.""" @@ -765,10 +781,6 @@ class AutofocusThing(lt.Thing): - Final z position - List of z positions captured """ - # Validate parameters and raise Exception if unsuitable - validate_stack_params(stack_parameters=stack_parameters) - validate_capture_params(capture_parameters=capture_parameters) - captures: list[CaptureInfo] = [] z_positions: list[int] = [] @@ -869,24 +881,3 @@ def _count_turning_points(sharpnesses: np.ndarray) -> int: d_sharpnesses = d_sharpnesses[prominent] # count the sign changes return int(np.sum(d_sharpnesses[1:] * d_sharpnesses[:-1] < 0)) - - -def validate_stack_params(stack_parameters: StackParams) -> None: - """Validate stack parameters for a z-stack acquisition. - - Ensures that the parameters allow a physical stack, without negative or zero - values where they would cause crashes. - - :param stack_parameters: StackParams object containing stacking settings. - - :raises ValueError: If any stack_parameters are found to be unusable. - """ - if stack_parameters.images_to_save <= 0: - raise ValueError( - f"Invalid number of images to save: {stack_parameters.images_to_save}. Must be > 0." - ) - - if stack_parameters.settling_time < 0: - raise ValueError( - f"Invalid settling time: {stack_parameters.settling_time}. Must be positive or 0." - ) diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index e9dc75bc..bd09f173 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -20,7 +20,7 @@ from typing import Any, Literal, Mapping, Optional, Self, Tuple import numpy as np import piexif from PIL import Image -from pydantic import BaseModel +from pydantic import BaseModel, field_validator import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray @@ -54,37 +54,28 @@ class CaptureParams(BaseModel): images_dir: str save_resolution: tuple[int, int] + @field_validator("save_resolution") + @classmethod + def validate_save_resolution(cls, value: Tuple[int, int]) -> Tuple[int, int]: + """Validate that save_resolution is a tuple with exactly two positive integers.""" + if not isinstance(value, tuple): + raise TypeError("Save resolution should be a tuple") + if len(value) != 2 or any(not isinstance(x, int) or x <= 0 for x in value): + raise ValueError( + f"Invalid save_resolution: {value}. " + "Must be a tuple of two positive integers." + ) + return value -def validate_capture_params(capture_parameters: CaptureParams) -> None: - """Validate capture parameters for a capture. - - Ensures that the save resolution is a tuple of two positive integers - and that the images directory is a non-empty string. - - :param capture_parameters: CaptureParams object containing acquisition settings. - - :raises ValueError: If `save_resolution` is not a tuple of two positive integers, - or if `images_dir` is not a non-empty string. - """ - if ( - not isinstance(capture_parameters.save_resolution, tuple) - or len(capture_parameters.save_resolution) != 2 - or not all( - isinstance(x, int) and x > 0 for x in capture_parameters.save_resolution - ) - ): - raise ValueError( - f"Invalid save_resolution: {capture_parameters.save_resolution}. Must " - "be a tuple of two positive integers." - ) - - if ( - not isinstance(capture_parameters.images_dir, str) - or not capture_parameters.images_dir - ): - raise ValueError( - f"Invalid images_dir: {capture_parameters.images_dir}. Must be a non-empty string." - ) + @field_validator("images_dir") + @classmethod + def validate_images_dir(cls, value: str) -> str: + """Validate that images_dir is a non-empty string.""" + if not isinstance(value, str) or not value: + raise ValueError( + f"Invalid images_dir: {value}. Must be a non-empty string." + ) + return value class NoImageInMemoryError(RuntimeError): diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index 20a1cc65..ef33e1f6 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -240,10 +240,14 @@ class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType] correlation_resize=STITCHING_RESOLUTION[0] / self.save_resolution[0], ) + def _build_scan_settings(self, base_kwargs: dict) -> SettingModelType: + """Construct the _settings_model.""" + return self._settings_model(**base_kwargs) + def all_settings( self, images_dir: str ) -> tuple[SettingModelType, Optional[StitchingSettings]]: - """Return the scan settings and the stitching settings. + """Return scan settings and the stitching settings. :param images_dir: The directory that images are to be written to. :return: A tuple containing the settings model for this workflow and the @@ -252,19 +256,17 @@ class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType] stitching_settings = self._get_stitching_settings_model() dx, dy = self._calc_displacement_from_overlap(self.overlap) - capture_params = CaptureParams( - images_dir=images_dir, save_resolution=self.save_resolution - ) + base_kwargs = { + "overlap": self.overlap, + "dx": dx, + "dy": dy, + "capture_params": CaptureParams( + images_dir=images_dir, save_resolution=self.save_resolution + ), + "autofocus_params": AutofocusParams(dz=self.autofocus_dz), + } - autofocus_params = AutofocusParams(dz=self.autofocus_dz) - - scan_settings = self._settings_model( - overlap=self.overlap, - dx=dx, - dy=dy, - capture_params=capture_params, - autofocus_params=autofocus_params, - ) + scan_settings = self._build_scan_settings(base_kwargs) return scan_settings, stitching_settings @@ -389,38 +391,14 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): return True return self._background_detector.ready - def all_settings( - self, images_dir: str - ) -> tuple[HistoScanSettingsModel, StitchingSettings]: - """Return the workflow settings and stitching settings. - - :param images_dir: The directory that images are to be written to. - :return: A tuple containing the settings model for this workflow and the - settings model for stitching. - """ - stitching_settings = self._get_stitching_settings_model() - dx, dy = self._calc_displacement_from_overlap(self.overlap) - - capture_params = CaptureParams( - images_dir=images_dir, - save_resolution=self.save_resolution, - ) - autofocus_params = AutofocusParams(dz=self.autofocus_dz) - smart_stack_params = self.create_smart_stack_params() - - scan_settings = HistoScanSettingsModel( - overlap=self.overlap, + def _build_scan_settings(self, base_kwargs: dict) -> HistoScanSettingsModel: + return HistoScanSettingsModel( + **base_kwargs, max_dist=self.max_range, - dx=dx, - dy=dy, skip_background=self.skip_background, - capture_params=capture_params, - autofocus_params=autofocus_params, - smart_stack_params=smart_stack_params, + smart_stack_params=self.create_smart_stack_params(), ) - return scan_settings, stitching_settings - def create_smart_stack_params( self, ) -> SmartStackParams: @@ -615,38 +593,14 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): _planner_cls = RegularGridPlanner _grid_style: Literal["snake", "raster"] - def all_settings( - self, images_dir: str - ) -> tuple[RegularGridSettingsModel, Optional[StitchingSettings]]: - """Return the workflow and stitching settings. - - :param images_dir: The directory that images are to be written to. - :return: A tuple containing the settings model for this workflow and the - settings model for stitching. - """ - stitching_settings = self._get_stitching_settings_model() - dx, dy = self._calc_displacement_from_overlap(self.overlap) - - capture_params = CaptureParams( - images_dir=images_dir, - save_resolution=self.save_resolution, - ) - - autofocus_params = AutofocusParams(dz=self.autofocus_dz) - - scan_settings = self._settings_model( - overlap=self.overlap, - dx=dx, - dy=dy, + def _build_scan_settings(self, base_kwargs: dict) -> RegularGridSettingsModel: + return RegularGridSettingsModel( + **base_kwargs, x_count=self.x_count, y_count=self.y_count, style=self._grid_style, - capture_params=capture_params, - autofocus_params=autofocus_params, ) - return scan_settings, stitching_settings - def pre_scan_routine(self, settings: RegularGridSettingsModel) -> None: """Perform these steps before starting the scan. diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 40db949d..8ed3611d 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -27,6 +27,7 @@ from openflexure_microscope_server.things.autofocus import ( _get_capture_index_by_id, _get_peak_turning_point, ) +from openflexure_microscope_server.things.camera import CaptureParams from openflexure_microscope_server.things.scan_workflows import HistoScanWorkflow RANDOM_GENERATOR = np.random.default_rng() @@ -108,7 +109,8 @@ def test_stack_params_negative_images_to_save(save_ims, extra_ims): # Depending on the values multiple messages are possible match = ( "(Can't test for focus with fewer than 3 images|" - "Images to save must be positive and odd)" + "Images to save must be positive and odd)|" + "Invalid number of images to save" ) with pytest.raises(ValueError, match=match): SmartStackParams( @@ -152,7 +154,8 @@ def test_even_images_to_save(save_ims, extra_ims): """ match = ( "(Can't test for focus with fewer than 3 images|" - "Images to save must be positive and odd)" + "Images to save must be positive and odd)|" + "Invalid number of images to save" ) with pytest.raises(ValueError, match=match): SmartStackParams( @@ -942,80 +945,61 @@ def test_run_basic_stack_end_origin( assert final_z == expected_final_z, "Final Z position for END origin incorrect" -def test_invalid_stack_images_raises(autofocus_thing, mocker): +def test_invalid_stack_images_raises(): """Test basic stack raises expected error for negative or zero image count.""" for capture_count in [-3, 0]: - stack_params = StackParams( - stack_dz=10, - images_to_save=capture_count, - settling_time=0, - backlash_correction=0, - origin=StackOrigin.START, - ) - capture_params = mocker.Mock() - with pytest.raises(ValueError, match="Invalid number of images to save"): - autofocus_thing.run_basic_stack(stack_params, capture_params) + StackParams( + stack_dz=10, + images_to_save=capture_count, + settling_time=0, + backlash_correction=0, + origin=StackOrigin.START, + ) -def test_invalid_stack_settling_raises(autofocus_thing, mocker): +def test_invalid_stack_settling_raises(): """Test basic stack raises expected error for negative settling time.""" - stack_params = StackParams( - stack_dz=10, - images_to_save=1, - settling_time=-10, - backlash_correction=0, - origin=StackOrigin.START, - ) - capture_params = mocker.Mock() - with pytest.raises(ValueError, match="Invalid settling time"): - autofocus_thing.run_basic_stack(stack_params, capture_params) + StackParams( + stack_dz=10, + images_to_save=1, + settling_time=-1, + backlash_correction=0, + origin=StackOrigin.START, + ) -def test_invalid_capture_dir_raises(autofocus_thing, mocker): +@pytest.mark.parametrize( + ("bad_path", "match_err"), + [ + ("", "Must be a non-empty string"), + (None, "Input should be a valid string"), + (67, "Input should be a valid string"), + ], +) +def test_invalid_capture_dir_raises(bad_path, match_err): """Test basic stack raises expected error for bad image dir paths.""" - for bad_path in ["", None, 67, ["path"]]: - stack_params = StackParams( - stack_dz=10, - images_to_save=1, - settling_time=0, - backlash_correction=0, - origin=StackOrigin.START, - ) - capture_params = mocker.Mock() - capture_params.images_dir = bad_path - capture_params.save_resolution = (100, 100) - mocker.patch.object(autofocus_thing._cam, "save_from_memory") - mocker.patch.object(autofocus_thing._cam, "clear_buffers") - - with pytest.raises(ValueError, match="Invalid images_dir"): - autofocus_thing.run_basic_stack(stack_params, capture_params) + with pytest.raises(ValueError, match=match_err): + CaptureParams(images_dir=bad_path, save_resolution=(20, 20)) -def test_invalid_capture_res_raises(autofocus_thing, mocker): +@pytest.mark.parametrize( + ("bad_res", "match_err"), + [ + ((-100, 50), "Invalid save_resolution:"), + ((20, 0), "Invalid save_resolution:"), + ("", "Input should be a valid tuple"), + (None, "Input should be a valid tuple"), + (67, "Input should be a valid tuple"), + ( + ["path"], + "Input should be a valid integer, unable to parse string as an integer", + ), + ((20, 20, 20), "Tuple should have at most 2 items"), + ], +) +def test_invalid_capture_res_raises(bad_res, match_err): """Test basic stack raises expected error for invalid save resolutions.""" - for bad_res in [ - "", - None, - 67, - ["path"], - (-100, 50), - (20, 0), - (20, 20, 20), - [30, 30], - ]: - stack_params = StackParams( - stack_dz=10, - images_to_save=1, - settling_time=0, - backlash_correction=0, - origin=StackOrigin.START, - ) - capture_params = mocker.Mock() - capture_params.images_dir = "dummy" - capture_params.save_resolution = bad_res - autofocus_thing.capture_stack_image = mocker.Mock() - - with pytest.raises(ValueError, match="Invalid save_resolution:"): - autofocus_thing.run_basic_stack(stack_params, capture_params) + with pytest.raises(ValueError, match=match_err): + CaptureParams(images_dir="dummy", save_resolution=bad_res) From 2ae4f832bf155670bbe1b965a5c6d372f5e96eb9 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Feb 2026 19:05:24 +0000 Subject: [PATCH 15/17] Simplify setting model validation --- .../things/autofocus.py | 22 ++---------- .../things/camera/__init__.py | 35 +++++-------------- tests/unit_tests/test_stack.py | 18 +++++----- 3 files changed, 20 insertions(+), 55 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 5d3c68ec..0ee8d32b 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -16,7 +16,7 @@ from types import TracebackType from typing import Literal, Mapping, Optional, Self, Sequence import numpy as np -from pydantic import BaseModel, computed_field, field_validator, model_validator +from pydantic import BaseModel, Field, computed_field, field_validator, model_validator import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray @@ -59,12 +59,12 @@ class StackParams(BaseModel): """A class for holding stack parameters, and returning computed ones.""" stack_dz: int - images_to_save: int + images_to_save: int = Field(gt=0) # Using docstrings under variables as this is how pdoc would expect # attributed to be documented - settling_time: float = 0.3 + settling_time: float = Field(default=0.3, ge=0) """Time (in seconds) between moving and capturing an image""" backlash_correction: int = 250 @@ -75,22 +75,6 @@ class StackParams(BaseModel): origin: StackOrigin = StackOrigin.START """Where the stack is positioned relative to the current z position.""" - @field_validator("images_to_save") - @classmethod - def validate_images_to_save(cls, value: int) -> int: - """Validate that the number of images to save is greater than zero.""" - if value <= 0: - raise ValueError(f"Invalid number of images to save: {value}. Must be > 0.") - return value - - @field_validator("settling_time") - @classmethod - def validate_settling_time(cls, value: float) -> float: - """Validate that settling time is zero or positive.""" - if value < 0: - raise ValueError(f"Invalid settling time: {value}. Must be positive or 0.") - return value - class SmartStackParams(StackParams): """A class for holding smart stack parameters, and returning computed ones.""" diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index bd09f173..f656ed1b 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -15,12 +15,12 @@ import tempfile import time from datetime import datetime from types import TracebackType -from typing import Any, Literal, Mapping, Optional, Self, Tuple +from typing import Annotated, Any, Literal, Mapping, Optional, Self, Tuple import numpy as np import piexif from PIL import Image -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, Field import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray @@ -48,34 +48,15 @@ class CaptureError(RuntimeError): """An error trying to capture from a CameraThing.""" +PositiveInt = Annotated[int, Field(ge=1)] +NonEmptyString = Annotated[str, Field(min_length=1)] + + class CaptureParams(BaseModel): """A class for capturing at least a single image.""" - images_dir: str - save_resolution: tuple[int, int] - - @field_validator("save_resolution") - @classmethod - def validate_save_resolution(cls, value: Tuple[int, int]) -> Tuple[int, int]: - """Validate that save_resolution is a tuple with exactly two positive integers.""" - if not isinstance(value, tuple): - raise TypeError("Save resolution should be a tuple") - if len(value) != 2 or any(not isinstance(x, int) or x <= 0 for x in value): - raise ValueError( - f"Invalid save_resolution: {value}. " - "Must be a tuple of two positive integers." - ) - return value - - @field_validator("images_dir") - @classmethod - def validate_images_dir(cls, value: str) -> str: - """Validate that images_dir is a non-empty string.""" - if not isinstance(value, str) or not value: - raise ValueError( - f"Invalid images_dir: {value}. Must be a non-empty string." - ) - return value + images_dir: NonEmptyString + save_resolution: tuple[PositiveInt, PositiveInt] class NoImageInMemoryError(RuntimeError): diff --git a/tests/unit_tests/test_stack.py b/tests/unit_tests/test_stack.py index 8ed3611d..900e58ab 100644 --- a/tests/unit_tests/test_stack.py +++ b/tests/unit_tests/test_stack.py @@ -109,8 +109,8 @@ def test_stack_params_negative_images_to_save(save_ims, extra_ims): # Depending on the values multiple messages are possible match = ( "(Can't test for focus with fewer than 3 images|" - "Images to save must be positive and odd)|" - "Invalid number of images to save" + "Images to save must be positive and odd|" + "Input should be greater than 0)" ) with pytest.raises(ValueError, match=match): SmartStackParams( @@ -154,8 +154,8 @@ def test_even_images_to_save(save_ims, extra_ims): """ match = ( "(Can't test for focus with fewer than 3 images|" - "Images to save must be positive and odd)|" - "Invalid number of images to save" + "Images to save must be positive and odd|" + "Input should be greater than 0)" ) with pytest.raises(ValueError, match=match): SmartStackParams( @@ -948,7 +948,7 @@ def test_run_basic_stack_end_origin( def test_invalid_stack_images_raises(): """Test basic stack raises expected error for negative or zero image count.""" for capture_count in [-3, 0]: - with pytest.raises(ValueError, match="Invalid number of images to save"): + with pytest.raises(ValueError, match="Input should be greater than 0"): StackParams( stack_dz=10, images_to_save=capture_count, @@ -960,7 +960,7 @@ def test_invalid_stack_images_raises(): def test_invalid_stack_settling_raises(): """Test basic stack raises expected error for negative settling time.""" - with pytest.raises(ValueError, match="Invalid settling time"): + with pytest.raises(ValueError, match="Input should be greater than or equal to 0"): StackParams( stack_dz=10, images_to_save=1, @@ -973,7 +973,7 @@ def test_invalid_stack_settling_raises(): @pytest.mark.parametrize( ("bad_path", "match_err"), [ - ("", "Must be a non-empty string"), + ("", "String should have at least 1 character"), (None, "Input should be a valid string"), (67, "Input should be a valid string"), ], @@ -987,8 +987,8 @@ def test_invalid_capture_dir_raises(bad_path, match_err): @pytest.mark.parametrize( ("bad_res", "match_err"), [ - ((-100, 50), "Invalid save_resolution:"), - ((20, 0), "Invalid save_resolution:"), + ((-100, 50), "Input should be greater than or equal to 1"), + ((20, 0), "Input should be greater than or equal to 1"), ("", "Input should be a valid tuple"), (None, "Input should be a valid tuple"), (67, "Input should be a valid tuple"), From 296e044a86e00a8f89cf65a7f948f46eed259527 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Feb 2026 19:06:04 +0000 Subject: [PATCH 16/17] Further subclassing of workflow settings --- .../things/scan_workflows.py | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index ef33e1f6..9bc61846 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -168,7 +168,24 @@ class ScanWorkflow(Generic[SettingModelType], lt.Thing): ) -class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType]): +class RectGridSettingsModel(BaseModel): + """Base setting model for all RectGrid workflows.""" + + overlap: float + dx: int + dy: int + capture_params: CaptureParams + autofocus_params: AutofocusParams + + +RectGridSettingModelType = TypeVar( + "RectGridSettingModelType", bound=RectGridSettingsModel +) + + +class RectGridWorkflow( + ScanWorkflow[RectGridSettingModelType], Generic[RectGridSettingModelType] +): """A generic workflow for any scan that captures images on a rectilinear grid.""" # Redefine _csm Thing Slot, as CSM is required for any RectGridWorkflow @@ -240,19 +257,23 @@ class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType] correlation_resize=STITCHING_RESOLUTION[0] / self.save_resolution[0], ) - def _build_scan_settings(self, base_kwargs: dict) -> SettingModelType: + def _build_scan_settings(self, base_kwargs: dict) -> RectGridSettingModelType: """Construct the _settings_model.""" + # Developer Note: This needs to be overridden if the settings model for this + # class contains extra keys. return self._settings_model(**base_kwargs) def all_settings( self, images_dir: str - ) -> tuple[SettingModelType, Optional[StitchingSettings]]: + ) -> tuple[RectGridSettingModelType, Optional[StitchingSettings]]: """Return scan settings and the stitching settings. :param images_dir: The directory that images are to be written to. :return: A tuple containing the settings model for this workflow and the settings model for stitching. """ + # Developer Note: When subclassing RectGridWorkflow rather than override + # this method first consider overriding _build_scan_settings stitching_settings = self._get_stitching_settings_model() dx, dy = self._calc_displacement_from_overlap(self.overlap) @@ -276,20 +297,15 @@ class RectGridWorkflow(ScanWorkflow[SettingModelType], Generic[SettingModelType] return not self._csm.calibration_required -class HistoScanSettingsModel(BaseModel): +class HistoScanSettingsModel(RectGridSettingsModel): """The settings for a scan with the HistoScanWorkflow. This includes settings calculated when starting. This will be held by smart scan during a scan and serialised to disk. """ - overlap: float - dx: int - dy: int max_dist: int skip_background: bool - capture_params: CaptureParams - autofocus_params: AutofocusParams smart_stack_params: SmartStackParams @@ -392,6 +408,7 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): return self._background_detector.ready def _build_scan_settings(self, base_kwargs: dict) -> HistoScanSettingsModel: + """Construct the SettingModel for all_settings.""" return HistoScanSettingsModel( **base_kwargs, max_dist=self.max_range, @@ -564,21 +581,16 @@ class HistoScanWorkflow(RectGridWorkflow[HistoScanSettingsModel]): ] -class RegularGridSettingsModel(BaseModel): +class RegularGridSettingsModel(RectGridSettingsModel): """The settings for a scan with a regular grid of dx and dy for x_count, y_count steps. This includes settings calculated when starting. This will be held by smart scan during a scan and serialised to disk. """ - overlap: float - dx: int - dy: int x_count: int y_count: int style: Literal["snake", "raster"] - capture_params: CaptureParams - autofocus_params: AutofocusParams class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): @@ -594,6 +606,7 @@ class RegularGridWorkflow(RectGridWorkflow[RegularGridSettingsModel]): _grid_style: Literal["snake", "raster"] def _build_scan_settings(self, base_kwargs: dict) -> RegularGridSettingsModel: + """Construct the SettingModel for all_settings.""" return RegularGridSettingsModel( **base_kwargs, x_count=self.x_count, From 07ad48550b58cb0bd54ce74cbb1078b49f56f414 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 25 Feb 2026 19:24:41 +0000 Subject: [PATCH 17/17] Update picamera tests --- picamera_coverage.zip | Bin 54038 -> 54038 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/picamera_coverage.zip b/picamera_coverage.zip index 0e85922e462387484dbf0913004b72c17b2a4b55..bdf5503ecd885e733ccf8e901643820229d1de0f 100644 GIT binary patch delta 960 zcmbQXjCtBJW}yIYW)=|!5coDLGA44zY~77QN)9$MoIwoy=lG}c2l1=%J>}cPSI8&F zdzyC^Z!)g|&o`c(Jas${Je=HzxjVT%xp}!xb9HlhaB*>-=j`ST+UzJG#K~kXGx?yW z1*Rw`uQj_hxtQaLHulLcJe8G=>@)|ELRtXk{M)t{!eg>0Yd-1V| zvoJJH-Wa1lS;w1~Rg8t9k##bopXTIrZxI$zW`=4oXN5OEiwH!W_T*>Yf-J(!4CNph zp~TXX{IvY!(qg@WN*5s(hDP4WjDGr)Q+377weqBTFMIn3v;0 zC388*9A4zm5}s@r>K-1Nkol-|0n-P{sa7Jd@uRd@zwKX@~Lfh6yV{DaO7lV$S!Ff%Yb5aD29ohWMpV$U}ln-Vwq-SYLH}^YMPW{ fkYs9RX=toe8{o~zB*KiED<}6}vOw5=_L3(6rxg|? delta 878 zcmbQXjCtBJW}yIYW)=|!5J)a^i^-{u=iMlz7g0)d>Y=F-dzrIQbO zYEItZ$-^i$d9SB4qvT{pFDn)aW`_F7jD7}_^S!uO#aS2{*}=SZUVJQKEDVj49it5< zfAHdA6rKFn%aKuJvc0!8i!ek&d-6(eK^7rqhVsb=y)-93_vU34WMOFJoy_Q`KUu>^ zh(&;zp>Xm+FD>56;*!){y@E;`evoYkz4UnFb5j$GON&zV3Mwu6CNp}ePEPRQn0(iV zht+}?Y@ny|W(B(EX=H16a4!Fp*2whZshANsu