Merge pull request #241 from lsst-sqre/u/rra/ruff-format

Update Ruff configuration

.pre-commit-config.yaml

@@ -12,15 +12,11 @@ repos:
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]
-
-  - repo: https://github.com/ambv/black
-    rev: 24.2.0
-    hooks:
-      - id: black
+      - id: ruff-format
 
   - repo: https://github.com/asottile/blacken-docs
     rev: 1.16.0
     hooks:
       - id: blacken-docs
-        additional_dependencies: [black==23.1.0]
+        additional_dependencies: [black==24.2.0]
         args: [-l, '76', -t, py311]

docs/user-guide/arq.rst

@@ -71,9 +71,9 @@ If your app uses a configuration system like ``pydantic.BaseSettings``, this exa
             redis_settings = RedisSettings(
                 host=url_parts.hostname or "localhost",
                 port=url_parts.port or 6379,
-                database=int(url_parts.path.lstrip("/"))
-                if url_parts.path
-                else 0,
+                database=(
+                    int(url_parts.path.lstrip("/")) if url_parts.path else 0
+                ),
             )
             return redis_settings
 

docs/user-guide/fastapi-errors.rst

@@ -147,8 +147,7 @@ For example, for a single route:
        ...,
        responses={404: {"description": "Not found", "model": ErrorModel}},
    )
-   async def route(foo: str) -> None:
-       ...
+   async def route(foo: str) -> None: ...
 
 If all routes provided by a router have the same error handling behavior for a given response code, it saves some effort to instead do this when including the router, normally in ``main.py``:
 

pyproject.toml

@@ -211,10 +211,13 @@ ignore = [
     "FIX002",  # point of a TODO comment is that we're not ready to fix it
     "G004",    # forbidding logging f-strings is appealing, but not our style
     "RET505",  # disagree that omitting else always makes code more readable
+    "PLR0911", # often many returns is clearer and simpler style
     "PLR0913", # factory pattern uses constructors with many arguments
     "PLR2004", # too aggressive about magic values
+    "PLW0603", # yes global is discouraged but if needed, it's needed
     "S105",    # good idea but too many false positives on non-passwords
     "S106",    # good idea but too many false positives on non-passwords
+    "S107",    # good idea but too many false positives on non-passwords
     "S603",    # not going to manually mark every subprocess call as reviewed
     "S607",    # using PATH is not a security vulnerability
     "SIM102",  # sometimes the formatting of nested if statements is clearer
@@ -225,14 +228,31 @@ ignore = [
     "TD003",   # we don't require issues be created for TODOs
     "TID252",  # if we're going to use relative imports, use them always
     "TRY003",  # good general advice but lint is way too aggressive
+    "TRY301",  # sometimes raising exceptions inside try is the best flow
 
-    # Safir-specific rules.
-    "N818",    # Exception is correct in some cases, others are part of API
-    "PLW0603", # necessary trick for safir.logging
+    # The following settings should be disabled when using ruff format
+    # per https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
+    "W191",
+    "E111",
+    "E114",
+    "E117",
+    "D206",
+    "D300",
+    "Q000",
+    "Q001",
+    "Q002",
+    "Q003",
+    "COM812",
+    "COM819",
+    "ISC001",
+    "ISC002",
 ]
 select = ["ALL"]
 
 [tool.ruff.lint.per-file-ignores]
+"src/safir/**" = [
+    "N818",    # Exception is correct in some cases, others are part of API
+]
 "src/safir/testing/**" = [
     "S101",    # test support functions are allowed to use assert
 ]
@@ -243,6 +263,7 @@ select = ["ALL"]
     "PLR0915", # tests are allowed to be long, sometimes that's convenient
     "PT012",   # way too aggressive about limiting pytest.raises blocks
     "S101",    # tests should use assert
+    "S106",    # tests are allowed to hard-code dummy passwords
     "SLF001",  # tests are allowed to access private members
 ]
 

src/safir/asyncio.py

@@ -200,7 +200,7 @@ def qsize(self) -> int:
 
 
 def run_with_asyncio(
-    f: Callable[P, Coroutine[None, None, T]]
+    f: Callable[P, Coroutine[None, None, T]],
 ) -> Callable[P, T]:
     """Run the decorated function with `asyncio.run`.
 

src/safir/dependencies/gafaelfawr.py

@@ -15,7 +15,7 @@
 
 
 async def auth_dependency(
-    x_auth_request_user: Annotated[str, Header(include_in_schema=False)]
+    x_auth_request_user: Annotated[str, Header(include_in_schema=False)],
 ) -> str:
     """Retrieve authentication information from HTTP headers.
 
@@ -27,7 +27,7 @@ async def auth_dependency(
 
 
 async def auth_delegated_token_dependency(
-    x_auth_request_token: Annotated[str, Header(include_in_schema=False)]
+    x_auth_request_token: Annotated[str, Header(include_in_schema=False)],
 ) -> str:
     """Retrieve Gafaelfawr delegated token from HTTP headers.
 

src/safir/pydantic.py

@@ -158,7 +158,7 @@ class Model(BaseModel):
 
 
 def _copy_type(
-    parent: Callable[P, T]
+    parent: Callable[P, T],
 ) -> Callable[[Callable[..., T]], Callable[P, T]]:
     """Copy the type of a parent method.
 

src/safir/testing/kubernetes.py

@@ -169,7 +169,8 @@ class _KubernetesModel(Protocol):
     """
 
     def to_dict(
-        self, serialize: bool = False  # noqa: FBT001, FBT002
+        self,
+        serialize: bool = False,  # noqa: FBT001, FBT002
     ) -> dict[str, Any]: ...
 
 

tests/dependencies/gafaelfawr_test.py

@@ -26,7 +26,7 @@ async def test_auth_dependency() -> None:
 
     @app.get("/")
     async def handler(
-        user: Annotated[str, Depends(auth_dependency)]
+        user: Annotated[str, Depends(auth_dependency)],
     ) -> dict[str, str]:
         return {"user": user}
 

tests/logging_test.py

@@ -208,7 +208,7 @@ def test_dev_exception_logging(caplog: LogCaptureFixture) -> None:
     logger = structlog.get_logger("myapp")
 
     try:
-        raise ValueError("this is some exception")  # noqa: TRY301
+        raise ValueError("this is some exception")
     except Exception:
         logger.exception("exception happened", foo="bar")
 
@@ -226,7 +226,7 @@ def test_production_exception_logging(caplog: LogCaptureFixture) -> None:
     logger = structlog.get_logger("myapp")
 
     try:
-        raise ValueError("this is some exception")  # noqa: TRY301
+        raise ValueError("this is some exception")
     except Exception:
         logger.exception("exception happened", foo="bar")
 

tests/middleware/ivoa_test.py

@@ -32,7 +32,7 @@ async def simple_handler() -> dict[str, str]:
 
     @app.get("/list")
     async def list_handler(
-        param: Annotated[list[str], Query()]
+        param: Annotated[list[str], Query()],
     ) -> dict[str, list[str]]:
         return {"param": param}