From 08046ff657267c9957458973307c82f6742e7b83 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 24 Oct 2025 17:27:43 +0100 Subject: [PATCH] Clarify behaviour of overloads, and better error checking of _move_and_measure inputs --- .../things/stage_measure.py | 27 ++++++++++++------ tests/test_stage_measure.py | 28 +++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/openflexure_microscope_server/things/stage_measure.py b/src/openflexure_microscope_server/things/stage_measure.py index 24aca185..45175c2c 100644 --- a/src/openflexure_microscope_server/things/stage_measure.py +++ b/src/openflexure_microscope_server/things/stage_measure.py @@ -83,7 +83,7 @@ class RomDataTracker: :param axis: The axis which is being measured. This must be 'x' or 'y'. :param stage_movement: The movement to be performed in stage coordinates. :param stage_position: The current stage position in stage coordinates. - :returns: The predicted relative z displacement needed to stay in focus. + :return: The predicted relative z displacement needed to stay in focus. """ # x or y positions lateral_positions = [i[axis] for i in self.stage_coords] @@ -479,6 +479,11 @@ class RangeofMotionThing(lt.Thing): autofocus is repeated. :return: The calculated offset from cross correlation. """ + if max_autofocus_repeats > 0 and abs_min_offset <= 0: + raise ValueError( + "abs_min_offset must be positive if max_autofocus_repeats > 0." + ) + # Take image before move before_img = rom_deps.cam.grab_as_array() # Move and autofocus if required @@ -533,22 +538,26 @@ class RangeofMotionThing(lt.Thing): return {"x": offset[1], "y": offset[0]} -@overload -def _axis_from_movement_dict(movement: dict[str, float]) -> str: ... - - +# The number of return arguments depends on if return_other is True or False +# let MyPy know with overloads typed to Literal[False] and Literal[True]. @overload def _axis_from_movement_dict( movement: dict[str, float], return_other: Literal[False] ) -> str: ... - - @overload def _axis_from_movement_dict( movement: dict[str, float], return_other: Literal[True] ) -> tuple[str, str]: ... +# Overload the case where return_other is not specified, this is needed +# because MyPy doesn't see that return other's default is `False` and use +# the `Literal[False]` overload. +@overload +def _axis_from_movement_dict(movement: dict[str, float]) -> str: ... + + +# Finally The function. def _axis_from_movement_dict( movement: dict[str, float], return_other: bool = False ) -> str | tuple[str, str]: @@ -573,10 +582,12 @@ def _axis_from_movement_dict( def _parasitic_motion_detected( movement: dict[str, float], offset: dict[str, float] ) -> bool: - """Compare a desired movement to measured offset and error if parasitic motion is too high. + """Compare desired movement to measured offset, report if parasitic motion was detected. :param movement: The movement dictionary in image coordinates. :param offset: The offset calculated from correlation. + :return: True if too much parasitic motion is detects. False if movement is as + expected. """ movement_axis, other_axis = _axis_from_movement_dict(movement, return_other=True) parasitic_motion = abs(offset[other_axis]) diff --git a/tests/test_stage_measure.py b/tests/test_stage_measure.py index 69f6c1bd..9f4e3a4e 100644 --- a/tests/test_stage_measure.py +++ b/tests/test_stage_measure.py @@ -273,6 +273,34 @@ def test_move_and_measure_with_refocus( assert offset == expected_return +def test_move_and_measure_with_bad_refocus_args(rom_thing, mocker): + """Check error if abs_min_offset is not positive when using it to determine if to autofocus.""" + offset_from_mock = mocker.patch.object( + rom_thing, "_offset_from", return_value={"x": 0, "y": 0} + ) + + # First check with abs_min_offset not set. The default should be zero. + with pytest.raises(ValueError, match="abs_min_offset must be positive"): + rom_thing._move_and_measure( + movement={"x": 10, "y": 0}, + rom_deps=mock_rom_deps, + perform_autofocus=False, + max_autofocus_repeats=3, + ) + # Then check with abs_min_offset negative, this might happen if the the expected + # move calculation is not made absolute. + with pytest.raises(ValueError, match="abs_min_offset must be positive"): + rom_thing._move_and_measure( + movement={"x": 10, "y": 0}, + rom_deps=mock_rom_deps, + perform_autofocus=False, + max_autofocus_repeats=3, + abs_min_offset=-123.456, + ) + # Should have never measured and offset. Just error straight away. + assert offset_from_mock.call_count == 0 + + def test_move_back_until_motion_detected(rom_thing, mock_rom_deps, mocker): """Check that _move_back_until_motion_detected is making increasing negative moves.