Merge branch 'more-linting' into 'v3'

Add some extra rules to ruff, also add a more complete ruff config that fails

See merge request openflexure/openflexure-microscope-server!241

.coveragerc

@@ -0,0 +1,4 @@
+[run]
+concurrency = multiprocessing, thread
+parallel = true
+sigterm = true

.gitignore

@@ -48,6 +48,7 @@ htmlcov/
 .cache
 nosetests.xml
 coverage.xml
+report.xml
 pytest_report.xml
 
 # Translations
@@ -82,4 +83,4 @@ openflexure_microscope/cobertura.xml
 /openflexure_settings/
 
 # web app build
-/src/openflexure_microscope_server/static/
+/src/openflexure_microscope_server/static/

.gitlab-ci.yml

@@ -58,14 +58,24 @@ meta-build-image:
     - tags
     - web
 
-# Python formatting and static analysis with ruff
+# Python static analysis with ruff
 ruff-lint:
   stage: analysis
   extends: .python
   script:
     - ruff check
 
-# Python formatting and static analysis with ruff
+# Increased checking of the code with ruff this is allowed to fail
+# while we improve the code quality. Rules from this are
+# slowly moved into the main check
+ruff-lint-increased:
+  stage: analysis
+  extends: .python
+  allow_failure: true
+  script:
+    - ruff --config ruff-increased-checking.toml check
+
+# Python formatting with ruff
 ruff-format:
   stage: analysis
   extends: .python
@@ -94,11 +104,8 @@ pytest:
   stage: testing
   extends: .python
   script:
-    - >
+    - pytest
-      pytest --cov --cov-report term
+#    --gitlab-code-quality-report=pytest-warnings.json # can restore when it installs ok
-      --cov-report xml:coverage.xml
-      --junitxml=report.xml
-#      --gitlab-code-quality-report=pytest-warnings.json # can restore when it installs ok
   coverage: '/(?i)total.*? (100(?:\.0+)?\%|[1-9]?\d(?:\.\d+)?\%)$/'
   artifacts:
     when: always

pyproject.toml

@@ -73,8 +73,48 @@ target-version = "py311"
 [tool.pytest.ini_options]
 addopts = [
     "--import-mode=importlib",
+    "--cov=openflexure_microscope_server",
+    "--cov-report=term",
+    "--cov-report=xml:coverage.xml",
+    "--junitxml=report.xml",
 ]
 
+norecursedirs = [".git", "build", "node_modules"]
+
 [tool.ruff.format]
 # Use native line endings for all files
 line-ending = "native"
+
+[tool.ruff.lint]
+
+# Slowly add linting rules from ruff-increased-checking.toml commented
+# out rules can be checked by running
+# ruff --config ruff-increased-checking.toml check
+select = [
+    "E", # pycodestyle errors
+    "W", #pycodestyle warnings
+    "F", # PyFlakes
+#    "PL", # Pylint (a subset of, catches far less than pylint does)
+#    "B", # Flake8 bugbear
+    "A", # Flake8 builtins checker
+#    "C", # Flake8 comprehensions
+#    "FIX", #TODO/FIXME's in code. These should be added during develoment for ongoing tasks.
+           # If they are not fixed before merge they should be converted to GitLab issues
+#    "LOG", # Flake8 logging issues (pedantic logger formatting issues can be added with "G")
+#    "T20", # Warns for print statments, production code should log
+#    "PT", # pytest linting
+#    "RET", # Consistent clear return statments
+    "RSE", # Raise parentheses
+#    "SIM", # Simplifications detected
+#    "ARG", # unused arguments
+#    "C90", # McCabe complexity!
+    "NPY", # Numpy linting
+    "N", # PEP8 naming
+#    "DOC", # Enforce argument, return, and raise to be mentioned in docstrings there is
+           # also "D" but this even warns about docstring punctuation. Perhaps a step too
+           # far?
+]
+
+ignore = [
+    "E501" # Ignore long lines for now
+]

pytest.ini

@@ -1,2 +0,0 @@
-[pytest]
-norecursedirs = .git build node_modules

ruff-increased-checking.toml

@@ -0,0 +1,37 @@
+# This can be passed into ruff with nano ruff-increased-checking.toml
+#
+# Once the codebase is cleaner this can be added to pyproject.toml
+# under the [tool.ruff.lint] heading 
+#
+# Select more than the bare minimum checking in ruff
+# This is only a subset of possible rules
+# See: https://docs.astral.sh/ruff/rules
+# 
+# It will take a long time to get this all passing. We should
+# slowly move rules into the pyproject.toml as we improve the
+# codebase
+
+select = [
+    "E", # pycodestyle errors
+    "W", #pycodestyle warnings
+    "F", # PyFlakes
+    "PL", # Pylint (a subset of, catches far less than pylint does)
+    "B", # Flake8 bugbear
+    "A", # Flake8 builtins checker
+    "C", # Flake8 comprehensions
+    "FIX", #TODO/FIXME's in code. These should be added during develoment for ongoing tasks.
+           # If they are not fixed before merge they should be converted to GitLab issues
+    "LOG", # Flake8 logging issues (pedantic logger formatting issues can be added with "G")
+    "T20", # Warns for print statments, production code should log
+    "PT", # pytest linting
+    "RET", # Consistent clear return statments
+    "RSE", # Raise parentheses
+    "SIM", # Simplifications detected
+    "ARG", # unused arguments
+    "C90", # McCabe complexity!
+    "NPY", # Numpy linting
+    "N", # PEP8 naming
+    "DOC", # Enforce argument, return, and raise to be mentioned in docstrings there is
+           # also "D" but this even warns about docstring punctuation. Perhaps a step too
+           # far?
+]

src/openflexure_microscope_server/things/background_detect.py

@@ -1,5 +1,3 @@
-# ruff: noqa: E722
-
 from typing import Mapping, Optional
 import cv2
 import numpy as np
@@ -88,8 +86,8 @@ class BackgroundDetectThing(Thing):
         current_image = np.array(Image.open(current_image.open()))
 
         # we're working in the LUV colourspace as it collect colours together in a human-intuitive way
-        current_image_LUV = cv2.cvtColor(current_image, cv2.COLOR_RGB2LUV)
+        current_image_luv = cv2.cvtColor(current_image, cv2.COLOR_RGB2LUV)
-        mask = self.background_mask(current_image_LUV)
+        mask = self.background_mask(current_image_luv)
         return np.count_nonzero(mask) / np.prod(mask.shape) * 100
 
     @thing_action
@@ -114,11 +112,11 @@ class BackgroundDetectThing(Thing):
         background = np.array(Image.open(background.open()))
 
         # we're working in the LUV colourspace as it collect colours together in a human-intuitive way
-        background_LUV = cv2.cvtColor(background, cv2.COLOR_RGB2LUV)
+        background_luv = cv2.cvtColor(background, cv2.COLOR_RGB2LUV)
 
-        ch1 = (background_LUV.T[0]).flatten()
+        ch1 = (background_luv.T[0]).flatten()
-        ch2 = (background_LUV.T[1]).flatten()
+        ch2 = (background_luv.T[1]).flatten()
-        ch3 = (background_LUV.T[2]).flatten()
+        ch3 = (background_luv.T[2]).flatten()
 
         points = np.array([np.asarray(ch1), np.asarray(ch2), np.asarray(ch3)]).T
 

src/openflexure_microscope_server/things/camera/simulation.py

@@ -71,18 +71,18 @@ class SimulatedCamera(BaseCamera):
             sprite[rr < i] = 255
             self.sprites.append(sprite)
 
-    def generate_blobs(self, N: int = 1000):
+    def generate_blobs(self, n_blobs: int = 1000):
         """Generate coordinates of blobs
 
         Blobs are characterised by X, Y, sprite
         We also generate a KD tree to rapidly find blobs in an image
         """
-        self.blobs = np.zeros((N, 3))
+        self.blobs = np.zeros((n_blobs, 3))
         rng = np.random.default_rng()
         w = np.max(self.glyph_shape)
-        self.blobs[:, 0] = rng.uniform(w / 2, self.canvas_shape[0] - w / 2, N)
+        self.blobs[:, 0] = rng.uniform(w / 2, self.canvas_shape[0] - w / 2, n_blobs)
-        self.blobs[:, 1] = rng.uniform(w / 2, self.canvas_shape[1] - w / 2, N)
+        self.blobs[:, 1] = rng.uniform(w / 2, self.canvas_shape[1] - w / 2, n_blobs)
-        self.blobs[:, 2] = rng.choice(len(self.sprites), N)
+        self.blobs[:, 2] = rng.choice(len(self.sprites), n_blobs)
 
     def generate_canvas(self):
         """Generate a blank canvas"""

src/openflexure_microscope_server/things/camera_stage_mapping.py

@@ -296,7 +296,9 @@ class CameraStageMapper(Thing):
     def assert_calibrated(self):
         """Raise an exception if the image_to_stage_displacement matrix is not set"""
         if self.image_to_stage_displacement_matrix is None:
-            raise CSMUncalibratedError()
+            # Disable check of no message in raised exception as the message is explicitly
+            # added by CSMUncalibratedError
+            raise CSMUncalibratedError()  # noqa: RSE102
 
     @thing_property
     def last_calibration(self) -> Optional[Dict]:

src/openflexure_microscope_server/things/smart_scan.py

@@ -73,7 +73,7 @@ def unpack_autofocus(scan_data):
     """
     jpeg_times = scan_data["jpeg_times"]
     jpeg_sizes = scan_data["jpeg_sizes"]
-    jpeg_sizes_MB = [x / 10**3 for x in jpeg_sizes]
+    jpeg_sizes_mb = [x / 10**3 for x in jpeg_sizes]
     stage_times = scan_data["stage_times"]
     stage_positions = scan_data["stage_positions"]
     stage_height = [pos[2] for pos in stage_positions]
@@ -86,7 +86,7 @@ def unpack_autofocus(scan_data):
 
     turning = np.where(turningpoints(jpeg_heights))[0] + 1
 
-    return jpeg_heights[turning[0] : turning[1]], jpeg_sizes_MB[turning[0] : turning[1]]
+    return jpeg_heights[turning[0] : turning[1]], jpeg_sizes_mb[turning[0] : turning[1]]
 
 
 def limit_focus_change(prev_pos, prev_z, new_pos, new_z, limit):
@@ -115,10 +115,12 @@ def steps_from_centre(current_loc, starting_loc, dx, dy):
     return np.max(np.abs(np.divide(np.subtract(current_loc, starting_loc), step_size)))
 
 
-def distance_to_site(current, next):
+def distance_to_site(current_pos, next_pos):
-    next = np.array(next, dtype="float64")
+    next_pos = np.array(next_pos, dtype="float64")
-    current = np.array(current, dtype="float64")
+    current_pos = np.array(current_pos, dtype="float64")
-    return np.sqrt((next[1] - current[1]) ** 2 + (next[0] - current[0]) ** 2)
+    return np.sqrt(
+        (next_pos[1] - current_pos[1]) ** 2 + (next_pos[0] - current_pos[0]) ** 2
+    )
 
 
 class NotEnoughFreeSpaceError(IOError):
@@ -439,13 +441,21 @@ class SmartScanThing(Thing):
             # TODO: Consider using CSM calibration size instead
             # TODO: generalise to have 2D displacements for x and y (as the
             # camera and stage may not be aligned).
-            CSM = self._csm.image_to_stage_displacement_matrix
+            csm_disp_matrix = self._csm.image_to_stage_displacement_matrix
 
             dx = int(
-                np.abs(np.dot(np.array([0, arr.shape[1] * (1 - overlap)]), CSM)[0])
+                np.abs(
+                    np.dot(
+                        np.array([0, arr.shape[1] * (1 - overlap)]), csm_disp_matrix
+                    )[0]
+                )
             )
             dy = int(
-                np.abs(np.dot(np.array([arr.shape[0] * (1 - overlap), 0]), CSM)[1])
+                np.abs(
+                    np.dot(
+                        np.array([arr.shape[0] * (1 - overlap), 0]), csm_disp_matrix
+                    )[1]
+                )
             )
 
             self._scan_logger.info(
@@ -1144,7 +1154,7 @@ class SmartScanThing(Thing):
         # as some of them should only be added at the end (as we can't overwrite)
         # them once they change
         if not os.path.isfile(zip_fname):
-            with zipfile.ZipFile(zip_fname, mode="w") as zip:
+            with zipfile.ZipFile(zip_fname, mode="w") as scan_zip:
                 pass
 
         # get a list of files in the existing zip
@@ -1167,7 +1177,7 @@ class SmartScanThing(Thing):
         ]
         tiff_name = ""
 
-        with zipfile.ZipFile(zip_fname, mode="a") as zip:
+        with zipfile.ZipFile(zip_fname, mode="a") as scan_zip:
             for file in files:
                 if "stitched.jp" in file:
                     stitch_name = os.path.split(file)[1]
@@ -1181,32 +1191,30 @@ class SmartScanThing(Thing):
                     pass
                 else:
                     logger.info(f"appending {file} to zip")
-                    zip.write(os.path.join(scan_folder, file), arcname=file)
+                    scan_zip.write(os.path.join(scan_folder, file), arcname=file)
 
         # Promote key files to the top level of the zip only at the end of the scan (when downloading)
         # and finally zip some of the final files
         # TODO: if you download multiple times, you get duplicate files - is this a problem?
         if download_zip:
-            with zipfile.ZipFile(zip_fname, mode="a") as zip:
+            with zipfile.ZipFile(zip_fname, mode="a") as scan_zip:
                 for fname in ["stitched_from_stage.jpg", stitch_name, tiff_name]:
                     fpath = os.path.join(images_folder, fname)
                     if os.path.exists(fpath):
                         logger.info(f"copying {fpath} to upper level")
-                        zip.write(fpath, arcname=fname)
+                        scan_zip.write(fpath, arcname=fname)
                 for file in files:
                     if any(banned_name in file for banned_name in files_to_delay):
                         logger.info(f"we are finally adding {file} into zip")
-                        zip.write(os.path.join(scan_folder, file), arcname=file)
+                        scan_zip.write(os.path.join(scan_folder, file), arcname=file)
             logger.info("about to download zip")
             return ZipBlob.from_file(zip_fname)
 
     @thing_action
     def get_files_in_zip(self, zip_path):
         """List the relative paths of all files and folders in the zip folder specified"""
-        zip = zipfile.ZipFile(zip_path)
+        scan_zip = zipfile.ZipFile(zip_path)
-        zip = [os.path.normpath(i) for i in zip.namelist()]
+        return [os.path.normpath(i) for i in scan_zip.namelist()]
-
-        return zip
 
 
 class CaptureError(RuntimeError):