Apply suggestions from code review of branch fast-stack

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

src/openflexure_microscope_server/scan_planners.py

@@ -357,7 +357,7 @@ class SmartSpiral(ScanPlanner):
         for this location. This overrides the default behaviour of ScanPlanner
         to take the lowest value of nearest neighbours as this works best for smart stack
 
-        Note z-position may be None! This indicates that the current z, position
+        Note z-position may be None! This indicates that the current z position
         should be used.
         """
         if self.scan_complete:
@@ -365,7 +365,7 @@ class SmartSpiral(ScanPlanner):
 
         next_location = self._remaining_locations[0]
 
-        # If focussed locations exist, return the neighbour with the lowest z position
+        # If focused locations exist, return the neighbour with the lowest z position
         closest_pos = self.select_nearby_focus_site(next_location)
         if closest_pos is None:
             z = None
@@ -381,7 +381,7 @@ class SmartSpiral(ScanPlanner):
         autofocus and restart. Starting too low just requires extra movements in +z.
         Nearby is defined as within NEIGHBOUR_CUTOFF times the distance to the closest neighbour.
 
-        Returns None if there if no focussed locations are present
+        Returns None if there if no focused locations are present
         """
         if not self._focused_locations:
             return None
@@ -390,7 +390,7 @@ class SmartSpiral(ScanPlanner):
         current_pos = np.array(xy_pos, dtype="float64")
         path_pos = np.array(self._focused_locations, dtype="float64")[:, :2]
 
-        # Use linalg.norm to calculate the direct distance bweween the points
+        # Use linalg.norm to calculate the direct distance between the points
         # Note linalg.norm always uses float64
         dists = np.linalg.norm((path_pos - current_pos), axis=1)
 

src/openflexure_microscope_server/things/autofocus.py

@@ -37,7 +37,7 @@ class StackParams:
     :param min_images_to_test: The minimum number of images in the stack before, the
     stack is evaluated for focus. As more images are captured evaluation of the focus
     is always evaluated with the same number of images. i.e. if min_images_to_test=9,
-    then 9 images are captured, if the stack is not well focussed, a 10th image is
+    then 9 images are captured, if the stack is not well focused, a 10th image is
     captured and images 2 to 10 are evaluated for focus
     :param autofocus_dz: The number of steps in a full autofocus (when required)
     :param images_dir: The directory to save images to disk
@@ -582,7 +582,7 @@ class AutofocusThing(Thing):
 
         # If the sharpest image isn't found within the maximum number of images
         # end the loop and return "restart"
-        while len(captures) <= stack_parameters.max_images_to_test:
+        while len(captures) < stack_parameters.max_images_to_test:
             time.sleep(stack_parameters.settling_time)
 
             # Append a new image to the stack

src/openflexure_microscope_server/things/camera/__init__.py

@@ -69,7 +69,7 @@ class CameraMemoryBuffer:
 
     def add_image(
         self, image: Any, metadata: Optional[dict] = None, buffer_max: int = 1
-    ) -> None:
+    ) -> int:
         """
         Add an image to the Memory buffer
 

tests/test_camera_buffer.py

@@ -27,7 +27,8 @@ def random_image():
 
 def random_metadata():
     """Create a misc dictionary to pretend to be metadata"""
-    # Not very metadata like, but we are just the same dict it is returned
+    # Not very metadata like, but we are just checking that the same dict it is
+    # returned
     return {"a": randint(1, 100), "b": randint(1, 100)}
 
 
@@ -122,7 +123,7 @@ def test_buffer_size_changing():
     with pytest.raises(NoImageInMemoryError):
         mem_buf.get_image(buffer_id2)
     returned_image3, _ = mem_buf.get_image(buffer_id3)
-    # Image 2 the expected image
+    # Image 3 the expected image
     assert misc_image3 is returned_image3
 
 
@@ -134,9 +135,10 @@ def test_capture_two_images_get_without_id():
     mem_buf.add_image(misc_image1, buffer_max=2)
     mem_buf.add_image(misc_image2, buffer_max=2)
     returned_image, _ = mem_buf.get_image()
-    # It is the same image
+    # When buffer_id is not specified, the most recent image (image2) is expected to
+    # be retrieved
     assert returned_image is misc_image2
-    # All were wiped from memory
+    # Check all images were wiped from memory, but trying get_image without an id
     with pytest.raises(NoImageInMemoryError):
         mem_buf.get_image()
 
@@ -184,7 +186,7 @@ def test_clear_buffer():
 
 
 def test_get_metadata_too():
-    """Capture 10 images clear the buffer and check they are gone"""
+    """Capture 10 images with metadata and check metadata is returned as expected"""
     mem_buf = CameraMemoryBuffer()
 
     images = []

tests/test_stack.py

@@ -215,9 +215,9 @@ def test_capture_filename_matches_regex():
 
 
 @given(st.integers(min_value=0, max_value=5000))
-def test_retriaval_of_captures(start):
+def test_retrieval_of_captures(start):
     """
-    For 20 random captures each chan be retried correctly
+    For 20 random captures, check each can be retrieved correctly by id
     """
     captures = [random_capture(start + i) for i in range(20)]