From 260d524ab8992b27f7813b12858ef84397404ad7 Mon Sep 17 00:00:00 2001 From: Michael Lynch Date: Mon, 25 Sep 2023 12:21:54 -0400 Subject: [PATCH] Use ruff to enforce Python docstring conventions (#1633) We've been enforcing Python docstring conventions purely through code review, but ruff can enforce some of the conventions automatically. I've adjusted some of our docstrings to match ruff's expectations. Review on CodeApprove --- .ruff.toml | 18 +++++++++++++++++- app/api.py | 3 +-- app/db/store.py | 3 ++- app/db_connection.py | 2 +- app/execute.py | 6 ++++-- app/request_parsers/hostname.py | 7 ++++--- app/request_parsers/paste.py | 3 +-- app/update/settings.py | 2 +- app/version.py | 7 ++++--- 9 files changed, 35 insertions(+), 16 deletions(-) diff --git a/.ruff.toml b/.ruff.toml index fd7dda4a7..c21e4e717 100644 --- a/.ruff.toml +++ b/.ruff.toml @@ -1,11 +1,24 @@ src = ["app"] select = [ + "D", # Enable pydocstyle rules. "F", # Enable pyflakes rules. "I", # Enable isort rules. "Q", # Enable flake8-quotes rules. ] -ignore = [] +ignore = [ + # Disable rules that require everything to have a docstring. + "D100", # Missing docstring in public module + "D101", # Missing docstring in public class + "D102", # Missing docstring in public method + "D103", # Missing docstring in public function + "D104", # Missing docstring in public package + "D105", # Missing docstring in magic method + "D106", # Missing docstring in public nested class + "D107", # Missing docstring in __init__ + # We sometimes do our own indenting that contradicts pydocstyle expectations. + "D214", # Section is over-indented +] exclude = [ ".git", @@ -28,3 +41,6 @@ multiline-quotes = "double" [isort] force-single-line = true force-to-top = ["log"] + +[pydocstyle] +convention = "google" diff --git a/app/api.py b/app/api.py index e0a35bb8b..a07bd21c2 100644 --- a/app/api.py +++ b/app/api.py @@ -78,7 +78,6 @@ def update_get(): "updateError": null } """ - status, error = update.status.get() return json_response.success({'status': str(status), 'updateError': error}) @@ -179,7 +178,7 @@ def hostname_get(): @api_blueprint.route('/hostname', methods=['PUT']) def hostname_set(): - """Changes the machine’s hostname + """Changes the machine’s hostname. Expects a JSON data structure in the request body that contains the new hostname as string. Example: diff --git a/app/db/store.py b/app/db/store.py index b5e835e74..4a95f1383 100644 --- a/app/db/store.py +++ b/app/db/store.py @@ -1,4 +1,5 @@ -""" +"""Manages database access and schema migrations. + For evolving our database schema over time, we follow the idea of “evolutionary database design” (https://www.martinfowler.com/articles/evodb.html), which is also used by DB migration tools like Liquibase or the one in Django. diff --git a/app/db_connection.py b/app/db_connection.py index 4293bf5b3..f67da7e9f 100644 --- a/app/db_connection.py +++ b/app/db_connection.py @@ -7,7 +7,7 @@ def get(): - """Returns a database connection. (sqlite3.dbapi2.connection)""" + """Returns a database connection (sqlite3.dbapi2.connection).""" # Keep in mind that Flask only caches the connection object on a per-request # basis, and not throughout the entire runtime of the server. connection = _get_flask_db() diff --git a/app/execute.py b/app/execute.py index dfc00b8c4..9b08733e5 100644 --- a/app/execute.py +++ b/app/execute.py @@ -14,8 +14,10 @@ def was_successful(self) -> bool: class ProcessWithResult(multiprocessing.Process): - """A multiprocessing.Process object that keeps track of the child process' - result (i.e., the return value and exception raised). + """Extension of Process that Keeps track of the child process' result. + + This class is useful for executing a function in a child process and storing + the result (i.e., the return value and exception raised). Inspired by: https://stackoverflow.com/a/33599967/3769045 diff --git a/app/request_parsers/hostname.py b/app/request_parsers/hostname.py index 50827ab9e..d88f3a563 100644 --- a/app/request_parsers/hostname.py +++ b/app/request_parsers/hostname.py @@ -7,9 +7,10 @@ def parse_hostname(request): - """ - Parses the hostname property from the request. - Checks whether the hostname is valid according to the rules in + """Parses the hostname property from the request. + + Parses the hostname from a JSON request and checks whether the hostname is + valid according to the rules in https://man7.org/linux/man-pages/man7/hostname.7.html Args: diff --git a/app/request_parsers/paste.py b/app/request_parsers/paste.py index 11df7da58..281748a49 100644 --- a/app/request_parsers/paste.py +++ b/app/request_parsers/paste.py @@ -4,8 +4,7 @@ def parse_keystrokes(request): - """ - Parses HID keystrokes from the request. + """Parses HID keystrokes from the request. Args: request: Flask request with the following fields in the JSON body: diff --git a/app/update/settings.py b/app/update/settings.py index af62b213a..5d1833a74 100644 --- a/app/update/settings.py +++ b/app/update/settings.py @@ -118,7 +118,7 @@ def ustreamer_h264_bitrate(self, value): def load(): - """Retrieves the current TinyPilot update settings + """Retrieves the current TinyPilot update settings. Parses the contents of settings file and generates a settings object that represents the values in the settings file. diff --git a/app/version.py b/app/version.py index 9f80e80f2..599fddf9c 100644 --- a/app/version.py +++ b/app/version.py @@ -22,7 +22,9 @@ class VersionRequestError(Error): @dataclasses.dataclass class UpdateInfo: - """This data structure reflects the response of Gatekeeper’s + """Metadata about a TinyPilot update package. + + This data structure reflects the response of Gatekeeper’s `/available-update` routes. """ version: str @@ -63,8 +65,7 @@ def local_version(): def latest_version(): - """Requests info about the latest release from the TinyPilot Gatekeeper REST - API. + """Requests the latest release info from the TinyPilot Gatekeeper REST API. Returns: An UpdateInfo object, containing the information about the release.