Doc strings to imperative mood

hardware-specific-tests/picamera2/test_acquisition.py

@@ -11,10 +11,13 @@ from openflexure_microscope_server.things.camera.picamera import StreamingPiCame
 
 @fixture(scope="module")
 def client():
-    """
+    """Initialise a test client for the StreamingPiCamera2 Thing.
-    A pytest fixture that initialises a test client for the StreamingPiCamera2 Thing.
+
-    This fixture sets up a ThingServer, registers a StreamingPiCamera2 instance at the
+    This fixture:
-    "/camera/" endpoint, and provides a ThingClient for interacting with it during tests.
+
+    * Sets up a ThingServer,
+    * Registers a StreamingPiCamera2 instance at the "/camera/" endpoint
+    * Provides a ThingClient for interacting with it during tests.
     """
     server = ThingServer()
     server.add_thing(StreamingPiCamera2(), "/camera/")
@@ -24,9 +27,7 @@ def client():
 
 
 def test_calibration(client):
-    """
+    """Check that full auto calibrate completes without an exception."""
-    Check that full auto calibrate completes without an exception
-    """
     client.full_auto_calibrate()
 
 

pyproject.toml

@@ -134,7 +134,6 @@ ignore = [
     "D400",
     "D200",
     "D404",
-    "D401",
     # These should be turned on before this MR is complete
     "D100",
     "D102",
@@ -146,4 +145,6 @@ ignore = [
 ]
 
 [tool.ruff.lint.pydocstyle]
+# This lets the D401 checker understand that decorated thing properties and thing
+# settings act like properties so should be documented as such.
 property-decorators = ["labthings_fastapi.thing_property", "labthings_fastapi.thing_setting"]

scripts/zenodo/upload_to_zenodo.py

@@ -25,7 +25,7 @@ def parse_arguments() -> Namespace:
 
 
 def script_directory(path):
-    """Resolves path to directory of the current script"""
+    """Resolve path to directory of the current script"""
     return os.path.join(os.path.dirname(os.path.realpath(__file__)), path)
 
 

scripts/zenodo/zenodo.py

@@ -22,7 +22,7 @@ class Zenodo:
             self.zenodo_url = "https://zenodo.org/api/deposit/depositions"
 
     def create_new_deposit(self):
-        """Creates a new (unpublished) Zenodo deposit and return its deposition ID."""
+        """Create a new (unpublished) Zenodo deposit and return its deposition ID."""
         headers = {"Content-Type": "application/json"}
         r = requests.post(
             self.zenodo_url,
@@ -35,7 +35,7 @@ class Zenodo:
         return r.json()
 
     def set_metadata(self, deposition_id, metadata):
-        """Sets the given metadata for the specified deposit."""
+        """Set the given metadata for the specified deposit."""
         headers = {"Content-Type": "application/json"}
         r = requests.put(
             self.zenodo_url + "/{}".format(deposition_id),
@@ -47,7 +47,7 @@ class Zenodo:
         print(r.json())
 
     def upload_file(self, deposition_id, file_path):
-        """Uploads a new file for the given deposit."""
+        """Upload a new file for the given deposit."""
         file_name = os.path.basename(file_path)
         data = {"filename": file_name}
         files = {"file": open(file_path, "rb")}
@@ -61,7 +61,7 @@ class Zenodo:
         print(r.json())
 
     def publish_deposit(self, deposition_id):
-        """Publishes the given deposit. BEWARE: It is now visible to all!!!"""
+        """Publish the given deposit. BEWARE: It is now visible to all!!!"""
         r = requests.post(
             self.zenodo_url + "/{}/actions/publish".format(deposition_id),
             params={"access_token": self._api_token},
@@ -70,7 +70,7 @@ class Zenodo:
         print(r.json())
 
     def create_new_version(self, deposition_id):
-        """Creates a new version of an already published deposit."""
+        """Create a new version of an already published deposit."""
         r = requests.post(
             self.zenodo_url + "/{}/actions/newversion".format(deposition_id),
             params={"access_token": self._api_token},
@@ -80,7 +80,7 @@ class Zenodo:
         return os.path.basename(r.json()["links"]["latest_draft"])
 
     def remove_all_files(self, deposition_id):
-        """Removes all uploaded files of a unpublished deposit."""
+        """Remove all uploaded files of a unpublished deposit."""
         r = requests.get(
             self.zenodo_url + "/{}/files".format(deposition_id),
             params={"access_token": self._api_token},

src/openflexure_microscope_server/logging.py

@@ -39,9 +39,9 @@ def configure_logging(log_folder):
 
 
 def retrieve_log() -> PlainTextResponse:
-    """
+    """Return logs since the server started, up to a maximum of 250 messages.
-    Returns logs since we started running the server, up to a maximum of
+
-    250 messages. This log is the one shown in the UI and on the logging page.
+    This log is the one shown in the UI and on the logging page.
 
     It does not include any of the ``uvicorn.access`` logs as these are emmitted
     every time any API route is called.
@@ -53,11 +53,10 @@ def retrieve_log() -> PlainTextResponse:
 
 
 def retrieve_log_from_file() -> PlainTextResponse:
-    """
+    """Return the full log from the file for downloading.
-    Returns the full log from the file for downloading.
 
     Note this is read and then sent as otherwise it causes a RuntimeError if it
-    is written to while sending through FileResponse
+    is written to while sending through FileResponse.
     """
     if OFM_LOG_FILE is None:
         raise RuntimeError(

src/openflexure_microscope_server/scan_directories.py

@@ -53,7 +53,7 @@ class ScanDirectoryManager:
         return self._base_scan_dir
 
     def exists(self, scan_name: str) -> bool:
-        """True if scan of this name exists on disk"""
+        """Return True if scan of this name exists on disk"""
         return os.path.isdir(self.path_for(scan_name))
 
     def path_for(self, scan_name: str) -> str:

src/openflexure_microscope_server/scan_planners.py

@@ -27,8 +27,11 @@ NEIGHBOUR_CUTOFF = 1.4
 
 
 def enforce_xy_tuple(value: XYPos) -> XYPos:
-    """
+    """Check input is a tuple and is of length 2.
-    Used for enforcing that an input is a tuple and is the correct length
+
+    If possible it will coerce the value to a tuple.
+
+    :raises ValueError: if the input cannot be coerced to a tuple of length 2.
     """
     if not isinstance(value, (list, tuple)):
         raise ValueError("2 value tuple expected")
@@ -40,8 +43,11 @@ def enforce_xy_tuple(value: XYPos) -> XYPos:
 
 
 def enforce_xyz_tuple(value: XYZPos) -> XYZPos:
-    """
+    """Check input is a tuple and is of length 3.
-    Used for enforcing that an input is a tuple and is the correct length
+
+    If possible it will coerce the value to a tuple.
+
+    :raises ValueError: if the input cannot be coerced to a tuple of length 3.
     """
     if not isinstance(value, (list, tuple)):
         raise ValueError("3 value tuple expected")
@@ -75,9 +81,7 @@ class ScanPlanner:
     """
 
     def __init__(self, intial_position: XYPos, planner_settings: Optional[dict] = None):
-        """
+        """Set up lists for the path planning, and scan history."""
-        Set up lists for the path planning, and scan history.
-        """
         self._initial_position = enforce_xy_tuple(intial_position)
         self._parse(planner_settings)
 
@@ -103,16 +107,12 @@ class ScanPlanner:
 
     @property
     def scan_complete(self) -> bool:
-        """
+        """Return True if there are no locations left to scan."""
-        Return True if there are no locations left to scan.
-        """
         return not self._remaining_locations
 
     @property
     def remaining_locations(self) -> XYPosList:
-        """
+        """Property to access a copy of the remaining_locations."""
-        Property to access a copy of the remaining_locations
-        """
         return copy(self._remaining_locations)
 
     @property
@@ -124,27 +124,22 @@ class ScanPlanner:
 
     @property
     def focused_locations(self) -> XYZPosList:
-        """
+        """Property to access a copy of the focused_locations."""
-        Property to access a copy of the focused_locations
-        """
         return copy(self._focused_locations)
 
     @property
     def path_history(self) -> XYPosList:
-        """
+        """Property to access a copy of the path_history."""
-        Property to access a copy of the path_history
-        """
         return copy(self._path_history)
 
     def _parse(self, planner_settings: Optional[dict] = None) -> None:
-        """
+        """Parse any settings sent to this planner and store them if needed."""
-        Parse any settings sent to this planner and store them if needed.
-        """
         raise NotImplementedError("Did you call the ScanPlanner base class?")
 
     def _intial_location_list(self) -> XYPosList:
-        """
+        """Set the initial list of locations for this scan planner.
-        Called on initalisation. Sets the initial list of locations for this scan planner
+
+        This is called on initalisation.
 
         For a simple grid scan/snake scan this would be all locations to move to.
 
@@ -267,12 +262,11 @@ class SmartSpiral(ScanPlanner):
     _dy: int = 0
 
     def _parse(self, planner_settings: Optional[dict] = None) -> None:
-        """
+        """Parse SmartSpiral Settings dictionary.
-        Parse SmartSpiral Settings. This should be a dictionary
 
-        "dx" - the movement size in x
+        * ``dx`` - the movement size in x
-        "dy" - the movement size in y
+        * ``dy`` - the movement size in y
-        "max_dist" - The maximum distance to a location can be from the centre.
+        * ``max_dist`` - The maximum distance to a location can be from the centre.
         """
         expected_keys = ["max_dist", "dx", "dy"]
         invalid_msg = "SmartSpiral requires a planner_settings dictionary with keys: "
@@ -286,8 +280,9 @@ class SmartSpiral(ScanPlanner):
         self._max_dist = int(planner_settings["max_dist"])
 
     def _intial_location_list(self) -> XYPosList:
-        """
+        """Set the initial list of locations for this scan planner.
-        Called on initalisation. Sets the initial list of locations for this scan planner
+
+        This is salled on initialisation.
 
         For smart spiral this is just the first point
         """
@@ -314,10 +309,14 @@ class SmartSpiral(ScanPlanner):
         self._re_sort_remaining_locations(xy_pos)
 
     def _add_surrounding_positions(self, xy_pos: XYPos) -> None:
-        """
+        """Add the 4 surrounding positions to the list of remaining locations to visit.
-        This adds the surrounding (4 point connectivity) positions
+
-        to the remaining locations if they are not too far away or
+        This adds the surrounding positions (with 4 point connectivity) to the
-        already planned or already visited
+        remaining locations list if they are not:
+
+        * too far away
+        * already planned
+        * already visited
         """
         new_positions = [
             (xy_pos[0] - self._dx, xy_pos[1]),

src/openflexure_microscope_server/server/legacy_api.py

@@ -12,7 +12,10 @@ def add_v2_endpoints(thing_server: lt.ThingServer):
     # This is necessary until Connect is rebuilt.
     @app.get("/routes")
     def routes_stub() -> dict[str, dict]:
-        """A stub list of routes, used by OF Connect to identify the microscope"""
+        """Return a stub list of routes.
+
+        This is used by OF Connect to identify the microscope.
+        """
         fake_routes = [
             "/api/v2/",
             "/api/v2/streams/snapshot",
@@ -26,11 +29,11 @@ def add_v2_endpoints(thing_server: lt.ThingServer):
     @app.get("/api/v2/streams/snapshot")
     @app.head("/api/v2/streams/snapshot")
     async def thumbnail() -> JPEGResponse:
-        """A low-resolution snapshot, for compatibility with OF connect"""
+        """Return a low-resolution snapshot, for compatibility with OF connect."""
         blob = await thing_server.things["/camera/"].lores_mjpeg_stream.grab_frame()
         return JPEGResponse(blob)
 
     @app.get("/api/v2/instrument/settings/name")
     def get_hostname() -> str:
-        """Get the hostname of the device, for compatibility with OF connect"""
+        """Get the hostname of the device, for compatibility with OF connect."""
         return gethostname()

src/openflexure_microscope_server/things/camera/__init__.py

@@ -377,9 +377,12 @@ class BaseCamera(lt.Thing):
         logger: lt.deps.InvocationLogger,
         save_resolution: Optional[Tuple[int, int]] = None,
     ) -> None:
-        """Saving the captured image and metadata to disk
+        """Save the captured image and metadata to disk.
-        logger warning (via InvocationLogger) is raised if metadata is failed to be added
+
-        IOError is raised if the file cannot be saved
+        A warning (via InvocationLogger) is raised if metadata is failed to be added
+
+        :raises IOError: if the file cannot be saved
+
         nothing is returned on success
         """
         if save_resolution is not None and image.size != save_resolution:

src/openflexure_microscope_server/things/smart_scan.py

@@ -46,7 +46,8 @@ class ScanNotRunningError(RuntimeError):
 
 
 def _scan_running(method):
-    """
+    """Decorate a method so that it will error if a scan is not running.
+
     This decorator is used by all methods in SmartScanThing that are using
     the variables set for the scan. It will throw a runtime error if
     self._scan_logger is not set, as all scan variables are set at
@@ -206,12 +207,13 @@ class SmartScanThing(lt.Thing):
     def _move_to_next_point(
         self, next_point: tuple[int, int], z_estimate: Optional[int] = None
     ) -> tuple[int, int, int]:
-        """Moves the stage to the next poistion. If no z_estimate is given then
+        """Move the stage to the next poistion.
-        the current stage position is used. Must move to the estimated focused position
-        (although moving below would be marginally faster) because background detect is
-        most reliable at the focused position.
 
-        Returns the (x,y,z) with the chosen z_estimate
+        If no z_estimate is given then the current stage position is used. Must move
+        to the estimated focused position (although moving below would be marginally
+        faster) because background detect is most reliable at the focused position.
+
+        :returns: the (x,y,z) with the chosen z_estimate
         """
         if z_estimate is None:
             z_estimate = self._stage.position["z"]
@@ -266,9 +268,9 @@ class SmartScanThing(lt.Thing):
 
     @_scan_running
     def _set_scan_data(self):
-        """
+        """Set date for this scan to the ``self._scan_data`` variable.
-        This sets the self._scan_data dictionary. This needs to become a
+
-        dataclass.
+        This needs to become a dataclass at some point.
         """
         overlap = self.overlap
         dx, dy = self._calc_displacement_from_test_image(overlap)
@@ -454,8 +456,9 @@ class SmartScanThing(lt.Thing):
 
     @_scan_running
     def _main_scan_loop(self):
-        """
+        """Run the main loop of the scan.
-        The loop to run through during a scan, until no more scan x,y positions
+
+        This loop runs during a scan, until no more scan x,y positions
         are remaining.
         """
         # The initial plan for the scan should be a single x,y position. All future
@@ -709,8 +712,10 @@ class SmartScanThing(lt.Thing):
                 self._delete_scan(scan_info.name, logger)
 
     def _delete_scan(self, scan_name, logger: lt.deps.InvocationLogger) -> bool:
-        """
+        """Delete a scan.
-        A wrapper around scan manager's delete_scan that logs to the invocation logger
+
+        This is a wrapper around scan manager's delete_scan that logs to the
+        invocation logger id there is a problem.
         """
         try:
             self._scan_dir_manager.delete_scan(scan_name)
@@ -832,9 +837,7 @@ class SmartScanThing(lt.Thing):
         logger.info(f"Running command in subprocess: `{' '.join(cmd)}`")
 
         def log_buffer(buffer):
-            """A short internal function to read everything in the buffer to
+            """Log everything in the buffer at INFO level."""
-            the log
-            """
             while line := buffer.readline():
                 logger.info(line)