Start adding ANN ruff rules

change_log_helper.py

@@ -12,7 +12,7 @@ from gitlab.exceptions import GitlabGetError
 PROJECT_ID = 9238334
 
 
-def main(tag, branch: str = "v3"):
+def main(tag: str, branch: str = "v3"):
     """Create a list of the MRs into a given branch after a tag.
 
     :param tag: The tag the mrs should be since.

pyproject.toml

@@ -124,8 +124,9 @@ select = [
     "C90", # McCabe complexity!
     "NPY", # Numpy linting
     "N", # PEP8 naming
-    "D", # Docstring checks these may need to be added gradually
+    "D", # Docstring checks
     "ERA001", # Commented out code!
+    "ANN001",
 ]
 
 ignore = [
@@ -140,6 +141,11 @@ ignore = [
 # Tests are currently not fully docstring-ed, we'll ignore this for now.
 "tests/*" = [
     "B018", # Complaining about useless attribute access in tests, but we need them to check errors are raised
+    "ANN", # Tests are not typehinted for fixtures etc
+]
+"hardware-specific-tests/*" = [
+    "B018", # Complaining about useless attribute access in tests, but we need them to check errors are raised
+    "ANN", # Tests are not typehinted for fixtures etc
 ]
 
 [tool.ruff.lint.pydocstyle]

src/openflexure_microscope_server/logging.py

@@ -21,7 +21,7 @@ from fastapi import HTTPException
 OFM_LOG_FILE = None
 
 
-def configure_logging(log_folder):
+def configure_logging(log_folder: str):
     """Configure logging for the server while it is running.
 
     This modifies the root logger to have a rotating file handler and
@@ -120,7 +120,7 @@ class OFMLogFileFormatter(logging.Formatter):
 class OFMHandler(logging.Handler):
     """A logging.Handler that stores the most recent logs for access by the server."""
 
-    def __init__(self, level=logging.INFO, max_logs=250):
+    def __init__(self, level: int = logging.INFO, max_logs: int = 250):
         """Initialise the handler with a set logging level and message buffer size.
 
         :param level: The level of logs captured. As standard logs of INFO and above

src/openflexure_microscope_server/scan_directories.py

@@ -551,7 +551,7 @@ class ScanDirectory:
         return zip_fname
 
 
-def get_files_in_zip(zip_path):
+def get_files_in_zip(zip_path: str):
     """List the relative paths of all files and folders in the zip folder specified."""
     scan_zip = zipfile.ZipFile(zip_path)
     return [os.path.normpath(i) for i in scan_zip.namelist()]

src/openflexure_microscope_server/scan_planners.py

@@ -332,7 +332,7 @@ class SmartSpiral(ScanPlanner):
         """Sort the remaining positions based on the current location."""
 
         # Defined rather than use a lambda for readability
-        def sort_key(pos):
+        def sort_key(pos: XYPos):
             return (
                 self.moves_between(current_pos, pos),
                 self.moves_between(self._initial_position, pos),

src/openflexure_microscope_server/stitching.py

@@ -185,7 +185,7 @@ class FinalStitcher(BaseStitcher):
 
     def __init__(
         self,
-        images_dir,
+        images_dir: str,
         *,
         logger: lt.deps.InvocationLogger,
         overlap: Optional[float] = None,

src/openflexure_microscope_server/things/autofocus.py

@@ -127,7 +127,7 @@ class StackParams:
         """
         return self.min_images_to_test + 15
 
-    def slice_to_save(self, sharpest_index):
+    def slice_to_save(self, sharpest_index: int):
         """Return the slice of images to save given the index of the sharpest image."""
         images_each_side = (self.images_to_save - 1) // 2
         return slice(
@@ -401,7 +401,7 @@ class AutofocusThing(lt.Thing):
         self,
         stage: Stage,
         sharpness_monitor: SharpnessMonitorDep,
-        dz=2000,
+        dz: int = 2000,
         start: Literal["centre", "base"] = "centre",
     ):
         """Repeatedly autofocus the stage until it looks focused.

src/openflexure_microscope_server/things/camera/__init__.py

@@ -8,6 +8,7 @@ See repository root for licensing information.
 
 from __future__ import annotations
 from typing import Literal, Optional, Tuple, Any
+from types import TracebackType
 import json
 import io
 import time
@@ -185,7 +186,12 @@ class BaseCamera(lt.Thing):
         """Open hardware connection when the Thing context manager is opened."""
         raise NotImplementedError("CameraThings must define their own __enter__ method")
 
-    def __exit__(self, _exc_type, _exc_value, _traceback) -> None:
+    def __exit__(
+        self,
+        _exc_type: type[BaseException],
+        _exc_value: Optional[BaseException],
+        _traceback: Optional[TracebackType],
+    ) -> None:
         """Close hardware connection when the Thing context manager is closed."""
         raise NotImplementedError("CameraThings must define their own __exit__ method")
 
@@ -575,7 +581,7 @@ class BaseCamera(lt.Thing):
         return self.active_detector.status
 
     @lt.thing_action
-    def update_detector_settings(self, data) -> None:
+    def update_detector_settings(self, data: dict[str, Any]) -> None:
         """Update the settings of the current detector.
 
         This is an action not a setting/property as the data model depends on the

src/openflexure_microscope_server/things/camera/opencv.py

@@ -10,6 +10,7 @@ from __future__ import annotations
 
 import logging
 from typing import Literal, Optional
+from types import TracebackType
 from threading import Thread
 
 import cv2
@@ -42,7 +43,12 @@ class OpenCVCamera(BaseCamera):
         self._capture_thread.start()
         return self
 
-    def __exit__(self, _exc_type, _exc_value, _traceback):
+    def __exit__(
+        self,
+        _exc_type: type[BaseException],
+        _exc_value: Optional[BaseException],
+        _traceback: Optional[TracebackType],
+    ):
         """Release the camera when the Thing context manager is closed.
 
         Before releasing the camera the capture thread is closed.

src/openflexure_microscope_server/things/camera/picamera.py

@@ -15,7 +15,8 @@ https://datasheets.raspberrypi.com/camera/raspberry-pi-camera-guide.pdf
 """
 
 from __future__ import annotations
-from typing import Annotated, Iterator, Literal, Mapping, Optional, overload
+from typing import Annotated, Iterator, Literal, Mapping, Optional, overload, Any
+from types import TracebackType
 import json
 import logging
 import os
@@ -65,7 +66,12 @@ class PicameraStreamOutput(Output):
         self.portal = portal
 
     def outputframe(
-        self, frame, _keyframe=True, _timestamp=None, _packet=None, _audio=False
+        self,
+        frame: bytes,
+        _keyframe: Optional[bool] = True,
+        _timestamp: Optional[int] = None,
+        _packet: Any = None,
+        _audio: bool = False,
     ):
         """Add a frame to the stream's ringbuffer."""
         self.stream.add_frame(frame, self.portal)
@@ -396,7 +402,7 @@ class StreamingPiCamera2(BaseCamera):
         return self._picamera is not None and self._picamera.started
 
     @contextmanager
-    def _streaming_picamera(self, pause_stream=False) -> Iterator[Picamera2]:
+    def _streaming_picamera(self, pause_stream: bool = False) -> Iterator[Picamera2]:
         """Lock access to picamera and return the underlying ``Picamera2`` instance.
 
         Optionally the stream can be paused to allow updating the camera settings.
@@ -419,7 +425,12 @@ class StreamingPiCamera2(BaseCamera):
                 if pause_stream and already_streaming:
                     self.start_streaming()
 
-    def __exit__(self, exc_type, exc_value, traceback):
+    def __exit__(
+        self,
+        _exc_type: type[BaseException],
+        _exc_value: Optional[BaseException],
+        _traceback: Optional[TracebackType],
+    ):
         """Close the picamera connection when the Thing context manager is closed."""
         self.stop_streaming()
         with self._streaming_picamera() as cam:
@@ -725,7 +736,10 @@ class StreamingPiCamera2(BaseCamera):
         return tuple(recalibrate_utils.get_static_ccm(self.tuning)[0]["ccm"])
 
     @colour_correction_matrix.setter  # type: ignore
-    def colour_correction_matrix(self, value) -> None:
+    def colour_correction_matrix(
+        self,
+        value: tuple[float, float, float, float, float, float, float, float, float],
+    ) -> None:
         recalibrate_utils.set_static_ccm(self.tuning, value)
 
         if self._picamera is not None:

src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py

@@ -353,7 +353,9 @@ def upsample_channels(grids: np.ndarray, shape: tuple[int]) -> np.ndarray:
     return zoom(grids, zoom_factors, order=1)[:, : shape[0], : shape[1]]
 
 
-def downsampled_channels(channels: np.ndarray, blacklevel=64) -> list[np.ndarray]:
+def downsampled_channels(
+    channels: np.ndarray, blacklevel: int = 64
+) -> list[np.ndarray]:
     """Generate a downsampled, un-normalised image from which to calculate the LST.
 
     TODO: blacklevel probably ought to be determined from the camera...

src/openflexure_microscope_server/things/camera/simulation.py

@@ -9,6 +9,7 @@ See repository root for licensing information.
 from __future__ import annotations
 import logging
 from typing import Literal, Optional
+from types import TracebackType
 from threading import Thread
 import time
 
@@ -212,7 +213,12 @@ class SimulatedCamera(BaseCamera):
         self.start_streaming()
         return self
 
-    def __exit__(self, _exc_type, _exc_value, _traceback):
+    def __exit__(
+        self,
+        _exc_type: type[BaseException],
+        _exc_value: Optional[BaseException],
+        _traceback: Optional[TracebackType],
+    ):
         """Close the capture thread when the Thing context manager is closed."""
         if self.stream_active:
             self._capture_enabled = False

src/openflexure_microscope_server/things/smart_scan.py

@@ -7,7 +7,7 @@ It also controls external processes for live stitching composite images, and
 the creation of the final stitched images.
 """
 
-from typing import Optional
+from typing import Optional, Self, TypeVar, ParamSpec, Callable, Concatenate
 import threading
 import os
 import time
@@ -30,6 +30,9 @@ from .camera_stage_mapping import CameraStageMapper
 from .camera import CameraDependency as CameraClient
 from .stage import StageDependency as StageDep
 
+T = TypeVar("T")
+P = ParamSpec("P")
+
 CSMDep = lt.deps.direct_thing_client_dependency(
     CameraStageMapper, "/camera_stage_mapping/"
 )
@@ -43,7 +46,9 @@ class ScanNotRunningError(RuntimeError):
     """Exception called when scan not running that requires a scan to be running."""
 
 
-def _scan_running(method):
+def _scan_running(
+    method: Callable[Concatenate[Self, P], T],
+) -> Callable[Concatenate[Self, P], T]:
     """Decorate a method so that it will error if a scan is not running.
 
     This decorator is used by all methods in SmartScanThing that are using
@@ -52,7 +57,7 @@ def _scan_running(method):
     the same time and released with the lock
     """
 
-    def scan_running_wrapper(self, *args, **kwargs):
+    def scan_running_wrapper(self: Self, *args: P.args, **kwargs: P.kwargs) -> T:
         # Only start the method is the scan logger is set
         if self._scan_logger is not None:
             return method(self, *args, **kwargs)
@@ -623,7 +628,7 @@ class SmartScanThing(lt.Thing):
             if scan_info.number_of_images == 0:
                 self._delete_scan(scan_info.name, logger)
 
-    def _delete_scan(self, scan_name, logger: lt.deps.InvocationLogger) -> bool:
+    def _delete_scan(self, scan_name: str, logger: lt.deps.InvocationLogger) -> bool:
         """Delete a scan.
 
         This is a wrapper around scan manager's delete_scan that logs to the

src/openflexure_microscope_server/things/stage/__init__.py

@@ -142,7 +142,7 @@ class BaseStage(lt.Thing):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ):
         """Make a relative move. Keyword arguments should be axis names."""
         self._hardware_move_relative(
@@ -155,7 +155,7 @@ class BaseStage(lt.Thing):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ):
         """Make a relative move in the coordinate system used by the physical hardware.
 
@@ -170,7 +170,7 @@ class BaseStage(lt.Thing):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ):
         """Make an absolute move. Keyword arguments should be axis names."""
         self._hardware_move_absolute(
@@ -183,7 +183,7 @@ class BaseStage(lt.Thing):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ):
         """Make a absolute move in the coordinate system used by the physical hardware.
 

src/openflexure_microscope_server/things/stage/dummy.py

@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+from typing import Optional
+from types import TracebackType
 from collections.abc import Mapping
 import time
 
@@ -33,7 +35,12 @@ class DummyStage(BaseStage):
         """Register the stage position when the Thing context manager is opened."""
         self.instantaneous_position = self._hardware_position
 
-    def __exit__(self, _exc_type, _exc_value, _traceback):
+    def __exit__(
+        self,
+        _exc_type: type[BaseException],
+        _exc_value: Optional[BaseException],
+        _traceback: Optional[TracebackType],
+    ):
         """Nothing to do when the Thing context manager is closed."""
 
     axis_inverted = lt.ThingSetting(
@@ -47,7 +54,7 @@ class DummyStage(BaseStage):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ):
         """Make a relative move. Keyword arguments should be axis names."""
         displacement = [kwargs.get(k, 0) for k in self.axis_names]
@@ -85,7 +92,7 @@ class DummyStage(BaseStage):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ):
         """Make an absolute move. Keyword arguments should be axis names."""
         displacement = {

src/openflexure_microscope_server/things/stage/sangaboard.py

@@ -5,7 +5,8 @@ import logging
 import threading
 import time
 from copy import copy
-from typing import Iterator, Literal
+from typing import Iterator, Literal, Optional
+from types import TracebackType
 from contextlib import contextmanager
 from collections.abc import Mapping
 
@@ -53,7 +54,12 @@ class SangaboardThing(BaseStage):
             sb.query("blocking_moves false")
         self.update_position()
 
-    def __exit__(self, _exc_type, _exc_value, _traceback):
+    def __exit__(
+        self,
+        _exc_type: type[BaseException],
+        _exc_value: Optional[BaseException],
+        _traceback: Optional[TracebackType],
+    ):
         """Close the sangaboard connection when the Thing context manager is closed."""
         with self.sangaboard() as sb:
             sb.close()
@@ -85,7 +91,7 @@ class SangaboardThing(BaseStage):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ) -> None:
         """Make a relative move in the coordinate system used by the sangaboard."""
         displacement = [kwargs.get(axis, 0) for axis in self.axis_names]
@@ -112,7 +118,7 @@ class SangaboardThing(BaseStage):
         self,
         cancel: lt.deps.CancelHook,
         block_cancellation: bool = False,
-        **kwargs: Mapping[str, int],
+        **kwargs: int,
     ) -> None:
         """Make a absolute move in the coordinate system used by the sangaboard."""
         with self.sangaboard():

src/openflexure_microscope_server/ui.py

@@ -1,7 +1,11 @@
 """Functionality for communicating the required user interface for a thing."""
 
+from typing import Callable, Any
+
 from pydantic import BaseModel
 
+import labthings_fastapi as lt
+
 
 class ActionButton(BaseModel):
     """The data required for creating an actionButton in Vue.
@@ -43,7 +47,7 @@ class ActionButton(BaseModel):
     """The message to show on successful completion."""
 
 
-def action_button_for(action, **kwargs) -> ActionButton:
+def action_button_for(action: Callable[..., Any], **kwargs: Any) -> ActionButton:
     """Create a ActionButton data for the specified Thing Action.
 
     :param action: The thing action to create a button for.
@@ -80,7 +84,9 @@ class PropertyControl(BaseModel):
     """The delay in ms before reading back the property."""
 
 
-def property_control_for(thing, property_name, **kwargs) -> PropertyControl:
+def property_control_for(
+    thing: lt.Thing, property_name: str, **kwargs: Any
+) -> PropertyControl:
     """Create an PropertyControl data for the specified Thing Property.
 
     :param thing: The instance of the thing that has the property to be controlled.

src/openflexure_microscope_server/utilities.py

@@ -1,6 +1,6 @@
 """Utility functions and classes."""
 
-from typing import TypeVar, Callable, ParamSpec
+from typing import TypeVar, Callable, ParamSpec, Optional, Any, Concatenate, Self
 import os
 import re
 import sys
@@ -24,7 +24,7 @@ COMMIT_REGEX = re.compile(r"[0-9a-f]{40}")
 REF_REGEX = re.compile(r"^ref:\s(.*)$")
 
 
-def _is_lock_like(obj):
+def _is_lock_like(obj: Any) -> bool:
     """Check if an object is a lock.
 
     Cannot use ``isinstance(obj, threading.RLock)``, may be possible to use
@@ -34,14 +34,16 @@ def _is_lock_like(obj):
     return hasattr(obj, "acquire") and hasattr(obj, "release")
 
 
-def requires_lock(method: Callable[P, T]) -> Callable[P, T]:
+def requires_lock(
+    method: Callable[Concatenate[Self, P], T],
+) -> Callable[Concatenate[Self, P], T]:
     """Decorate a class method so that it requires the class lock to run.
 
     The class should have a reentrant lock with the name ``self._lock``.
     """
 
     @wraps(method)
-    def wrapper(self, *args: P.args, **kwargs: P.kwargs) -> T:
+    def wrapper(self: Self, *args: P.args, **kwargs: P.kwargs) -> T:
         # Confirm the object the method is attached to has a self._lock property
         if not hasattr(self, "_lock"):
             raise AttributeError(f"{self.__class__.__name__} has no '_lock' attribute")
@@ -67,7 +69,16 @@ class ErrorCapturingThread(Thread):
     join is run.
     """
 
-    def __init__(self, group=None, target=None, args=None, kwargs=None, daemon=None):
+    def __init__(
+        self,
+        group: Optional[Any] = None,
+        target: Optional[Callable[..., Any]] = None,
+        name: Optional[str] = None,
+        args: tuple[Any, ...] = (),
+        kwargs: Optional[dict[str, Any]] = None,
+        *,
+        daemon: Optional[bool] = None,
+    ):
         """Initialise with the same arguments as Thread."""
         # As all inputs are keywords we need to set the default values for args and kwargs:
         if args is None:
@@ -88,12 +99,13 @@ class ErrorCapturingThread(Thread):
         super().__init__(
             group=group,
             target=_wrap_and_catch_errors,
+            name=name,
             args=args,
             kwargs=kwargs,
             daemon=daemon,
         )
 
-    def join(self, timeout=None):
+    def join(self, timeout: Optional[float] = None):
         """Join when the thread is complete.
 
         If the thread ended due to an unhandled exception, the exception will be raised
@@ -107,7 +119,9 @@ class ErrorCapturingThread(Thread):
             raise err
 
 
-def _wrap_and_catch_errors(target, error_buffer, *args, **kwargs):
+def _wrap_and_catch_errors(
+    target: Callable[..., Any], error_buffer: list, *args, **kwargs
+):
     """Run target function in a try-except block.
 
     This function is designed only to be used by ErrorCapturingThread.

tests/test_stage.py

@@ -1,6 +1,5 @@
 """Test the stage without creating a full HTTP server and socket connection."""
 
-from collections.abc import Mapping
 import tempfile
 import itertools
 
@@ -69,7 +68,7 @@ def test_override_base_movement():
             self,
             cancel: lt.deps.CancelHook,
             block_cancellation: bool = False,
-            **kwargs: Mapping[str, int],
+            **kwargs: int,
         ):
             pass
 
@@ -82,7 +81,7 @@ def test_override_base_movement():
             self,
             cancel: lt.deps.CancelHook,
             block_cancellation: bool = False,
-            **kwargs: Mapping[str, int],
+            **kwargs: int,
         ):
             pass