chore: consolidate linters and formatter into Ruff for improved perfo…

…rmance and simplicity (#216)

.pre-commit-config.yaml

@@ -42,25 +42,13 @@ repos:
         args: [--no-update]
         files: ^(.*/)?(poetry\.lock|pyproject\.toml)$
 
-  - repo: https://github.com/pycqa/pydocstyle
-    rev: 6.3.0
-    hooks:
-      - id: pydocstyle
-        exclude: "generated/|tests.py"
-        additional_dependencies: [tomli]
-
-  - repo: https://github.com/pycqa/isort
-    rev: 5.13.2
-    hooks:
-      - id: isort
-        name: isort (python)
-        exclude: *generated
 
-  - repo: https://github.com/psf/black
-    rev: 24.10.0
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.8.1
     hooks:
-      - id: black
-        args: ["--config=pyproject.toml"]
+      - id: ruff
+        args: [ --fix ]
+      - id: ruff-format
         exclude: *generated
 
   - repo: https://github.com/rtts/djhtml

canvas_cli/apps/auth/tests.py

@@ -8,6 +8,8 @@
 
 @pytest.fixture
 def valid_token_response() -> Any:
+    """Returns a valid token response."""
+
     class TokenResponse:
         status_code = 200
 
@@ -19,6 +21,8 @@ def json(self) -> dict:
 
 @pytest.fixture
 def error_token_response() -> Any:
+    """Returns an error token response."""
+
     class TokenResponse:
         status_code = 500
 
@@ -27,6 +31,8 @@ class TokenResponse:
 
 @pytest.fixture
 def expired_token_response() -> Any:
+    """Returns an expired token response."""
+
     class TokenResponse:
         status_code = 200
 
@@ -45,6 +51,7 @@ def test_get_or_request_api_token_uses_stored_token(
     mock_get_password: MagicMock,
     valid_token_response: Any,
 ) -> None:
+    """Test that get_or_request_api_token uses a stored token if it is valid."""
     mock_is_token_valid.return_value = True
     mock_get_password.return_value = "a-stored-valid-token"
     mock_post.return_value = valid_token_response
@@ -66,6 +73,7 @@ def test_get_or_request_api_token_requests_token_if_none_stored(
     mock_set_password: MagicMock,
     valid_token_response: Any,
 ) -> None:
+    """Test that get_or_request_api_token requests a new token if none is stored."""
     mock_client_credentials.return_value = "client_id=id&client_secret=secret"
     mock_get_password.return_value = None
     mock_post.return_value = valid_token_response
@@ -95,6 +103,7 @@ def test_get_or_request_api_token_raises_exception_if_error_token_response(
     mock_get_password: MagicMock,
     error_token_response: Any,
 ) -> None:
+    """Test that get_or_request_api_token raises an exception if an error token response is received."""
     mock_client_credentials.return_value = "client_id=id&client_secret=secret"
     mock_get_password.return_value = None
     mock_post.return_value = error_token_response
@@ -123,6 +132,7 @@ def test_get_or_request_api_token_raises_exception_if_expired_token(
     mock_get_password: MagicMock,
     expired_token_response: Any,
 ) -> None:
+    """Test that get_or_request_api_token raises an exception if an expired token is received."""
     mock_client_credentials.return_value = "client_id=id&client_secret=secret"
     mock_get_password.return_value = None
     mock_post.return_value = expired_token_response

canvas_cli/apps/emit/emit.py

@@ -7,7 +7,6 @@
 import typer
 
 from canvas_generated.messages.events_pb2 import Event as PluginRunnerEvent
-from canvas_generated.messages.events_pb2 import EventType as PluginRunnerEventType
 from canvas_generated.services.plugin_runner_pb2_grpc import PluginRunnerStub
 
 

canvas_cli/apps/logs/logs.py

@@ -1,5 +1,5 @@
 import json
-from typing import Optional, cast
+from typing import cast
 from urllib.parse import urlparse
 
 import typer
@@ -31,9 +31,9 @@ def _on_open(ws: websocket.WebSocket) -> None:
 
 
 def logs(
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host, help="Canvas instance to connect to", default=None
-    )
+    ),
 ) -> None:
     """Listens and prints log streams from the instance."""
     if not host:
@@ -62,4 +62,4 @@ def logs(
         ws.run_forever()
 
     except KeyboardInterrupt:
-        raise typer.Exit(0)
+        raise typer.Exit(0) from None

canvas_cli/apps/plugin/plugin.py

@@ -2,8 +2,9 @@
 import json
 import tarfile
 import tempfile
+from collections.abc import Iterable
 from pathlib import Path
-from typing import Any, List, Optional, cast
+from typing import Any, cast
 from urllib.parse import urljoin
 
 import requests
@@ -53,14 +54,15 @@ def _build_package(package: Path) -> Path:
 def _get_name_from_metadata(host: str, token: str, package: Path) -> str | None:
     """Extract metadata from a provided package and return the package name if it exists in the metadata."""
     try:
-        metadata_response = requests.post(
-            plugin_url(host, "extract-metadata"),
-            headers={"Authorization": f"Bearer {token}"},
-            files={"package": open(package, "rb")},
-        )
+        with open(package) as package_file:
+            metadata_response = requests.post(
+                plugin_url(host, "extract-metadata"),
+                headers={"Authorization": f"Bearer {token}"},
+                files={"package": package_file},
+            )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if metadata_response.status_code != requests.codes.ok:
         print(f"Status code {metadata_response.status_code}: {metadata_response.text}")
@@ -116,8 +118,8 @@ def _get_meta_properties(protocol_path: Path, classname: str) -> dict[str, str]:
 
 
 def _get_protocols_with_new_cqm_properties(
-    protocol_classes: List[dict[str, Any]], plugin: Path
-) -> List[dict[str, Any]] | None:
+    protocol_classes: Iterable[dict[str, Any]], plugin: Path
+) -> Iterable[dict[str, Any]] | None:
     """Extract the meta properties of any ClinicalQualityMeasure Protocols included in the plugin if they have changed."""
     has_updates = False
     protocol_props = []
@@ -146,14 +148,14 @@ def init() -> None:
     try:
         project_dir = cookiecutter(str(template))
     except OutputDirExistsException:
-        raise typer.BadParameter(f"The supplied directory already exists")
+        raise typer.BadParameter("The supplied directory already exists") from None
 
     print(f"Project created in {project_dir}")
 
 
 def install(
     plugin_name: Path = typer.Argument(..., help="Path to plugin to install"),
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host,
         help="Canvas instance to connect to",
         default=None,
@@ -181,15 +183,16 @@ def install(
     print(f"Posting {built_package_path.absolute()} to {url}")
 
     try:
-        r = requests.post(
-            url,
-            data={"is_enabled": True},
-            files={"package": open(built_package_path, "rb")},
-            headers={"Authorization": f"Bearer {token}"},
-        )
+        with open(built_package_path, "rb") as package:
+            r = requests.post(
+                url,
+                data={"is_enabled": True},
+                files={"package": package},
+                headers={"Authorization": f"Bearer {token}"},
+            )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if r.status_code == requests.codes.created:
         print(f"Plugin {plugin_name} successfully installed!")
@@ -207,7 +210,7 @@ def install(
 
 def uninstall(
     name: str = typer.Argument(..., help="Plugin name to uninstall"),
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host,
         help="Canvas instance to connect to",
         default=None,
@@ -232,7 +235,7 @@ def uninstall(
         )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if r.status_code == requests.codes.no_content:
         print(f"Plugin {name} successfully uninstalled!")
@@ -243,7 +246,7 @@ def uninstall(
 
 def enable(
     name: str = typer.Argument(..., help="Plugin name to enable"),
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host,
         help="Canvas instance to connect to",
         default=None,
@@ -269,7 +272,7 @@ def enable(
         )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if r.ok:
         print(f"Plugin {name} successfully enabled!")
@@ -280,7 +283,7 @@ def enable(
 
 def disable(
     name: str = typer.Argument(..., help="Plugin name to disable"),
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host,
         help="Canvas instance to connect to",
         default=None,
@@ -306,7 +309,7 @@ def disable(
         )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if r.ok:
         print(f"Plugin {name} successfully disabled!")
@@ -316,11 +319,11 @@ def disable(
 
 
 def list(
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host,
         help="Canvas instance to connect to",
         default=None,
-    )
+    ),
 ) -> None:
     """List all plugins from a Canvas instance."""
     if not host:
@@ -337,7 +340,7 @@ def list(
         )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if r.status_code == requests.codes.ok:
         plugins = r.json().get("results", [])
@@ -382,7 +385,7 @@ def validate_manifest(
 
     except json.JSONDecodeError:
         print("There was a problem loading the manifest file, please ensure it's valid JSON")
-        raise typer.Abort()
+        raise typer.Abort() from None
 
     validate_manifest_file(manifest_json)
 
@@ -391,14 +394,14 @@ def validate_manifest(
 
 def update(
     name: str = typer.Argument(..., help="Plugin name to update"),
-    package: Optional[Path] = typer.Option(
+    package_path: Path | None = typer.Option(
         help="Path to a wheel or sdist file containing the python package to install",
         default=None,
     ),
-    is_enabled: Optional[bool] = typer.Option(
+    is_enabled: bool | None = typer.Option(
         None, "--enable/--disable", show_default=False, help="Enable/disable the plugin"
     ),
-    host: Optional[str] = typer.Option(
+    host: str | None = typer.Option(
         callback=get_default_host,
         help="Canvas instance to connect to",
         default=None,
@@ -408,27 +411,36 @@ def update(
     if not host:
         raise typer.BadParameter("Please specify a host or set a default via the `auth` command")
 
-    if package:
-        validate_package(package)
+    if package_path:
+        validate_package(package_path)
 
     token = get_or_request_api_token(host)
 
-    print(f"Updating plugin {name} from {host} with {is_enabled=}, {package=}")
-
-    binary_package = {"package": open(package, "rb")} if package else None
+    print(f"Updating plugin {name} from {host} with {is_enabled=}, {package_path=}")
 
     url = plugin_url(host, name)
 
     try:
-        r = requests.patch(
-            url,
-            data={"is_enabled": is_enabled} if is_enabled is not None else {},
-            files=binary_package,
-            headers={"Authorization": f"Bearer {token}"},
-        )
+        data = {"is_enabled": is_enabled} if is_enabled is not None else {}
+        headers = {"Authorization": f"Bearer {token}"}
+
+        if package_path:
+            with open(package_path, "rb") as package:
+                r = requests.patch(
+                    url,
+                    data=data,
+                    headers=headers,
+                    files={"package": package},
+                )
+        else:
+            r = requests.patch(
+                url,
+                data=data,
+                headers=headers,
+            )
     except requests.exceptions.RequestException:
         print(f"Failed to connect to {host}")
-        raise typer.Exit(1)
+        raise typer.Exit(1) from None
 
     if r.status_code == requests.codes.ok:
         print("Plugin successfully updated!")

canvas_cli/apps/plugin/tests.py

@@ -1,7 +1,8 @@
 import shutil
+from collections.abc import Generator
 from datetime import datetime
 from pathlib import Path
-from typing import Any, Generator
+from typing import Any
 
 import pytest
 import typer
@@ -39,12 +40,13 @@ def test_validate_package_valid_file(tmp_path: Path) -> None:
 
 @pytest.fixture(scope="session")
 def init_plugin_name() -> str:
-    """The plugin name to be used for the canvas cli init test"""
+    """The plugin name to be used for the canvas cli init test."""
     return f"testing_init-{datetime.now().timestamp()}".replace(".", "")
 
 
 @pytest.fixture(autouse=True, scope="session")
 def clean_up_plugin(init_plugin_name: str) -> Generator[Any, Any, Any]:
+    """Cleans up the plugin directory after the test."""
     yield
     if Path(f"./{init_plugin_name}").exists():
         shutil.rmtree(Path(f"./{init_plugin_name}"))

canvas_cli/conftest.py

@@ -19,6 +19,7 @@ def app_dir() -> str:
 @pytest.fixture(autouse=True)
 def reset_context_variables() -> None:
     """Reset the context properties to their default value.
+
     This is needed because we cannot build a `reset` method in the CLIContext class,
     because `load_from_file` loads properties dynamically.
     Also since this is a CLI, it's not expected to keep the global context in memory for more than a run,