From 24233412af0016c44edd84fca1483d168a9004be Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Wed, 27 Aug 2025 13:21:34 +0100 Subject: [PATCH] Start adding ANN ruff rules --- change_log_helper.py | 2 +- pyproject.toml | 8 +++++- src/openflexure_microscope_server/logging.py | 4 +-- .../scan_directories.py | 2 +- .../scan_planners.py | 2 +- .../stitching.py | 2 +- .../things/autofocus.py | 4 +-- .../things/camera/__init__.py | 10 +++++-- .../things/camera/opencv.py | 8 +++++- .../things/camera/picamera.py | 24 ++++++++++++---- .../camera/picamera_recalibrate_utils.py | 4 ++- .../things/camera/simulation.py | 8 +++++- .../things/smart_scan.py | 13 ++++++--- .../things/stage/__init__.py | 8 +++--- .../things/stage/dummy.py | 13 +++++++-- .../things/stage/sangaboard.py | 14 +++++++--- src/openflexure_microscope_server/ui.py | 10 +++++-- .../utilities.py | 28 ++++++++++++++----- tests/test_stage.py | 5 ++-- 19 files changed, 123 insertions(+), 46 deletions(-) diff --git a/change_log_helper.py b/change_log_helper.py index 94202aaf..79296132 100755 --- a/change_log_helper.py +++ b/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. diff --git a/pyproject.toml b/pyproject.toml index 746cc24e..b1d8aaa9 100644 --- a/pyproject.toml +++ b/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] diff --git a/src/openflexure_microscope_server/logging.py b/src/openflexure_microscope_server/logging.py index 69a03ffc..0df9a60e 100644 --- a/src/openflexure_microscope_server/logging.py +++ b/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 diff --git a/src/openflexure_microscope_server/scan_directories.py b/src/openflexure_microscope_server/scan_directories.py index a6fd124f..cb96bd25 100644 --- a/src/openflexure_microscope_server/scan_directories.py +++ b/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()] diff --git a/src/openflexure_microscope_server/scan_planners.py b/src/openflexure_microscope_server/scan_planners.py index 065b3258..87f9000b 100644 --- a/src/openflexure_microscope_server/scan_planners.py +++ b/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), diff --git a/src/openflexure_microscope_server/stitching.py b/src/openflexure_microscope_server/stitching.py index 1660944e..32a5253e 100644 --- a/src/openflexure_microscope_server/stitching.py +++ b/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, diff --git a/src/openflexure_microscope_server/things/autofocus.py b/src/openflexure_microscope_server/things/autofocus.py index a6c0e92a..e80d1e47 100644 --- a/src/openflexure_microscope_server/things/autofocus.py +++ b/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. diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index abee8f06..94badd0c 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/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 diff --git a/src/openflexure_microscope_server/things/camera/opencv.py b/src/openflexure_microscope_server/things/camera/opencv.py index 5f179301..e87befec 100644 --- a/src/openflexure_microscope_server/things/camera/opencv.py +++ b/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. diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 5e55ea92..25b29a44 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/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: diff --git a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py b/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py index fbf827df..44f269f6 100644 --- a/src/openflexure_microscope_server/things/camera/picamera_recalibrate_utils.py +++ b/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... diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index 7c7a856b..31e1c9aa 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/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 diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index 9e44a329..a7b70fb8 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/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 diff --git a/src/openflexure_microscope_server/things/stage/__init__.py b/src/openflexure_microscope_server/things/stage/__init__.py index a57f903d..92bd76d7 100644 --- a/src/openflexure_microscope_server/things/stage/__init__.py +++ b/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. diff --git a/src/openflexure_microscope_server/things/stage/dummy.py b/src/openflexure_microscope_server/things/stage/dummy.py index 8b71f78f..3015b42c 100644 --- a/src/openflexure_microscope_server/things/stage/dummy.py +++ b/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 = { diff --git a/src/openflexure_microscope_server/things/stage/sangaboard.py b/src/openflexure_microscope_server/things/stage/sangaboard.py index d6ab3b29..8fea6684 100644 --- a/src/openflexure_microscope_server/things/stage/sangaboard.py +++ b/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(): diff --git a/src/openflexure_microscope_server/ui.py b/src/openflexure_microscope_server/ui.py index be2a5c12..7a595dd9 100644 --- a/src/openflexure_microscope_server/ui.py +++ b/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. diff --git a/src/openflexure_microscope_server/utilities.py b/src/openflexure_microscope_server/utilities.py index bbad56f9..5975ed1e 100644 --- a/src/openflexure_microscope_server/utilities.py +++ b/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. diff --git a/tests/test_stage.py b/tests/test_stage.py index c9ae0918..1beebf9b 100644 --- a/tests/test_stage.py +++ b/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