From 18e49ba50ce6c36ec1bbefdfd202ecf81cc91733 Mon Sep 17 00:00:00 2001 From: jaknapper Date: Wed, 24 Sep 2025 16:03:26 +0100 Subject: [PATCH 1/7] Test stack params at start of scan main loop --- .../things/autofocus.py | 64 +++++++++++++------ .../things/smart_scan.py | 14 ++-- 2 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 88ad91eb..63cc41e6 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -10,7 +10,7 @@ See repository root for licensing information. from contextlib import contextmanager import logging import time -from typing import Annotated, Mapping, Optional, Sequence, Literal +from typing import Annotated, Mapping, Optional, Sequence, Literal, Type import os from dataclasses import dataclass @@ -44,6 +44,7 @@ class StackParams: autofocus_dz: int, images_dir: str, save_resolution: tuple[int, int], + logger: lt.deps.InvocationLogger, ) -> None: """Initialise the parameters. All arguments are keyword only. @@ -58,7 +59,18 @@ class StackParams: :param autofocus_dz: The number of steps in a full autofocus (when required) :param images_dir: The directory to save images to disk :param save_resolution: The resolution to save the captures to disk with + :param logger: A logger passed from the calling Thing """ + if min_images_to_test < 3: + logger.warning( + "Can't test for focus with fewer than 3 images. Running scan with test images = 3" + ) + min_images_to_test = 3 + elif min_images_to_test > 9: + logger.warning( + "Testing with more than 9 images is likely to focus on the cover slip, or strike the sample. Running scan with test images = 9" + ) + min_images_to_test = 9 if min_images_to_test < images_to_save: raise ValueError("Can't save more images than the minimum number tested") if min_images_to_test % 2 == 0 or min_images_to_test <= 0: @@ -467,7 +479,7 @@ class AutofocusThing(lt.Thing): enough more images may be captured. After new images are captured the number sets the number of images used for checking if focus is central. - Defaults to 9 which balances reliability and speed/ + Defaults to 9 which balances reliability and speed. """ stack_dz = lt.ThingSetting(initial_value=50, model=int) @@ -480,15 +492,40 @@ class AutofocusThing(lt.Thing): * 200 for 20x """ + @lt.thing_action + def set_stack_params( + self, + images_dir: str, + autofocus_dz: int, + save_resolution: tuple[int, int], + logger: lt.deps.InvocationLogger, + ) -> Type[StackParams]: + """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 + :param logger: A logger passed from the calling Thing + + :returns: A StackParams object with the required parameters. + """ + return StackParams( + 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, + logger=logger, + ) + @lt.thing_action def run_smart_stack( self, cam: CameraClient, stage: Stage, sharpness_monitor: SharpnessMonitorDep, - images_dir: str, - autofocus_dz: int, - save_resolution: tuple[int, int], + stack_parameters: Type[StackParams], ) -> tuple[bool, int]: """Run a smart stack. @@ -503,27 +540,14 @@ class AutofocusThing(lt.Thing): :param stage: Stage Dependency supplied by LabThings dependency injection :param sharpness_monitor: Sharpness Monitor Dependency (for focus detection) supplied by LabThings dependency injection - :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 the images should be saved at, the - images will be resampled if this doesn't match the camera's capture - resolution + :param stack_parameters: A StackParams object containing the required + parameters to run a stack. :returns: A tuple containing: * A boolean, True if stack was successfully * The z position of the sharpest image """ - # Set the variables to prevent changes from the GUI or other windows - stack_parameters = StackParams( - 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, - ) - tries = 0 # Loop until a stack is successful while tries < stack_parameters.max_attempts: diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 1483278b..593237b2 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -26,7 +26,7 @@ from openflexure_microscope_server import scan_planners from openflexure_microscope_server import stitching # Things -from .autofocus import AutofocusThing +from .autofocus import AutofocusThing, StackParams from .camera_stage_mapping import CameraStageMapper from .camera import CameraDependency as CameraClient from .stage import StageDependency as StageDep @@ -121,6 +121,7 @@ class SmartScanThing(lt.Thing): self._stage: Optional[StageDep] = None self._cam: Optional[CameraClient] = None self._csm: Optional[CSMDep] = None + self._stack_params: Optional[StackParams] = None self._ongoing_scan: Optional[scan_directories.ScanDirectory] = None self._scan_data: Optional[scan_directories.ScanData] = None @@ -189,6 +190,7 @@ class SmartScanThing(lt.Thing): self._scan_lock.release() # Ensure any PreviewStitcher created cannot be reused. self._preview_stitcher = None + self._stack_params = None # Remove any scan folders containing zero images. self.purge_empty_scans(logger=logger) @@ -363,6 +365,12 @@ class SmartScanThing(lt.Thing): self._cam.start_streaming(main_resolution=(3280, 2464)) self._scan_data = self._collect_scan_data() self._ongoing_scan.save_scan_data(self._scan_data) + self._stack_params = self._autofocus.set_stack_params( + images_dir=self._ongoing_scan.images_dir, + autofocus_dz=self.autofocus_dz, + save_resolution=self._scan_data.save_resolution, + logger=self._scan_logger, + ) self._preview_stitcher = stitching.PreviewStitcher( self._ongoing_scan.images_dir, overlap=self._scan_data.overlap, @@ -451,9 +459,7 @@ class SmartScanThing(lt.Thing): continue focused, focused_height = self._autofocus.run_smart_stack( - images_dir=self._ongoing_scan.images_dir, - autofocus_dz=self._scan_data.autofocus_dz, - save_resolution=self._scan_data.save_resolution, + stack_parameters=self._stack_params ) current_pos_xyz = (new_pos_xyz[0], new_pos_xyz[1], focused_height) From f90e716d8e14216de91186289e4ac4f4245b3c95 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 25 Sep 2025 18:49:50 +0100 Subject: [PATCH 2/7] Stack limits as globals --- .../things/autofocus.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 63cc41e6..39e4e4c4 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -26,6 +26,8 @@ from .camera import CameraDependency as CameraClient from .stage import StageDependency as Stage LOGGER = logging.getLogger(__name__) +MIN_TEST_IMAGE_COUNT = 3 +MAX_TEST_IMAGE_COUNT = 9 class NotStreamingError(RuntimeError): @@ -61,16 +63,16 @@ class StackParams: :param save_resolution: The resolution to save the captures to disk with :param logger: A logger passed from the calling Thing """ - if min_images_to_test < 3: + if min_images_to_test < MIN_TEST_IMAGE_COUNT: logger.warning( - "Can't test for focus with fewer than 3 images. Running scan with test images = 3" + f"Can't test for focus with fewer than {MIN_TEST_IMAGE_COUNT} images. Running scan with test images = {MIN_TEST_IMAGE_COUNT}" ) - min_images_to_test = 3 - elif min_images_to_test > 9: + min_images_to_test = MIN_TEST_IMAGE_COUNT + elif min_images_to_test > MAX_TEST_IMAGE_COUNT: logger.warning( - "Testing with more than 9 images is likely to focus on the cover slip, or strike the sample. Running scan with test images = 9" + f"Testing with more than {MAX_TEST_IMAGE_COUNT} images is likely to focus on the cover slip, or strike the sample. Running scan with test images = {MAX_TEST_IMAGE_COUNT}" ) - min_images_to_test = 9 + min_images_to_test = MAX_TEST_IMAGE_COUNT if min_images_to_test < images_to_save: raise ValueError("Can't save more images than the minimum number tested") if min_images_to_test % 2 == 0 or min_images_to_test <= 0: From 05e0394485d6aee70b787f548aebe552924ee5c3 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Wed, 8 Oct 2025 10:40:01 +0100 Subject: [PATCH 3/7] StackParams as pydantic basemodel with validators --- .../things/autofocus.py | 99 +++++++++---------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 39e4e4c4..59f067bd 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -16,7 +16,7 @@ from dataclasses import dataclass from fastapi import Depends import numpy as np -from pydantic import BaseModel +from pydantic import BaseModel, field_validator, computed_field, model_validator import labthings_fastapi as lt from labthings_fastapi.types.numpy import NDArray @@ -34,60 +34,16 @@ class NotStreamingError(RuntimeError): """No images captured from stream. The camera is almost certainly not streaming.""" -class StackParams: +class StackParams(BaseModel): """A class for holding for stack parameters, and returning computed ones.""" - def __init__( - self, - *, - stack_dz: int, - images_to_save: int, - min_images_to_test: int, - autofocus_dz: int, - images_dir: str, - save_resolution: tuple[int, int], - logger: lt.deps.InvocationLogger, - ) -> None: - """Initialise the parameters. All arguments are keyword only. - - :param stack_dz: The number of motor steps between images - :param images_to_save: The number of images to save to disk - :param min_images_to_test: The minimum number of images in the stack before, - the stack is evaluated for focus. As more images are captured evaluation - of the focus is always evaluated with the same number of images. i.e. if - ``min_images_to_test=9``, then 9 images are captured, if the stack is not - well focused, a 10th image is captured and images 2 to 10 are evaluated - for focus - :param autofocus_dz: The number of steps in a full autofocus (when required) - :param images_dir: The directory to save images to disk - :param save_resolution: The resolution to save the captures to disk with - :param logger: A logger passed from the calling Thing - """ - if min_images_to_test < MIN_TEST_IMAGE_COUNT: - logger.warning( - f"Can't test for focus with fewer than {MIN_TEST_IMAGE_COUNT} images. Running scan with test images = {MIN_TEST_IMAGE_COUNT}" - ) - min_images_to_test = MIN_TEST_IMAGE_COUNT - elif min_images_to_test > MAX_TEST_IMAGE_COUNT: - logger.warning( - f"Testing with more than {MAX_TEST_IMAGE_COUNT} images is likely to focus on the cover slip, or strike the sample. Running scan with test images = {MAX_TEST_IMAGE_COUNT}" - ) - min_images_to_test = MAX_TEST_IMAGE_COUNT - if min_images_to_test < images_to_save: - raise ValueError("Can't save more images than the minimum number tested") - if min_images_to_test % 2 == 0 or min_images_to_test <= 0: - raise ValueError( - "Minimum number of images to test should be positive and odd" - ) - if images_to_save % 2 == 0 or images_to_save <= 0: - raise ValueError("Images to save must be positive and odd") - - self.stack_dz = stack_dz - self.images_to_save = images_to_save - self.min_images_to_test = min_images_to_test - self.autofocus_dz = autofocus_dz - self.images_dir = images_dir - self.save_resolution = save_resolution + stack_dz: int + images_to_save: int + min_images_to_test: int + autofocus_dz: int + images_dir: str + save_resolution: tuple[int, int] + logger: lt.deps.InvocationLogger # Using docstrings under variables as this is how pdoc would expect # attributed to be documented @@ -116,6 +72,41 @@ class StackParams: max_attempts: int = 3 """Maximum number of times to attempt fast stack""" + @field_validator("min_images_to_test") + def check_images_to_test(self, min_images_to_test: int) -> int: + """Verify that the images to test parameter matches various constraints.""" + if min_images_to_test < MIN_TEST_IMAGE_COUNT: + self.logger.warning( + f"Can't test for focus with fewer than {MIN_TEST_IMAGE_COUNT} images. Running scan with test images = {MIN_TEST_IMAGE_COUNT}" + ) + min_images_to_test = MIN_TEST_IMAGE_COUNT + elif min_images_to_test > MAX_TEST_IMAGE_COUNT: + self.logger.warning( + f"Testing with more than {MAX_TEST_IMAGE_COUNT} images is likely to focus on the cover slip, or strike the sample. Running scan with test images = {MAX_TEST_IMAGE_COUNT}" + ) + min_images_to_test = MAX_TEST_IMAGE_COUNT + + if min_images_to_test % 2 == 0 or min_images_to_test <= 0: + raise ValueError( + "Minimum number of images to test should be positive and odd" + ) + return min_images_to_test + + @field_validator("images_to_save") + def check_images_to_save(self, images_to_save: int) -> int: + """Verify that the images to save parameter is positive and odd.""" + if images_to_save % 2 == 0 or images_to_save <= 0: + raise ValueError("Images to save must be positive and odd") + return images_to_save + + @model_validator(mode="after") + def check_image_limits(self) -> "StackParams": + """Ensure the number of images to save isn't more than the minimum tested.""" + if self.images_to_save > self.min_images_to_test: + raise ValueError("Can't save more images than the minimum number tested.") + return self + + @computed_field @property def stack_z_range(self) -> int: """The range of the z stack, in steps. @@ -126,6 +117,7 @@ class StackParams: """ return self.stack_dz * (self.min_images_to_test - 1) + @computed_field @property def steps_undershoot(self) -> int: """The distance to deliberately undershoot the estimated optimal starting point.""" @@ -135,6 +127,7 @@ class StackParams: # requires extra +z movements and captures. return self.stack_dz * self.img_undershoot + @computed_field @property def max_images_to_test(self) -> int: """The maximum number of images that will be captured and tested in a stack. From c89db72d0afa39a982acc0f19c10ef721395086e Mon Sep 17 00:00:00 2001 From: Richard Bowman Date: Thu, 9 Oct 2025 11:12:22 +0100 Subject: [PATCH 4/7] Don't coerce or log from validators. Passing a logger in to the Model is hard: I've changed the StackParams model so that it only validates, and raises a ValueError if it's not correct. Pydantic will turn the ValueError into a ValidationError. --- .../things/autofocus.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 59f067bd..d24bff83 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -43,7 +43,6 @@ class StackParams(BaseModel): autofocus_dz: int images_dir: str save_resolution: tuple[int, int] - logger: lt.deps.InvocationLogger # Using docstrings under variables as this is how pdoc would expect # attributed to be documented @@ -73,19 +72,17 @@ class StackParams(BaseModel): """Maximum number of times to attempt fast stack""" @field_validator("min_images_to_test") - def check_images_to_test(self, min_images_to_test: int) -> int: + @classmethod + def check_images_to_test(cls, min_images_to_test: int) -> int: """Verify that the images to test parameter matches various constraints.""" if min_images_to_test < MIN_TEST_IMAGE_COUNT: - self.logger.warning( - f"Can't test for focus with fewer than {MIN_TEST_IMAGE_COUNT} images. Running scan with test images = {MIN_TEST_IMAGE_COUNT}" + raise ValueError( + f"Can't test for focus with fewer than {MIN_TEST_IMAGE_COUNT} images." ) - min_images_to_test = MIN_TEST_IMAGE_COUNT - elif min_images_to_test > MAX_TEST_IMAGE_COUNT: - self.logger.warning( - f"Testing with more than {MAX_TEST_IMAGE_COUNT} images is likely to focus on the cover slip, or strike the sample. Running scan with test images = {MAX_TEST_IMAGE_COUNT}" + if min_images_to_test > MAX_TEST_IMAGE_COUNT: + raise ValueError( + f"Testing with more than {MAX_TEST_IMAGE_COUNT} images is likely to focus on the cover slip, or strike the sample." ) - min_images_to_test = MAX_TEST_IMAGE_COUNT - if min_images_to_test % 2 == 0 or min_images_to_test <= 0: raise ValueError( "Minimum number of images to test should be positive and odd" @@ -93,7 +90,8 @@ class StackParams(BaseModel): return min_images_to_test @field_validator("images_to_save") - def check_images_to_save(self, images_to_save: int) -> int: + @classmethod + def check_images_to_save(cls, images_to_save: int) -> int: """Verify that the images to save parameter is positive and odd.""" if images_to_save % 2 == 0 or images_to_save <= 0: raise ValueError("Images to save must be positive and odd") From 33ff356f768484633db45ac5868b7546b073b9eb Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 9 Oct 2025 10:29:08 +0000 Subject: [PATCH 5/7] Apply suggestions from code review of branch stack-height-sanitise Co-authored-by: Julian Stirling --- src/openflexure_microscope_server/things/autofocus.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index d24bff83..14990dc9 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -104,7 +104,8 @@ class StackParams(BaseModel): raise ValueError("Can't save more images than the minimum number tested.") return self - @computed_field + # Note MyPy doesn't support decorating properties. See MyPy Pull #16571 and issue #14461. + @computed_field # type: ignore[prop-decorator] @property def stack_z_range(self) -> int: """The range of the z stack, in steps. @@ -115,7 +116,7 @@ class StackParams(BaseModel): """ return self.stack_dz * (self.min_images_to_test - 1) - @computed_field + @computed_field # type: ignore[prop-decorator] @property def steps_undershoot(self) -> int: """The distance to deliberately undershoot the estimated optimal starting point.""" @@ -125,7 +126,7 @@ class StackParams(BaseModel): # requires extra +z movements and captures. return self.stack_dz * self.img_undershoot - @computed_field + @computed_field # type: ignore[prop-decorator] @property def max_images_to_test(self) -> int: """The maximum number of images that will be captured and tested in a stack. @@ -492,7 +493,7 @@ class AutofocusThing(lt.Thing): autofocus_dz: int, save_resolution: tuple[int, int], logger: lt.deps.InvocationLogger, - ) -> Type[StackParams]: + ) -> StackParams: """Set up the parameters used for all stacks in a scan. :param images_dir: the folder to save all images @@ -518,7 +519,7 @@ class AutofocusThing(lt.Thing): cam: CameraClient, stage: Stage, sharpness_monitor: SharpnessMonitorDep, - stack_parameters: Type[StackParams], + stack_parameters: StackParams, ) -> tuple[bool, int]: """Run a smart stack. From fa6fdb1a67d75d97f851ce4eed26603440809cb3 Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 9 Oct 2025 12:22:42 +0100 Subject: [PATCH 6/7] Stack parameter coercion in create_stack_params Co-authored with Julian Stirling --- .../things/autofocus.py | 58 +++++++++++++++++-- .../things/smart_scan.py | 3 +- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index 14990dc9..ececadaf 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -10,7 +10,7 @@ See repository root for licensing information. from contextlib import contextmanager import logging import time -from typing import Annotated, Mapping, Optional, Sequence, Literal, Type +from typing import Annotated, Mapping, Optional, Sequence, Literal import os from dataclasses import dataclass @@ -105,7 +105,7 @@ class StackParams(BaseModel): return self # Note MyPy doesn't support decorating properties. See MyPy Pull #16571 and issue #14461. - @computed_field # type: ignore[prop-decorator] + @computed_field # type: ignore[prop-decorator] @property def stack_z_range(self) -> int: """The range of the z stack, in steps. @@ -116,7 +116,7 @@ class StackParams(BaseModel): """ return self.stack_dz * (self.min_images_to_test - 1) - @computed_field # type: ignore[prop-decorator] + @computed_field # type: ignore[prop-decorator] @property def steps_undershoot(self) -> int: """The distance to deliberately undershoot the estimated optimal starting point.""" @@ -126,7 +126,7 @@ class StackParams(BaseModel): # requires extra +z movements and captures. return self.stack_dz * self.img_undershoot - @computed_field # type: ignore[prop-decorator] + @computed_field # type: ignore[prop-decorator] @property def max_images_to_test(self) -> int: """The maximum number of images that will be captured and tested in a stack. @@ -487,7 +487,7 @@ class AutofocusThing(lt.Thing): """ @lt.thing_action - def set_stack_params( + def create_stack_params( self, images_dir: str, autofocus_dz: int, @@ -503,6 +503,53 @@ class AutofocusThing(lt.Thing): :returns: A StackParams object with the required parameters. """ + # Coerce min_images_to_test parameter + min_images_to_test = self.stack_min_images_to_test + if min_images_to_test < MIN_TEST_IMAGE_COUNT: + logger.warning( + f"Cannot test only {min_images_to_test} image(s) as this will fail. " + "Setting min images to test to lowest possible value of" + f"{MIN_TEST_IMAGE_COUNT}." + ) + min_images_to_test = MIN_TEST_IMAGE_COUNT + elif min_images_to_test > MAX_TEST_IMAGE_COUNT: + logger.warning( + f"Testing {min_images_to_test} images will cause defocus. " + "Setting min images to test to highest possible value of " + f"{MAX_TEST_IMAGE_COUNT}." + ) + min_images_to_test = MAX_TEST_IMAGE_COUNT + elif min_images_to_test % 2 == 0: + min_images_to_test += 1 + logger.warning( + "Minimum number of images to test should be odd, setting to " + f"{min_images_to_test}." + ) + # Set the Thing property to the coerced value + self.stack_min_images_to_test = min_images_to_test + + # Coerce the images to save parameter to be positive, odd, and less than + # min_images_to_save + images_to_save = self.stack_images_to_save + if images_to_save <= 0: + logger.warning( + "At least 1 images must be saved. Setting images to save to 1." + ) + images_to_save = 1 + elif images_to_save > min_images_to_test: + logger.warning( + f"Cannot save {min_images_to_test} images as this above the minimum " + f"number to test. Setting images to save to {MAX_TEST_IMAGE_COUNT}." + ) + images_to_save = min_images_to_test + elif images_to_save % 2 == 0: + images_to_save += 1 + logger.warning( + f"Images to save should be odd, setting to {images_to_save}." + ) + # Set the Thing property to the coerced value + self.stack_images_to_save = images_to_save + return StackParams( stack_dz=self.stack_dz, images_to_save=self.stack_images_to_save, @@ -510,7 +557,6 @@ class AutofocusThing(lt.Thing): autofocus_dz=autofocus_dz, images_dir=images_dir, save_resolution=save_resolution, - logger=logger, ) @lt.thing_action diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 593237b2..a6ab415c 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/src/openflexure_microscope_server/things/smart_scan.py @@ -122,7 +122,6 @@ class SmartScanThing(lt.Thing): self._cam: Optional[CameraClient] = None self._csm: Optional[CSMDep] = None self._stack_params: Optional[StackParams] = None - self._ongoing_scan: Optional[scan_directories.ScanDirectory] = None self._scan_data: Optional[scan_directories.ScanData] = None self._preview_stitcher: Optional[stitching.PreviewStitcher] = None @@ -365,7 +364,7 @@ class SmartScanThing(lt.Thing): self._cam.start_streaming(main_resolution=(3280, 2464)) self._scan_data = self._collect_scan_data() self._ongoing_scan.save_scan_data(self._scan_data) - self._stack_params = self._autofocus.set_stack_params( + self._stack_params = self._autofocus.create_stack_params( images_dir=self._ongoing_scan.images_dir, autofocus_dz=self.autofocus_dz, save_resolution=self._scan_data.save_resolution, From 089beae01a8e9ccf6a5bc085a70d6767449a979e Mon Sep 17 00:00:00 2001 From: Joe Knapper Date: Thu, 9 Oct 2025 13:56:03 +0100 Subject: [PATCH 7/7] Tests for coercing stack params to valid values. Co-authored with Julian Stirling --- .../things/autofocus.py | 2 +- tests/test_smart_scan.py | 3 + tests/test_stack.py | 110 +++++++++++++++++- 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index ececadaf..c8253f1d 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/src/openflexure_microscope_server/things/autofocus.py @@ -538,7 +538,7 @@ class AutofocusThing(lt.Thing): images_to_save = 1 elif images_to_save > min_images_to_test: logger.warning( - f"Cannot save {min_images_to_test} images as this above the minimum " + f"Cannot save {images_to_save} images as this above the minimum " f"number to test. Setting images to save to {MAX_TEST_IMAGE_COUNT}." ) images_to_save = min_images_to_test diff --git a/tests/test_smart_scan.py b/tests/test_smart_scan.py index c6e940fc..617caab1 100644 --- a/tests/test_smart_scan.py +++ b/tests/test_smart_scan.py @@ -292,8 +292,11 @@ def scan_thing_mocked_for_scan_data(smart_scan_thing, mocker): mock_stage = mocker.Mock() type(mock_stage).position = mocker.PropertyMock(return_value=MOCK_START_POS) + mock_autofocus = mocker.Mock() + smart_scan_thing._ongoing_scan = mock_ongoing_scan smart_scan_thing._stage = mock_stage + smart_scan_thing._autofocus = mock_autofocus return smart_scan_thing diff --git a/tests/test_stack.py b/tests/test_stack.py index 06e18686..912e588d 100644 --- a/tests/test_stack.py +++ b/tests/test_stack.py @@ -4,20 +4,29 @@ Currently these tests don't test the Thing itself, just surrounding functionalit """ from typing import Optional +import tempfile from random import randint +import logging import pytest import numpy as np from hypothesis import given, strategies as st +from fastapi.testclient import TestClient + +import labthings_fastapi as lt from openflexure_microscope_server.things.autofocus import ( + AutofocusThing, StackParams, CaptureInfo, + MIN_TEST_IMAGE_COUNT, + MAX_TEST_IMAGE_COUNT, _get_capture_by_id, _get_capture_index_by_id, ) from openflexure_microscope_server.scan_directories import IMAGE_REGEX +LOGGER = logging.getLogger("mock-invocation_logger") RANDOM_GENERATOR = np.random.default_rng() @@ -42,21 +51,24 @@ def even_integers(min_value=0, max_value=1000): @given( - save_ims=odd_integers(min_value=0, max_value=10), + save_ims=odd_integers(min_value=1, max_value=9), extra_ims=even_integers(min_value=0, max_value=10), ) def test_stack_params_validation(save_ims, extra_ims): - """Test the validation of the image numbers for a stack. + """Test the validation of the image numbers for a stack for valid combinations. save_ims is the number to save (must be odd and positive) extra_ims is how many more images there are in min_images_to_test than images_to_save. (even so that, min_images_to_test is odd and larger than images_to_save """ + # Coerce min_images_to_test as the max extra ims depends on save_ims so is hard + # 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) StackParams( stack_dz=50, images_to_save=save_ims, - min_images_to_test=save_ims + extra_ims, + min_images_to_test=min_images_to_test, autofocus_dz=2000, images_dir="/this/is/fake", save_resolution=(1640, 1232), @@ -228,3 +240,95 @@ def test_retrieval_of_captures(start): _get_capture_by_id(captures, start - 1) with pytest.raises(ValueError): _get_capture_by_id(captures, start + 21) + + +@pytest.fixture +def autofocus_thing(): + """Yield an autofocus thing connected to a server.""" + autofocus_thing = AutofocusThing() + with tempfile.TemporaryDirectory() as tmpdir: + server = lt.ThingServer(settings_folder=tmpdir) + server.add_thing(autofocus_thing, "/autofocus/") + with TestClient(server.app): + yield autofocus_thing + + +def test_create_stack(autofocus_thing, caplog): + """Run create stack with default values and check there is no coercion or logging.""" + initial_min_images_to_test = autofocus_thing.stack_min_images_to_test + initial_images_to_save = autofocus_thing.stack_images_to_save + with caplog.at_level(logging.INFO): + stack_params = autofocus_thing.create_stack_params( + autofocus_dz=2000, + images_dir="/this/is/fake", + save_resolution=(1640, 1232), + logger=LOGGER, + ) + + assert len(caplog.records) == 0 + assert autofocus_thing.stack_min_images_to_test == initial_min_images_to_test + assert autofocus_thing.stack_images_to_save == initial_images_to_save + assert stack_params.min_images_to_test == autofocus_thing.stack_min_images_to_test + assert stack_params.images_to_save == autofocus_thing.stack_images_to_save + + +@pytest.mark.parametrize( + ("initial_test_ims", "coerced_test_ims", "expected_log_start"), + [ + (0, MIN_TEST_IMAGE_COUNT, "Cannot test only 0"), + (-1, MIN_TEST_IMAGE_COUNT, "Cannot test only -1"), + (10, MAX_TEST_IMAGE_COUNT, "Testing 10 images will"), + (4, 5, "Minimum number of images to test should be odd"), + ], +) +def test_coercing_stack_test_ims( + initial_test_ims, coerced_test_ims, expected_log_start, autofocus_thing, caplog +): + """Run create stack with images to test set to values requiring coercion, and check result.""" + autofocus_thing.stack_min_images_to_test = initial_test_ims + + with caplog.at_level(logging.WARNING): + stack_params = autofocus_thing.create_stack_params( + autofocus_dz=2000, + images_dir="/this/is/fake", + save_resolution=(1640, 1232), + logger=LOGGER, + ) + + assert len(caplog.records) == 1 + assert str(caplog.records[0].msg).startswith(expected_log_start) + # Check the value is coerced in the stack_params + assert stack_params.min_images_to_test == coerced_test_ims + # Check that the setting in the Thing was updated to the coerced value + assert stack_params.min_images_to_test == autofocus_thing.stack_min_images_to_test + + +@pytest.mark.parametrize( + ("initial_save_ims", "coerced_save_ims", "expected_log_start"), + [ + (0, 1, "At least 1 images must be saved"), + (-1, 1, "At least 1 images must be saved"), + (10, 9, "Cannot save 10 images"), + (4, 5, "Images to save should be odd, setting to 5"), + ], +) +def test_coercing_stack_save_ims( + initial_save_ims, coerced_save_ims, expected_log_start, autofocus_thing, caplog +): + """Run create stack with images to save set to values requiring coercion, and check result.""" + autofocus_thing.stack_images_to_save = initial_save_ims + + with caplog.at_level(logging.WARNING): + stack_params = autofocus_thing.create_stack_params( + autofocus_dz=2000, + images_dir="/this/is/fake", + save_resolution=(1640, 1232), + logger=LOGGER, + ) + + assert len(caplog.records) == 1 + assert str(caplog.records[0].msg).startswith(expected_log_start) + # Check the value is coerced in the stack_params + assert stack_params.images_to_save == coerced_save_ims + # Check that the setting in the Thing was updated to the coerced value + assert stack_params.images_to_save == autofocus_thing.stack_images_to_save