Squash final todos for SmartScan refactor

src/openflexure_microscope_server/scan_directories.py

@@ -43,7 +43,7 @@ class ScanData(BaseModel):
 
     This serialises into a human readable format where possible with
 
-    timestamps in %H_%M_%S-%d_%m_%Y format
+    timestamps in %Y-%m-%s_%H:%M:%S format
     timedeltas in %H:%M:%S format
 
     Properties that are not known until the end have ``None`` serialised as "Unknown"
@@ -51,26 +51,59 @@ class ScanData(BaseModel):
 
     model_config = {"extra": "forbid"}
 
-    # TODO: Docs for each variable
-    # TODO: Make note about timestamp format. Is it too ambiguous?
     scan_name: str
+    """The name of the scan i.e. scan_0001"""
+
     starting_position: Mapping[str, int]
+    """The starting position in dictionary format."""
+
     overlap: float
+    """The overlap between adjacent images as a fraction of the image size."""
+
     max_dist: int
+    """The maximum distance the scan could move (in steps) from the starting position."""
+
     dx: int
+    """The number of steps between adjacent images in x."""
+
     dy: int
+    """The number of steps between adjacent images in y."""
+
     autofocus_dz: int
+    """The z range used for autofocus (in steps)."""
+
     autofocus_on: bool
+    """Whether autofocus is on."""
+
     start_time: datetime
+    """The time the scan started."""
+
     skip_background: bool
+    """Whether automatic background detection is on, skipping locations with no sample."""
+
     stitch_automatically: bool
-    # TODO: Think about changing stitch_resize name as it is NOT the resize for
+    """Whether the scan is set to automatically stitch when complete."""
-    # just for correlation stitching
+
-    stitch_resize: float
+    correlation_resize: float
+    """The resize factor applied to images when the stitching program is correlating."""
+
     save_resolution: tuple[int, int]
+    """The resolution that scan images are saved at."""
+
     image_count: int = 0
+    """The number of images taken."""
+
     duration: Optional[timedelta] = None
+    """The duration of the scan.
+
+    This is automatically set when ``set_final_data()`` is run.
+    """
+
     scan_result: Optional[str] = None
+    """The result of the scan.
+
+    This should be set with ``set_final_data()`` to ensure duration is set.
+    """
 
     def set_final_data(self, result: str):
         """Set the final data for the scan, scan duration is automatically calculated.
@@ -83,15 +116,15 @@ class ScanData(BaseModel):
     @field_validator("start_time", mode="before")
     @classmethod
     def parse_timestamp(cls, value: str | datetime) -> datetime:
-        """Validate a timestamp that may be a string in Hrs_Min_Sec-Day_Month_Year format."""
+        """Validate a timestamp that may be a string in Year-Month-Day_Hrs:Min:Sec format."""
         if isinstance(value, str):
-            return datetime.strptime(value, "%H_%M_%S-%d_%m_%Y")
+            return datetime.strptime(value, "%Y-%m-%d_%H:%M:%S")
         return value
 
     @field_serializer("start_time")
     def serialize_timestamp(self, value: datetime) -> str:
-        """Serialise timestamp to Hrs_Min_Sec-Day_Month_Year format."""
+        """Serialise timestamp to Year-Month-Day_Hrs:Min:Sec format."""
-        return value.strftime("%H_%M_%S-%d_%m_%Y")
+        return value.strftime("%Y-%m-%d_%H:%M:%S")
 
     @field_validator("duration", mode="before")
     @classmethod

src/openflexure_microscope_server/stitching.py

@@ -12,7 +12,6 @@ import subprocess
 import os
 from io import TextIOWrapper
 import shlex
-import time
 
 import labthings_fastapi as lt
 
@@ -289,12 +288,6 @@ class FinalStitcher(BaseStitcher):
         # Stop opening pipe blocking writing to it
         os.set_blocking(process.stdout.fileno(), False)
 
-        # TODO check if we still want this? Is it for debugging, or should it have
-        # more explanation.
-        self.logger.info(
-            time.strftime("%Y-%m-%d %H:%M:%S", time.localtime(time.time()))
-        )
-
         self._log_ongoing(process, cancel=cancel)
 
         if process.poll() == 0:

src/openflexure_microscope_server/things/smart_scan.py

@@ -11,6 +11,7 @@ from typing import Optional
 import threading
 import os
 import time
+from datetime import datetime
 from subprocess import SubprocessError
 
 from fastapi import HTTPException
@@ -267,10 +268,10 @@ class SmartScanThing(lt.Thing):
         starting_position = self._stage.position
         overlap = self.overlap
         dx, dy = self._calc_displacement_from_test_image(overlap)
-        stitch_resize = stitching.STITCHING_RESOLUTION[0] / self.save_resolution[0]
+        correlation_resize = stitching.STITCHING_RESOLUTION[0] / self.save_resolution[0]
 
         self._scan_logger.debug(
-            f"Resizing images when stitching by a factor of {stitch_resize}"
+            f"Resizing images when correlating by a factor of {correlation_resize}"
         )
 
         self._scan_logger.info(
@@ -297,10 +298,10 @@ class SmartScanThing(lt.Thing):
             dy=dy,
             autofocus_dz=autofocus_dz,
             autofocus_on=bool(autofocus_dz),
-            start_time=time.strftime("%H_%M_%S-%d_%m_%Y"),
+            start_time=datetime.now(),
             skip_background=self.skip_background,
             stitch_automatically=self.stitch_automatically,
-            stitch_resize=stitch_resize,
+            correlation_resize=correlation_resize,
             save_resolution=self.save_resolution,
         )
 
@@ -338,7 +339,7 @@ class SmartScanThing(lt.Thing):
             self._preview_stitcher = stitching.PreviewStitcher(
                 self._ongoing_scan.images_dir,
                 overlap=self._scan_data.overlap,
-                correlation_resize=self._scan_data.stitch_resize,
+                correlation_resize=self._scan_data.correlation_resize,
             )
 
             # This is the main loop of the scan!
@@ -471,7 +472,7 @@ class SmartScanThing(lt.Thing):
                 logger=self._scan_logger,
                 cancel=self._cancel,
                 scan_name=self._ongoing_scan.name,
-                stitch_resize=self._scan_data.stitch_resize,
+                correlation_resize=self._scan_data.correlation_resize,
                 overlap=self._scan_data.overlap,
             )
 
@@ -691,7 +692,7 @@ class SmartScanThing(lt.Thing):
         logger: lt.deps.InvocationLogger,
         cancel: lt.deps.CancelHook,
         scan_name: str,
-        stitch_resize: Optional[float] = None,
+        correlation_resize: Optional[float] = None,
         overlap: Optional[float] = None,
     ) -> None:
         """Generate a stitched image based on stage position metadata.
@@ -706,7 +707,7 @@ class SmartScanThing(lt.Thing):
             self._scan_dir_manager.img_dir_for(scan_name),
             logger=logger,
             overlap=overlap,
-            correlation_resize=stitch_resize,
+            correlation_resize=correlation_resize,
             stitch_tiff=self.stitch_tiff,
             scan_data_dict=scan_data_dict,
         )

tests/test_scan_data.py

@@ -45,7 +45,7 @@ def _fake_scan_data(**kwargs) -> ScanData:
         "start_time": copy(MOCK_START_TIME),
         "skip_background": True,
         "stitch_automatically": True,
-        "stitch_resize": 0.25,
+        "correlation_resize": 0.25,
         "save_resolution": (1000, 1000),
     }
     for key, value in kwargs.items():
@@ -79,7 +79,7 @@ def test_custom_serialisation():
     scan_data = _fake_scan_data()
     # Serialise to string then load directly as json
     scan_data_dict = json.loads(scan_data.model_dump_json())
-    assert scan_data_dict["start_time"] == "11_00_00-25_12_2024"
+    assert scan_data_dict["start_time"] == "2024-12-25_11:00:00"
     assert scan_data_dict["image_count"] == 0
     assert scan_data_dict["duration"] == "Unknown"
     assert scan_data_dict["scan_result"] == "Unknown"

tests/test_smart_scan.py

@@ -272,7 +272,7 @@ def _expected_scan_data():
         "autofocus_on": True,
         "skip_background": True,
         "stitch_automatically": True,
-        "stitch_resize": 0.5,
+        "correlation_resize": 0.5,
         "save_resolution": (1640, 1232),
     }
     return ScanData(start_time=datetime.now(), **expected_dict)

tests/test_stitching.py

@@ -8,7 +8,6 @@ import logging
 import os
 from copy import copy
 import uuid
-import re
 import threading
 from time import sleep
 
@@ -229,19 +228,15 @@ def test_final_stitching_command(caplog, mocker):
         )
         # For the final stitcher it will always complete before returning.
         stitcher.start(cancel=lt.deps.CancelHook(id=uuid.uuid4()))
-        # The mock command logs the inputs so should be 1 less than
+        # The mock command logs the inputs (but not the initial command) and the
-        # FINAL_EXPECTED_COMMAND as the command itself is not logged. However, there
+        # stitcher logs # "Stitching complete" when it ends.
-        # is two extra logs, The date (before the program starts) and
+        assert len(caplog.records) == len(FINAL_EXPECTED_COMMAND)
-        # "Stitching complete" when it ends.
-        assert len(caplog.records) == len(FINAL_EXPECTED_COMMAND) + 1
         for i, record in enumerate(caplog.records):
-            msg = record.message
+            msg = record.message.strip()
-            if i == 0:
+            if i == len(FINAL_EXPECTED_COMMAND) - 1:
-                assert re.match(r"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}", msg)
+                assert msg == "Stitching complete"
-            elif i == len(FINAL_EXPECTED_COMMAND):
-                assert msg.strip() == "Stitching complete"
             else:
-                assert msg.strip() == FINAL_EXPECTED_COMMAND[i]
+                assert msg == FINAL_EXPECTED_COMMAND[i + 1]
 
 
 def test_final_stitching_command_cancelled(caplog, mocker):