Apply suggestions from code review of branch jpeg-capture-in-stacking

Co-authored-by: Beth Probert <beth_probert@outlook.com>

hardware-specific-tests/picamera2/test_acquisition.py

@@ -11,6 +11,11 @@ from openflexure_microscope_server.things.camera.picamera import StreamingPiCame
 
 @fixture(scope="module")
 def client():
+    """
+    A pytest fixture that initialises a test client for the StreamingPiCamera2 Thing.
+    This fixture sets up a ThingServer, registers a StreamingPiCamera2 instance at the
+    "/camera/" endpoint, and provides a ThingClient for interacting with it during tests.
+    """
     server = ThingServer()
     server.add_thing(StreamingPiCamera2(), "/camera/")
     with TestClient(server.app) as test_client:
@@ -30,13 +35,20 @@ def test_jpeg_and_array(client):
     Check that grabbing a jpeg from the stream results in the same size
     image as a array capture or a jpeg capture.
     """
+    # Grab a jpeg from the stream
     blob = client.grab_jpeg()
     mjpeg_frame = Image.open(blob.open())
     assert mjpeg_frame
+
+    # Capture a jpeg
     blob = client.capture_jpeg(resolution="main")
     jpeg_capture = Image.open(blob.open())
     assert jpeg_capture
+
+    # Capture an array
     arrlist = client.capture_array(stream_name="main")
     array_main = np.array(arrlist)
+
+    # Verify image sizes are the same
     assert mjpeg_frame.size == jpeg_capture.size
     assert array_main.shape[1::-1] == jpeg_capture.size

hardware-specific-tests/picamera2/test_exposure_time_drift.py

@@ -1,7 +1,7 @@
 """
 Check exposure times do not drift.
 
-This can get very tedious. Recommende running pytest with -s option
+This can get very tedious. Recommend running pytest with -s option
 to monitor progress.
 """
 
@@ -38,16 +38,18 @@ def _test_exposure_time_drift(desired_time):
         print(f"Pre-capture the time is set to {pre_capture_et}")
         # Check exp is set correctly within known tolerance
         assert abs(pre_capture_et - desired_time) < exposure_tol
+
         for i in range(10):
             client.capture_jpeg(resolution="full")
             if i == 0:
                 # Exposure can update on first capture, due to frame rate restrictions
                 first_et = client.exposure_time
                 assert abs(first_et - pre_capture_et) < exposure_tol
-            frame_et = client.exposure_time
+            else:
-            print(f"Frame {i} captured with exposure time {frame_et}")
+                frame_et = client.exposure_time
-            # Check no further drift in value
+                print(f"Frame {i} captured with exposure time {frame_et}")
-            assert first_et == frame_et
+                # Check no further drift in value
+                assert first_et == frame_et
 
         # Set the exposure time to the value it already is. To check it doesn't shift
         print(f"Setting exposure time to {frame_et} to check it doesn't change")
@@ -62,6 +64,9 @@ def _test_exposure_time_drift(desired_time):
 
 
 def test_exposure_time_drift():
+    """
+    Performs the exposure time test for a range of exposure time values.
+    """
     for desired_time in [100, 1000, 10000]:
         _test_exposure_time_drift(desired_time)
 

hardware-specific-tests/picamera2/test_sensor_mode.py

@@ -24,6 +24,9 @@ def test_sensor_mode():
         for size in [(3280, 2464), (1640, 1232)]:
             client.sensor_mode = {"output_size": size, "bit_depth": 10}
             arr = np.array(client.capture_array(stream_name="raw"))
+            # Check that the array dimensions match the requested image size.
+            # Note: Numpy array shape is (y,x), but the sensor is set with (x,y)
+            # hence the need to compare index 0 with index 1.
             assert arr.shape[0] == size[1]
 
 

hardware-specific-tests/picamera2/test_tuning.py

@@ -33,14 +33,14 @@ def generate_bad_tuning():
     return bad_tuning
 
 
-def print_tuning(read_file=False):
+def print_tuning(read_file: bool = False):
     """
     Print the path of the default tuning file from the the environment variable.
 
     :param read_file: Boolean, set true to also print the file contents.
 
-    This is useful for debuging. As PyTest suppresses the printing by default the
+    This is useful for debugging. As pytest suppresses the printing by default the
-    -s option is needed when running pylint to see this.
+    -s option is needed when running pytest to see this.
     """
     key = "LIBCAMERA_RPI_TUNING_FILE"
     if key in os.environ:
@@ -52,7 +52,7 @@ def print_tuning(read_file=False):
         print("Tuning file environment variable not set")
 
 
-def _test_bad_tuning_after_good_tuning(configure):
+def _test_bad_tuning_after_good_tuning(configure: bool = False):
     """
     Load the default tuning file into the camera, re-load with a broken tuning file,
     check it errors. Finally check the default tuning file will load again afterwards.
@@ -66,14 +66,14 @@ def _test_bad_tuning_after_good_tuning(configure):
     bad_tuning = generate_bad_tuning()
     default_tuning = load_default_tuning()
     print_tuning()
-    print("opening camera with explicitly specified tuning")
+    print("opening camera with default tuning")
     with Picamera2(tuning=default_tuning) as cam:
         print_tuning()
         if configure:
             cam.configure(cam.create_preview_configuration())
     del cam
     recalibrate_utils.recreate_camera_manager()
-    print(f"Opening camera with tuning['version'] = {bad_tuning['version']}")
+    print(f"Opening camera with bad tuning - ['version'] = {bad_tuning['version']}")
     with pytest.raises(IndexError):
         # The bad version should cause a problem
         cam = Picamera2(tuning=bad_tuning)

pyproject.toml

@@ -40,7 +40,7 @@ dev = [
   "matplotlib~=3.10"
 ]
 pi = [
-  "picamera2~=0.3.12",
+  "picamera2~=0.3.27",
 ]
 
 [project.scripts]

src/openflexure_microscope_server/things/autofocus.py

@@ -294,6 +294,7 @@ class AutofocusThing(Thing):
 
         stack_z_range = stack_dz * (images_to_capture - 1)
         if stack_z_range > 0:
+            # Perform backlash corrected move. See issue #420
             stage.move_relative(z=-(STACK_OVERSHOOT + stack_z_range / 2))
             stage.move_relative(z=STACK_OVERSHOOT)
         time.sleep(0.3)
@@ -348,7 +349,7 @@ class AutofocusThing(Thing):
         images_dir: str,
         stack_dir: str,
         logger: InvocationLogger,
-    ):
+    ) -> None:
         """Gets a list of images in a folder (stack_dir), sorts them by filesize, and copies the sharpest
         image to images dir."""
         image_list = glob.glob(os.path.join(stack_dir, "*"))

src/openflexure_microscope_server/things/camera/__init__.py

@@ -31,6 +31,8 @@ class JPEGBlob(Blob):
 
 
 class PNGBlob(Blob):
+    """A class representing a PNG image as a LabThings FastAPI Blob"""
+
     media_type: str = "image/png"
 
 

src/openflexure_microscope_server/things/smart_scan.py

@@ -377,10 +377,13 @@ class SmartScanThing(Thing):
         return (next_point[0], next_point[1], z_estimate)
 
     @_scan_running
-    def _calc_displacement_from_test_image(self, overlap):
+    def _calc_displacement_from_test_image(self, overlap: int) -> tuple[int, int]:
         """
         Take a test image and use camera stage mapping to calculate x and y displacement
 
+        :param overlap: The desired overlap as a fraction of the image. i.e. 0.5 means
+        that each image should overlap its nearest neighbour by 50%.
+
         Return (dx, dy) - the x and y displacments in steps
         """
         test_jpg = self._cam.grab_jpeg()
@@ -422,7 +425,9 @@ class SmartScanThing(Thing):
         dx, dy = self._calc_displacement_from_test_image(overlap)
         stitch_resize = STITCHING_RESOLUTION[0] / self.capture_resolution[0]
 
-        self._scan_logger.debug(f"Resizing images when stitching by {stitch_resize}")
+        self._scan_logger.debug(
+            f"Resizing images when stitching by a factor of {stitch_resize}"
+        )
 
         self._scan_logger.info(
             f"Based on an overlap of {overlap}, we will make steps of {dx}, {dy}"