diff --git a/hardware-specific-tests/picamera2/test_acquisition.py b/hardware-specific-tests/picamera2/test_acquisition.py index 3d8ef5c0..b8226cad 100644 --- a/hardware-specific-tests/picamera2/test_acquisition.py +++ b/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(): - """ - A pytest fixture that initialises a test client for the StreamingPiCamera2 Thing. - This fixture sets up a ThingServer, registers a StreamingPiCamera2 instance at the - "/camera/" endpoint, and provides a ThingClient for interacting with it during tests. + """Initialise a test client for the StreamingPiCamera2 Thing. + + This fixture: + + * 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() diff --git a/pyproject.toml b/pyproject.toml index 89a9156c..3ab95dc3 100644 --- a/pyproject.toml +++ b/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"] \ No newline at end of file diff --git a/scripts/zenodo/upload_to_zenodo.py b/scripts/zenodo/upload_to_zenodo.py index 2531f4cf..a2709e9e 100644 --- a/scripts/zenodo/upload_to_zenodo.py +++ b/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) diff --git a/scripts/zenodo/zenodo.py b/scripts/zenodo/zenodo.py index 2d64cf1a..55904848 100644 --- a/scripts/zenodo/zenodo.py +++ b/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}, diff --git a/src/openflexure_microscope_server/logging.py b/src/openflexure_microscope_server/logging.py index cb50251f..e3bc8cdd 100644 --- a/src/openflexure_microscope_server/logging.py +++ b/src/openflexure_microscope_server/logging.py @@ -39,9 +39,9 @@ def configure_logging(log_folder): def retrieve_log() -> PlainTextResponse: - """ - 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. + """Return logs since the server started, up to a maximum of 250 messages. + + 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: - """ - Returns the full log from the file for downloading. + """Return 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( diff --git a/src/openflexure_microscope_server/scan_directories.py b/src/openflexure_microscope_server/scan_directories.py index 8a9282eb..291cdcb4 100644 --- a/src/openflexure_microscope_server/scan_directories.py +++ b/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: diff --git a/src/openflexure_microscope_server/scan_planners.py b/src/openflexure_microscope_server/scan_planners.py index a6f5951f..f8f2a799 100644 --- a/src/openflexure_microscope_server/scan_planners.py +++ b/src/openflexure_microscope_server/scan_planners.py @@ -27,8 +27,11 @@ NEIGHBOUR_CUTOFF = 1.4 def enforce_xy_tuple(value: XYPos) -> XYPos: - """ - Used for enforcing that an input is a tuple and is the correct length + """Check input is a tuple and is of length 2. + + 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: - """ - Used for enforcing that an input is a tuple and is the correct length + """Check input is a tuple and is of length 3. + + 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: - """ - Called on initalisation. Sets the initial list of locations for this scan planner + """Set 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. This should be a dictionary + """Parse SmartSpiral Settings dictionary. - "dx" - the movement size in x - "dy" - the movement size in y - "max_dist" - The maximum distance to a location can be from the centre. + * ``dx`` - the movement size in x + * ``dy`` - the movement size in y + * ``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: - """ - Called on initalisation. Sets the initial list of locations for this scan planner + """Set 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: - """ - This adds the surrounding (4 point connectivity) positions - to the remaining locations if they are not too far away or - already planned or already visited + """Add the 4 surrounding positions to the list of remaining locations to visit. + + This adds the surrounding positions (with 4 point connectivity) to the + remaining locations list if they are not: + + * too far away + * already planned + * already visited """ new_positions = [ (xy_pos[0] - self._dx, xy_pos[1]), diff --git a/src/openflexure_microscope_server/server/legacy_api.py b/src/openflexure_microscope_server/server/legacy_api.py index 5fd9a3e9..1742cfe2 100644 --- a/src/openflexure_microscope_server/server/legacy_api.py +++ b/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() diff --git a/src/openflexure_microscope_server/things/camera/__init__.py b/src/openflexure_microscope_server/things/camera/__init__.py index 5e72d066..6710ee4d 100644 --- a/src/openflexure_microscope_server/things/camera/__init__.py +++ b/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 - logger warning (via InvocationLogger) is raised if metadata is failed to be added - IOError is raised if the file cannot be saved + """Save the captured image and metadata to disk. + + 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: diff --git a/src/openflexure_microscope_server/things/smart_scan.py b/src/openflexure_microscope_server/things/smart_scan.py index bd485e09..2c45d20d 100644 --- a/src/openflexure_microscope_server/things/smart_scan.py +++ b/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 - 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. + """Move the stage to the next poistion. - 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): - """ - This sets the self._scan_data dictionary. This needs to become a - dataclass. + """Set date for this scan to the ``self._scan_data`` variable. + + 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): - """ - The loop to run through during a scan, until no more scan x,y positions + """Run the main loop of the scan. + + 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: - """ - A wrapper around scan manager's delete_scan that logs to the invocation logger + """Delete a scan. + + 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 - the log - """ + """Log everything in the buffer at INFO level.""" while line := buffer.readline(): logger.info(line)