From 071d62e8a0aa2913e8dd4506081b3beb8fa42ac2 Mon Sep 17 00:00:00 2001 From: Julian Stirling Date: Fri, 5 Jun 2026 15:53:04 +0000 Subject: [PATCH] Apply suggestions from code review of branch camera_modes Co-authored-by: Joe Knapper --- src/openflexure_microscope_server/things/__init__.py | 4 ++-- .../things/camera/__init__.py | 8 +++++--- .../things/camera/opencv.py | 4 ++-- .../things/camera/picamera.py | 10 +++++----- .../things/camera/simulation.py | 8 +++----- .../things/scan_workflows.py | 2 +- .../picamera2/test_acquisition.py | 6 +++--- tests/integration_tests/test_actions.py | 2 +- tests/unit_tests/test_base_camera.py | 4 ++-- tests/unit_tests/test_camera_buffer.py | 6 +++--- tests/unit_tests/test_ofm_thing.py | 2 +- 11 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/openflexure_microscope_server/things/__init__.py b/src/openflexure_microscope_server/things/__init__.py index cfa95023..b5855e6e 100644 --- a/src/openflexure_microscope_server/things/__init__.py +++ b/src/openflexure_microscope_server/things/__init__.py @@ -68,9 +68,9 @@ class OFMThing(lt.Thing): """Create a ``RelativeDataPath`` object with this Thing set as the saving Thing. :param path: The relative path within the data directory of this Thing's data - dir that the data shudl be saved. + dir that the data should be saved to. :param absolute: Set to True if the current path is absolute. A relative path - will be returned. An validation error will be raised if the absolute path + will be returned. A validation error will be raised if the absolute path is not within the data directory. :return: A ``RelativeDataPath`` object with the saving Thing already set. diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 9c80a03d..fa32d6d6 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/src/openflexure_microscope_server/things/camera/__init__.py @@ -230,8 +230,10 @@ class BaseCamera(OFMThing, ABC): # would be ideal. self._default_background_detector = "bg_channel_deviations_luv" self._background_detector_name: Optional[str] = None + self._framerate_monitor_running = False - if "default" and "full_resolution" not in self.streaming_modes: + required_modes = ("default", "full_resolution") + if not all(mode in self.streaming_modes for mode in required_modes): raise KeyError( f"Camera {type(self).__name__} doesn't define both a 'default' and a " "'full_resolution' streaming mode." @@ -559,7 +561,7 @@ class BaseCamera(OFMThing, ABC): def capture_downsampled_array(self) -> NDArray: """Acquire one image from the camera, downsample, and return as an array. - * The array is downsamples by the thing property `downsampled_array_factor`. + * The array is downsampled by the thing property `downsampled_array_factor`. * The default capture array arguments are used. This method provides the interface expected by the camera_stage_mapping. @@ -718,7 +720,7 @@ class BaseCamera(OFMThing, ABC): """Capture a PIL image from the camera. This unlike the ``grab_*`` methods this may pause the stream or temporarily - switch streaming mode to capute the image if required by the mode. + switch streaming mode to capture the image if required by the mode. """ @lt.action diff --git a/src/openflexure_microscope_server/things/camera/opencv.py b/src/openflexure_microscope_server/things/camera/opencv.py index 76d94502..4f31ea8a 100644 --- a/src/openflexure_microscope_server/things/camera/opencv.py +++ b/src/openflexure_microscope_server/things/camera/opencv.py @@ -149,7 +149,7 @@ class OpenCVCamera(BaseCamera): raise NotImplementedError( "OpenCV camera camera doesn't support raw capture." ) - # Warn if the capture mode is incorrect, but don't read the cooerced value as + # Warn if the capture mode is incorrect, but don't read the coerced value as # this camera only supports one mode. self._validate_capture_mode(capture_mode) ret, frame = self.cap.read() @@ -159,7 +159,7 @@ class OpenCVCamera(BaseCamera): def _capture_image(self, capture_mode: str = "standard") -> Image.Image: """Acquire one image from the camera and return as a PIL image.""" - # Warn if the capture mode is incorrect, but don't read the cooerced value as + # Warn if the capture mode is incorrect, but don't read the coerced value as # this camera only supports one mode. self._validate_capture_mode(capture_mode) return Image.fromarray(self.capture_as_array()) diff --git a/src/openflexure_microscope_server/things/camera/picamera.py b/src/openflexure_microscope_server/things/camera/picamera.py index 2f1ffbfb..c769947d 100644 --- a/src/openflexure_microscope_server/things/camera/picamera.py +++ b/src/openflexure_microscope_server/things/camera/picamera.py @@ -536,8 +536,8 @@ class StreamingPiCamera2(BaseCamera, ABC): If the camera is already in the correct mode, the stream isn't paused and this is the same as using ``self._streaming_picamera()``. - Otherwise, pause stream, and switch switch mode. Mode is reset and stream - restarts stream after the context manager closes. + Otherwise, pause stream, and switch mode. Mode is reset and stream + restarts after the context manager closes. """ required_streaming_mode = capture_mode_info.streaming_mode if ( @@ -568,7 +568,7 @@ class StreamingPiCamera2(BaseCamera, ABC): """Acquire one image from the camera and return it as a PIL Image. :param capture_mode: The capture mode to use. See the description field of each - mode for more detail in ``capture_modes`` for more detail. + mode in ``capture_modes`` for more detail. :raises TimeoutError: if this time is exceeded during capture. """ @@ -594,13 +594,13 @@ class StreamingPiCamera2(BaseCamera, ABC): :param capture_mode: (Optional) The name of the capture mode as defined by the camera. - :param raw: Whether to capture RAW data. Capturing RAW data may infore some + :param raw: Whether to capture RAW data. Capturing RAW data may ignore some of the camera mode settings. :raises TimeoutError: if this time is exceeded during capture. """ if raw: - # Raw cannot used _capture_image. + # Raw cannot use _capture_image. capture_mode = self._validate_capture_mode(capture_mode) capture_mode_info = self.capture_modes[capture_mode] diff --git a/src/openflexure_microscope_server/things/camera/simulation.py b/src/openflexure_microscope_server/things/camera/simulation.py index 405ff3fd..e5fe90c1 100644 --- a/src/openflexure_microscope_server/things/camera/simulation.py +++ b/src/openflexure_microscope_server/things/camera/simulation.py @@ -496,10 +496,8 @@ class SimulatedCamera(BaseCamera): Setting this to True will result in an error. """ if raw is True: - raise NotImplementedError( - "Simulation camera camera doesn't support raw capture." - ) - # Warn if the capture mode is incorrect, but don't read the cooerced value as + raise NotImplementedError("Simulation camera doesn't support raw capture.") + # Warn if the capture mode is incorrect, but don't read the coerced value as # this camera only supports one mode. self._validate_capture_mode(capture_mode) return np.array(self.generate_frame()) @@ -509,7 +507,7 @@ class SimulatedCamera(BaseCamera): It is used for capture to memory. """ - # Warn if the capture mode is incorrect, but don't read the cooerced value as + # Warn if the capture mode is incorrect, but don't read the coerced value as # this camera only supports one mode. self._validate_capture_mode(capture_mode) return self.generate_frame() diff --git a/src/openflexure_microscope_server/things/scan_workflows.py b/src/openflexure_microscope_server/things/scan_workflows.py index f8d1402c..40febefd 100644 --- a/src/openflexure_microscope_server/things/scan_workflows.py +++ b/src/openflexure_microscope_server/things/scan_workflows.py @@ -129,7 +129,7 @@ class ScanWorkflow(Generic[SettingModelType], lt.Thing): """Return the save resolution as determined by a test image.""" # Capture an example image. image = self._cam._capture_image(capture_mode=self.capture_mode) - # Check side to create a unit faction for downsampling. + # Check size to create a unit faction for downsampling. return image.size def pre_scan_routine(self, settings: SettingModelType) -> None: diff --git a/tests/hardware_specific_tests/picamera2/test_acquisition.py b/tests/hardware_specific_tests/picamera2/test_acquisition.py index 0df00962..ed864e1b 100644 --- a/tests/hardware_specific_tests/picamera2/test_acquisition.py +++ b/tests/hardware_specific_tests/picamera2/test_acquisition.py @@ -52,9 +52,9 @@ def test_format(picamera_client): image_format="png", retain_image=True, ) - jpeg_capture = Image.open(blob.open()) - jpeg_capture.verify() - assert jpeg_capture.format == "PNG" + png_capture = Image.open(blob.open()) + png_capture.verify() + assert png_capture.format == "PNG" def test_standard_capture_size(picamera_client): diff --git a/tests/integration_tests/test_actions.py b/tests/integration_tests/test_actions.py index 54f127dd..d16df380 100644 --- a/tests/integration_tests/test_actions.py +++ b/tests/integration_tests/test_actions.py @@ -53,7 +53,7 @@ def test_grab_jpeg(simulation_test_env): def test_capture_and_metadata(simulation_test_env, image_format, caplog): """Capture an image and check a attributes. - - Check that the position is encoded into the image metadata. + - Check that the position is encoded into the image metadata - Check the dimensions - Check the format """ diff --git a/tests/unit_tests/test_base_camera.py b/tests/unit_tests/test_base_camera.py index 4722568f..6efc72e3 100644 --- a/tests/unit_tests/test_base_camera.py +++ b/tests/unit_tests/test_base_camera.py @@ -86,7 +86,7 @@ class MemorySaveTestCase: SAVE_TEST_CASES = [ - # Default test case is a jpeg, ckec it works with all extensions. + # Default test case is a jpeg, check it works with all extensions. MemorySaveTestCase("foobar.jpeg"), MemorySaveTestCase("foobar.jpg"), MemorySaveTestCase("foobar.JPEG"), @@ -103,7 +103,7 @@ SAVE_TEST_CASES = [ @pytest.mark.parametrize("test_case", SAVE_TEST_CASES) def test_save_from_memory(test_case, test_env, mocker): - """Check the correct timage is retrieved and saved with correct settings.""" + """Check the correct image is retrieved and saved with correct settings.""" camera = test_env.get_thing_by_type(SimulatedCamera) camera._memory_buffer = mocker.Mock() camera._add_metadata_to_capture = mocker.Mock() diff --git a/tests/unit_tests/test_camera_buffer.py b/tests/unit_tests/test_camera_buffer.py index aa373299..fece4900 100644 --- a/tests/unit_tests/test_camera_buffer.py +++ b/tests/unit_tests/test_camera_buffer.py @@ -90,7 +90,7 @@ def test_get_two_images(): ) returned_image1, _, _ = mem_buf.get_image(buffer_id1) returned_image2, _, _ = mem_buf.get_image(buffer_id2) - # It they the same images + # Assert they are the same images assert misc_image1 is returned_image1 assert misc_image2 is returned_image2 # They are removed from memory @@ -134,7 +134,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 3 the expected image + # Image 3 is the expected image assert misc_image3 is returned_image3 @@ -226,7 +226,7 @@ def test_get_metadata_too(): def test_mode_is_returned(): - """Capture 10 images with metadata and check metadata is returned as expected.""" + """Check that the correct mode name is returned with the image from the buffer.""" mem_buf = CameraMemoryBuffer() misc_image = random_image() buffer_id = mem_buf.add_image(misc_image, random_metadata(), "standard") diff --git a/tests/unit_tests/test_ofm_thing.py b/tests/unit_tests/test_ofm_thing.py index ae9ac217..593c49d5 100644 --- a/tests/unit_tests/test_ofm_thing.py +++ b/tests/unit_tests/test_ofm_thing.py @@ -92,7 +92,7 @@ def test_saving_thing_propagates_on_join(): path2 = path.join("bar") assert not path2.save_location_set - # Set thing and check joiend path is as expected + # Set thing and check joined path is as expected path2.set_saving_thing(thing) assert path2.save_location_set assert path2.abs_data_path == os.path.join("data", "thing", "foo", "bar")