Clarify behaviour of overloads, and better error checking of _move_and_measure inputs

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])

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.