Apply suggestions from code review of branch bg_detect_in_camera

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

src/openflexure_microscope_server/background_detect.py

@@ -1,7 +1,7 @@
 """Provide functionality to detect if the camera is imaging sample or background.
 
 An example background image is captured by the camera and sent to classes in the module
-for analysis. Information from this images is used to detect whether an image from the
+for analysis. Information from these images is used to detect whether an image from the
 current camera field of view contains sample.
 """
 
@@ -36,7 +36,8 @@ class BackgroundDetectorStatus(BaseModel):
     """The settings for this this background detect Algorithm"""
 
     # Setting schema is a dict until LabThings FastAPI issue #154 is fixed and
-    # DataSchema can be used.
+    # DataSchema can be used directly. For now `model_dump()` must be used to dump schema
+    # to a dict.
     settings_schema: dict[str, Any]
 
 
@@ -63,6 +64,8 @@ class BackgroundDetectAlgorithm:
         return BackgroundDetectorStatus(
             ready=self.background_data is not None,
             settings=self.settings,
+            # Dump model with `model_dump()` for reason explained when defining
+            # BackgroundDetectorStatus
             settings_schema=type_to_dataschema(self.settings_data_model).model_dump(),
         )
 
@@ -161,7 +164,8 @@ class ColourChannelDetectLUV(BackgroundDetectAlgorithm):
     """Compare images with a known background in LUV colourspace.
 
     This uses an LUV colour space checking only the mean and standard deviation of the
-    U and V channels. The LUV colourspace as it collect colours together in a
+    U and V channels. The LUV colourspace as it collect colours together in a human-
+    intuitive way.
     """
 
     background_data_model: BaseModel = ChannelDistributions
@@ -219,7 +223,7 @@ class ColourChannelDetectLUV(BackgroundDetectAlgorithm):
             message = "only " + message
         return is_sample, message
 
-    def set_background(self, image: np.ndarray) -> ChannelDistributions:
+    def set_background(self, image: np.ndarray) -> None:
         """Use the input image to update the background distributions."""
         image_luv = cv2.cvtColor(image, cv2.COLOR_RGB2LUV)
 

src/openflexure_microscope_server/things/camera/__init__.py

@@ -168,6 +168,9 @@ class BaseCamera(lt.Thing):
         """Initialise the base camera, this creates the background detectors.
 
         This must be run by all child camera classes.
+
+        To add a new background detector to the server it must be added to the
+        dictionary in this function. Configuration will be added at a later date.
         """
         super().__init__()
         self.background_detectors = {"Colour Channels (LUV)": ColourChannelDetectLUV()}
@@ -515,9 +518,10 @@ class BaseCamera(lt.Thing):
 
     @background_detector_data.setter
     def background_detector_data(self, data: dict) -> None:
-        """Set the data for each detector. This should only be called on init.
+        """Set the data for each detector. Only to be used as settings are loaded from disk.
 
-        Do not call over HTTP. Would be good to have a way to make this private.
+        Do not call over HTTP. This needs to be updated once LbaThings Settings can be
+        read-only over HTTP (#484).
         """
         for name, instance_data in data.items():
             if name in self.background_detectors:
@@ -537,7 +541,7 @@ class BaseCamera(lt.Thing):
         return self.active_detector.image_is_sample(current_image)
 
     @lt.thing_action
-    def set_background(self, portal: lt.deps.BlockingPortal):
+    def set_background(self, portal: lt.deps.BlockingPortal) -> None:
         """Grab an image, and use its statistics to set the background.
 
         This should be run when the microscope is looking at an empty region,

src/openflexure_microscope_server/things/camera/opencv.py

@@ -30,7 +30,7 @@ class OpenCVCamera(BaseCamera):
 
         :param camera_index: The index of the camera to use for the microscope.
         """
-        super().__init()
+        super().__init__()
         self.camera_index = camera_index
         self._capture_thread: Optional[Thread] = None
         self._capture_enabled = False

tests/mock_things/mock_camera.py

@@ -14,11 +14,11 @@ from openflexure_microscope_server.background_detect import (
 
 
 class MockCameraThing:
-    """A mock background detect Thing that imports no code from BackgroundDetectThing.
+    """A mock camera Thing that imports no code from ``BaseCamera``.
 
     The class needs functionality added to it over time as more complex
-    mocking is needed. It imports no code from BackgroundDetectThing so that coverage
-    is not artificially inflated.
+    mocking is needed. It imports no code from ``BaseCamera or any other
+    camera Thing, so that coverage is not artificially inflated.
     """
 
     background_detector_status = BackgroundDetectorStatus(

tests/test_background_detectors.py

@@ -58,10 +58,10 @@ def test_bg_detect_base_class():
 
 
 def test_partial_base_class(background_image):
-    """Create a partial class so to initialise the base class and test other methods.
+    """Create a partial class to initialise the base class and test other methods.
 
     This test is to check that if the necessary methods are not set, that an
-    appropriate error is raise.
+    appropriate error is raised.
     """
 
     class BadAlgo1(BackgroundDetectAlgorithm):
@@ -109,7 +109,7 @@ def test_colour_channel_luv(background_image, sample_image):
     # Should be 50% background. Allowing 49.8-50.2% due to noise.
     assert 49.8 < float(match.group(1)) < 50.2
 
-    # Require 70% coverage
+    # Require 75% coverage
     cc_luv.settings = ColourChannelDetectSettings(min_sample_coverage=75.0)
 
     sample, message = cc_luv.image_is_sample(sample_image)
@@ -138,7 +138,7 @@ def test_colour_channel_luv_save_load(background_image, sample_image):
     setting_dict = cc_luv.settings.model_dump()
     data_dict = cc_luv.background_data.model_dump()
 
-    # Remove the old detector one so we don't accidentally use it!
+    # Remove the old detector so we don't accidentally use it!
     del cc_luv
 
     # Create a new instance
@@ -169,7 +169,7 @@ def test_colour_channel_luv_load_bad_data():
     """Check a type error is thrown on bad data input."""
 
     class WrongModel(BaseModel):
-        """Using a different BaseModel as this is most ilkley to cause confusion."""
+        """Using a different BaseModel as this is most likely to cause confusion."""
 
         prop1: int = 8
         prop2: str = "foo"