Add flake8-simplicity rules

pyproject.toml

@@ -119,7 +119,7 @@ select = [
     "PT", # pytest linting
     "RET", # Consistent clear return statements
     "RSE", # Raise parentheses
-#    "SIM", # Simplifications detected
+    "SIM", # Simplifications detected
     "ARG", # unused arguments
     "C90", # McCabe complexity!
     "NPY", # Numpy linting
@@ -142,6 +142,8 @@ ignore = [
                # magic (such as 255 when doing uint8 maths)
     "PT011", # Ifnore pytest.raises being used on too gereneral expectations without a
              # match. We may need to revisit this.
+    "SIM105", # Not enforcing use of `contextlib.suppress(NotConnectedToServerError)`
+              # instead of `try`-`except`-`pass`
 ]
 
 [tool.ruff.lint.per-file-ignores]

src/openflexure_microscope_server/scan_directories.py

@@ -210,9 +210,8 @@ class ScanDirectoryManager:
         not exist.
         """
         file_path = os.path.join(self.path_for(scan_name), filename)
-        if check_exists:
+        if check_exists and not os.path.exists(file_path):
-            if not os.path.exists(file_path):
+            return None
-                return None
         return file_path
 
     @requires_lock
@@ -225,9 +224,8 @@ class ScanDirectoryManager:
         then the path is returned anyway
         """
         file_path = os.path.join(self.img_dir_for(scan_name), filename)
-        if check_exists:
+        if check_exists and not os.path.exists(file_path):
-            if not os.path.exists(file_path):
+            return None
-                return None
         return file_path
 
     def get_final_stitch_path(self, scan_name: str) -> Optional[str]:
@@ -528,7 +526,8 @@ class ScanDirectory:
         """
         zip_fname = os.path.join(self.dir_path, "images.zip")
 
-        if os.path.isfile(zip_fname):
+        # Use noqa as converting this into a 1 liner is not more readable.
+        if os.path.isfile(zip_fname):  # noqa: SIM108
             # get a list of files in the existing zip
             zip_files = get_files_in_zip(zip_fname)
         else:

src/openflexure_microscope_server/scan_planners.py

@@ -163,10 +163,7 @@ class ScanPlanner:
 
         # If focussed locations exist return closest location, favouring most recent
         closest_pos = self.closest_focus_site(next_location)
-        if closest_pos is None:
+        z = None if closest_pos is None else closest_pos[2]
-            z = None
-        else:
-            z = closest_pos[2]
 
         return next_location, z
 
@@ -363,10 +360,7 @@ class SmartSpiral(ScanPlanner):
 
         # 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 if closest_pos is None else closest_pos[2]
-            z = None
-        else:
-            z = closest_pos[2]
 
         return next_location, z
 

src/openflexure_microscope_server/stitching.py

@@ -165,9 +165,7 @@ class PreviewStitcher(BaseStitcher):
         with self._popen_lock:
             if self._popen_obj is None:
                 return False
-            if self._popen_obj.poll() is None:
+            return self._popen_obj.poll() is None
-                return True
-            return False
 
     def wait(self, cancel: lt.deps.CancelHook) -> None:
         """Wait for this preview stitch to return.

src/openflexure_microscope_server/things/smart_scan.py

@@ -334,9 +334,8 @@ class SmartScanThing(lt.Thing):
         """Manage the stitching threads, starting them if needed and not already running."""
         # Assume 4 images means at least one offset in x and y, making the stitching
         # well constrained.
-        if self._scan_data.image_count > 3:
+        if self._scan_data.image_count > 3 and not self._preview_stitcher.running:
-            if not self._preview_stitcher.running:
+            self._preview_stitcher.start()
-                self._preview_stitcher.start()
 
     @_scan_running
     def _run_scan(self) -> None:

tests/test_logging.py

@@ -22,15 +22,14 @@ def test_no_warnings_if_correct_permissions(caplog):
     """
     # Reset handler at start of test
     ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler()
-    with caplog.at_level(logging.WARNING):
+    with caplog.at_level(logging.WARNING), tempfile.TemporaryDirectory() as tmpdir:
-        with tempfile.TemporaryDirectory() as tmpdir:
+        ofm_logging.configure_logging(tmpdir)
-            ofm_logging.configure_logging(tmpdir)
+        assert len(caplog.records) == 0
-            assert len(caplog.records) == 0
+        with open(ofm_logging.OFM_LOG_FILE, "r", encoding="utf-8") as log_file:
-            with open(ofm_logging.OFM_LOG_FILE, "r", encoding="utf-8") as log_file:
+            log_txt = log_file.read()
-                log_txt = log_file.read()
+        assert "OFM server root logger has been set up at INFO level" in log_txt
-            assert "OFM server root logger has been set up at INFO level" in log_txt
+        root_logger = logging.getLogger()
-            root_logger = logging.getLogger()
+        assert ofm_logging.OFM_HANDLER in root_logger.handlers
-            assert ofm_logging.OFM_HANDLER in root_logger.handlers
 
 
 def test_permission_error_raises_warning(mocker, caplog):
@@ -41,26 +40,24 @@ def test_permission_error_raises_warning(mocker, caplog):
     )
     # Reset handler at start of test
     ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler()
-    with caplog.at_level(logging.WARNING):
+    with caplog.at_level(logging.WARNING), tempfile.TemporaryDirectory() as tmpdir:
-        with tempfile.TemporaryDirectory() as tmpdir:
+        ofm_logging.configure_logging(tmpdir)
-            ofm_logging.configure_logging(tmpdir)
+        assert len(caplog.records) == 1
-            assert len(caplog.records) == 1
+        # Check OFM logger is added even if the file logger couldn't be.
-            # Check OFM logger is added even if the file logger couldn't be.
+        root_logger = logging.getLogger()
-            root_logger = logging.getLogger()
+        assert ofm_logging.OFM_HANDLER in root_logger.handlers
-            assert ofm_logging.OFM_HANDLER in root_logger.handlers
 
 
 def test_making_log_dir(caplog):
     """Check that configure_logging will make a dir if needed."""
     # Reset handler at start of test
     ofm_logging.OFM_HANDLER = ofm_logging.OFMHandler()
-    with caplog.at_level(logging.WARNING):
+    with caplog.at_level(logging.WARNING), tempfile.TemporaryDirectory() as tmpdir:
-        with tempfile.TemporaryDirectory() as tmpdir:
+        log_dir = os.path.join(tmpdir, "new_dir")
-            log_dir = os.path.join(tmpdir, "new_dir")
+        assert not os.path.isdir(log_dir)
-            assert not os.path.isdir(log_dir)
+        ofm_logging.configure_logging(log_dir)
-            ofm_logging.configure_logging(log_dir)
+        assert len(caplog.records) == 0
-            assert len(caplog.records) == 0
+        assert os.path.isdir(log_dir)
-            assert os.path.isdir(log_dir)
 
 
 def test_max_logs():

tests/test_scan_planners.py

@@ -23,8 +23,8 @@ def test_enforce_xy_tuple():
         with pytest.raises(TypeError):
             scan_planners.enforce_xy_tuple(value)
 
-    assert (1, 6) == scan_planners.enforce_xy_tuple((1, 6))
+    assert scan_planners.enforce_xy_tuple((1, 6)) == (1, 6)
-    assert (1, 6) == scan_planners.enforce_xy_tuple([1, 6])
+    assert scan_planners.enforce_xy_tuple([1, 6]) == (1, 6)
 
 
 def test_enforce_xyz_tuple():
@@ -39,8 +39,8 @@ def test_enforce_xyz_tuple():
         with pytest.raises(TypeError):
             scan_planners.enforce_xyz_tuple(value)
 
-    assert (1, 6, 2) == scan_planners.enforce_xyz_tuple((1, 6, 2))
+    assert scan_planners.enforce_xyz_tuple((1, 6, 2)) == (1, 6, 2)
-    assert (1, 6, 6) == scan_planners.enforce_xyz_tuple([1, 6, 6])
+    assert scan_planners.enforce_xyz_tuple([1, 6, 6]) == (1, 6, 6)
 
 
 def test_base_class_not_implemented():