Merge pull request #172 from labthings/enable-bugbear

rwb27 · web-flow · commit 168b903f8be0 · 2025-08-12T18:55:35.000+01:00
Enable bugbear rules
diff --git a/docs/source/quickstart/counter.py b/docs/source/quickstart/counter.py
@@ -18,7 +18,7 @@ def increment_counter(self) -> None:
     @lt.thing_action
     def slowly_increase_counter(self) -> None:
         """Increment the counter slowly over a minute"""
-        for i in range(60):
+        for _i in range(60):
             time.sleep(1)
             self.increment_counter()
 
diff --git a/pyproject.toml b/pyproject.toml
@@ -76,14 +76,17 @@ docstring-code-format = true
 
 [tool.ruff.lint]
 external = ["DOC401", "F824", "DOC101", "DOC103"]  # used via flake8/pydoclint
-select = ["E4", "E7", "E9", "F", "D", "DOC"]
+select = ["B", "E4", "E7", "E9", "F", "D", "DOC"]
 ignore = [
     "D203",  # incompatible with D204
     "D213",  # incompatible with D212
     "DOC402",  # doesn't work with sphinx-style docstrings, use flake8/pydoclint
     "DOC201",  # doesn't work with sphinx-style docstrings, use flake8/pydoclint
     "DOC501",  # doesn't work with sphinx-style docstrings, use flake8/pydoclint
     "DOC502",  # doesn't work with sphinx-style docstrings, use flake8/pydoclint
+    "B008",  # This disallows function calls in default values.
+    # FastAPI Depends() breaks this rule, and FastAPI's response is "disable it".
+    # see https://github.com/fastapi/fastapi/issues/1522
 ]
 preview = true
 
diff --git a/src/labthings_fastapi/actions/__init__.py b/src/labthings_fastapi/actions/__init__.py
@@ -491,11 +491,11 @@ def action_invocation(
             try:
                 with self._invocations_lock:
                     return self._invocations[id].response(request=request)
-            except KeyError:
+            except KeyError as e:
                 raise HTTPException(
                     status_code=404,
                     detail="No action invocation found with ID {id}",
-                )
+                ) from e
 
         @app.get(
             ACTION_INVOCATIONS_PATH + "/{id}/output",
@@ -530,11 +530,11 @@ def action_invocation_output(id: uuid.UUID, _blob_manager: BlobIOContextDep):
             with self._invocations_lock:
                 try:
                     invocation: Any = self._invocations[id]
-                except KeyError:
+                except KeyError as e:
                     raise HTTPException(
                         status_code=404,
                         detail="No action invocation found with ID {id}",
-                    )
+                    ) from e
                 if not invocation.output:
                     raise HTTPException(
                         status_code=503,
@@ -569,11 +569,11 @@ def delete_invocation(id: uuid.UUID) -> None:
             with self._invocations_lock:
                 try:
                     invocation: Any = self._invocations[id]
-                except KeyError:
+                except KeyError as e:
                     raise HTTPException(
                         status_code=404,
                         detail="No action invocation found with ID {id}",
-                    )
+                    ) from e
                 if invocation.status not in [
                     InvocationStatus.RUNNING,
                     InvocationStatus.PENDING,
diff --git a/src/labthings_fastapi/client/__init__.py b/src/labthings_fastapi/client/__init__.py
@@ -57,8 +57,8 @@ def _get_link(obj: dict, rel: str) -> Mapping:
         raise ObjectHasNoLinksError(f"Can't find any links on {obj}.")
     try:
         return next(link for link in obj["links"] if link["rel"] == rel)
-    except StopIteration:
-        raise KeyError(f"No link was found with rel='{rel}' on {obj}.")
+    except StopIteration as e:
+        raise KeyError(f"No link was found with rel='{rel}' on {obj}.") from e
 
 
 def invocation_href(invocation: dict) -> str:
diff --git a/src/labthings_fastapi/client/in_server.py b/src/labthings_fastapi/client/in_server.py
@@ -265,13 +265,24 @@ def direct_thing_client_class(
         This class may be used as a FastAPI dependency: see :ref:`things_from_things`.
     """
 
-    def init_proxy(self, request: Request, **dependencies: Mapping[str, Any]):
-        f"""A client for {thing_class} at {thing_path}"""
+    def init_proxy(
+        self: DirectThingClient, request: Request, **dependencies: Mapping[str, Any]
+    ):
+        r"""Initialise a DirectThingClient (this docstring will be replaced).
+
+        :param self: The DirectThingClient instance we're initialising.
+        :param request: a FastAPI Request option (will be supplied by FastAPI).
+        :param \**dependencies: Other keyword arguments will be saved as
+            dependencies. FastAPI will look at the signature (which we will
+            manipulate below) to determine these.
+        """
         # NB this definition isimportant, as we must modify its signature.
         # Inheriting __init__ means we'll accidentally modify the signature
         # of `DirectThingClient` with bad results.
         DirectThingClient.__init__(self, request, **dependencies)
 
+    init_proxy.__doc__ = f"""Initialise a client for {thing_class} at {thing_path}"""
+
     # Using a class definition gets confused by the scope of the function
     # arguments - this is equivalent to a class definition but all the
     # arguments are evaluated in the right scope.
diff --git a/src/labthings_fastapi/example_things/__init__.py b/src/labthings_fastapi/example_things/__init__.py
@@ -90,7 +90,7 @@ def slowly_increase_counter(self, increments: int = 60, delay: float = 1):
         :param increments: how many times to increment.
         :param delay: the wait time between increments.
         """
-        for i in range(increments):
+        for _i in range(increments):
             time.sleep(delay)
             self.increment_counter()
 
diff --git a/src/labthings_fastapi/outputs/blob.py b/src/labthings_fastapi/outputs/blob.py
@@ -364,11 +364,11 @@ def retrieve_data(self) -> Self:
             url_to_blobdata = url_to_blobdata_ctx.get()
             self._data = url_to_blobdata(self.href)
             self.href = "blob://local"
-        except LookupError:
+        except LookupError as e:
             raise LookupError(
                 "Blobs may only be created from URLs passed in over HTTP."
                 f"The URL in question was {self.href}."
-            )
+            ) from e
         return self
 
     @model_serializer(mode="plain", when_used="always")
@@ -398,11 +398,11 @@ def to_dict(self) -> Mapping[str, str]:
                 blobdata_to_url = blobdata_to_url_ctx.get()
                 # MyPy seems to miss that `self.data` is a property, hence the ignore
                 href = blobdata_to_url(self.data)  # type: ignore[arg-type]
-            except LookupError:
+            except LookupError as e:
                 raise LookupError(
                     "Blobs may only be serialised inside the "
                     "context created by BlobIOContextDep."
-                )
+                ) from e
         else:
             href = self.href
         return {
diff --git a/src/labthings_fastapi/server/__init__.py b/src/labthings_fastapi/server/__init__.py
@@ -210,7 +210,7 @@ async def lifespan(self, app: FastAPI) -> AsyncGenerator[None]:
                 for thing in self.things.values():
                     await stack.enter_async_context(thing)
                 yield
-            for name, thing in self.things.items():
+            for _name, thing in self.things.items():
                 # Remove the blocking portal - the event loop is about to stop.
                 thing._labthings_blocking_portal = None
 
@@ -284,9 +284,8 @@ def server_from_config(config: dict) -> ThingServer:
         except ImportError as e:
             raise ImportError(
                 f"Could not import {thing['class']}, which was "
-                f"specified as the class for {path}. The error is "
-                f"printed below:\n\n{e}"
-            )
+                f"specified as the class for {path}."
+            ) from e
         instance = cls(*thing.get("args", {}), **thing.get("kwargs", {}))
         assert isinstance(instance, Thing), f"{thing['class']} is not a Thing"
         server.add_thing(instance, path)
diff --git a/src/labthings_fastapi/server/cli.py b/src/labthings_fastapi/server/cli.py
@@ -96,8 +96,10 @@ def config_from_args(args: Namespace) -> dict:
         try:
             with open(args.config) as f:
                 config = json.load(f)
-        except FileNotFoundError:
-            raise FileNotFoundError(f"Could not find configuration file {args.config}")
+        except FileNotFoundError as e:
+            raise FileNotFoundError(
+                f"Could not find configuration file {args.config}"
+            ) from e
     else:
         config = {}
     if args.json:
diff --git a/src/labthings_fastapi/thing_description/__init__.py b/src/labthings_fastapi/thing_description/__init__.py
@@ -75,9 +75,8 @@ def look_up_reference(reference: str, d: JSONSchema) -> JSONSchema:
         return resolved
     except KeyError as ke:
         raise KeyError(
-            f"The JSON reference {reference} was not found in the schema "
-            f"(original error {ke})."
-        )
+            f"The JSON reference {reference} was not found in the schema."
+        ) from ke
 
 
 def is_an_object(d: JSONSchema) -> bool:
diff --git a/src/labthings_fastapi/utilities/object_reference_to_object.py b/src/labthings_fastapi/utilities/object_reference_to_object.py
@@ -26,9 +26,9 @@ def object_reference_to_object(object_reference: str) -> Any:
         for attr in qualname.split("."):
             try:
                 obj = getattr(obj, attr)
-            except AttributeError:
+            except AttributeError as e:
                 raise ImportError(
                     f"Cannot import name {attr} from {obj} "
                     f"when loading '{object_reference}'"
-                )
+                ) from e
     return obj
diff --git a/tests/module_with_deps.py b/tests/module_with_deps.py
@@ -1,20 +1,37 @@
-from __future__ import annotations
+"""A module for testing dependencies.
+
+This module provides some classes that are used as dependencies by unit tests.
+Note that `from __future__ import annotations` is not used here. If it is used,
+we would need to add the following to the classes:
+
+.. code-block:: python
+
+    class Whatever:
+        __globals__ = globals()  # "bake in" globals so dependency injection works
+
+This relates to the way FastAPI resolves annotations to objects. There's an issue
+thread that discusses the work-around above explicitly, but it's part of a bigger
+issue discussed here:
+
+https://github.com/pydantic/pydantic/issues/2678
+
+"""
+
+from dataclasses import dataclass
 from typing import Annotated
 from fastapi import Depends, Request
 
 
 class FancyID:
-    __globals__ = globals()  # "bake in" globals so dependency injection works
-
     def __init__(self, r: Request):
         self.id = 1234
 
 
 FancyIDDep = Annotated[FancyID, Depends()]
 
 
+@dataclass
 class ClassDependsOnFancyID:
-    __globals__ = globals()  # "bake in" globals so dependency injection works
+    """A dataclass that will request a FancyID when used as a Dependency."""
 
-    def __init__(self, sub: Annotated[FancyID, Depends()]):
-        self.sub = sub
+    sub: FancyIDDep
diff --git a/tests/test_action_cancel.py b/tests/test_action_cancel.py
@@ -20,7 +20,7 @@ class CancellableCountingThing(lt.Thing):
 
     @lt.thing_action
     def count_slowly(self, cancel: lt.deps.CancelHook, n: int = 10):
-        for i in range(n):
+        for _i in range(n):
             try:
                 cancel.sleep(0.1)
             except lt.exceptions.InvocationCancelledError as e:
@@ -35,7 +35,7 @@ def count_slowly_but_ignore_cancel(self, cancel: lt.deps.CancelHook, n: int = 10
         Used to check that cancellation alter task behaviour
         """
         counting_increment = 1
-        for i in range(n):
+        for _i in range(n):
             try:
                 cancel.sleep(0.1)
             except lt.exceptions.InvocationCancelledError:
@@ -52,7 +52,7 @@ def count_and_only_cancel_if_asked_twice(
         """
         cancelled_once = False
         counting_increment = 1
-        for i in range(n):
+        for _i in range(n):
             try:
                 cancel.sleep(0.1)
             except lt.exceptions.InvocationCancelledError as e:
diff --git a/tests/test_action_logging.py b/tests/test_action_logging.py
@@ -31,7 +31,9 @@ def test_invocation_logging(caplog):
         invocation = poll_task(client, r.json())
         assert invocation["status"] == "completed"
         assert len(invocation["log"]) == len(ThingOne.LOG_MESSAGES)
-        for expected, entry in zip(ThingOne.LOG_MESSAGES, invocation["log"]):
+        for expected, entry in zip(
+            ThingOne.LOG_MESSAGES, invocation["log"], strict=True
+        ):
             assert entry["message"] == expected
 
 
diff --git a/tests/test_base_descriptor.py b/tests/test_base_descriptor.py
@@ -161,7 +161,7 @@ def test_basedescriptor_orphaned():
     """Check the right error is raised if we ask for the name outside a class."""
     prop = MockProperty()
     with pytest.raises(DescriptorNotAddedToClassError):
-        prop.name
+        _ = prop.name
 
 
 def test_basedescriptor_fallback():
@@ -185,7 +185,7 @@ def test_basedescriptor_get():
     assert isinstance(Example.my_property, MockProperty)
     with pytest.raises(NotImplementedError):
         # BaseDescriptor requires `instance_get` to be overridden.
-        e.base_descriptor
+        _ = e.base_descriptor
 
 
 class MockFunctionalProperty(MockProperty):
diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py
@@ -4,6 +4,7 @@
 for actions.
 """
 
+from dataclasses import dataclass
 from fastapi import Depends, FastAPI, Request
 from labthings_fastapi.deps import InvocationID
 from fastapi.testclient import TestClient
@@ -41,9 +42,15 @@ def test_dependency_needing_request():
     """Test a dependency that requires Request object"""
     app = FastAPI()
 
+    @dataclass
     class DepClass:
-        def __init__(self, sub: Request):
-            self.sub = sub
+        r"""A class that has a dependency in its __init__.
+
+        This is a dataclass, so __init__ is generated automatically and
+        will have an argument `sub` with type `Request`\ .
+        """
+
+        sub: Request
 
     @app.post("/dep")
     def endpoint(id: DepClass = Depends()) -> bool:
diff --git a/tests/test_dependencies_2.py b/tests/test_dependencies_2.py
@@ -17,6 +17,7 @@
 mitigation against something changing in the future.
 """
 
+from dataclasses import dataclass
 from typing import Annotated
 from fastapi import Depends, FastAPI
 from fastapi.testclient import TestClient
@@ -45,6 +46,8 @@ def test_dep_from_module_with_subdep():
 
     @app.post("/endpoint")
     def endpoint(id: Annotated[ClassDependsOnFancyID, Depends()]) -> bool:
+        # Verify that the dependency is supplied, including its sub-dependency
+        assert id.sub.id == 1234
         return True
 
     with TestClient(app) as client:
@@ -101,25 +104,44 @@ def endpoint(id: DepClass = Depends()) -> bool:
 
 
 def test_class_dep_with_subdep():
-    """Add an endpoint that uses a dependency class with sub-dependency"""
+    """Add an endpoint that uses a dependency class with sub-dependency.
+
+    We do this twice, using a regular class and also a dataclass.
+    """
     app = FastAPI()
 
     class SubDepClass:
         pass
 
-    class DepClass:
+    class DepClass:  # noqa B903
+        """A regular class that has sub-dependencies via __init__.
+
+        Note that this could be a dataclass, but we want to check both
+        dataclasses and normal classes."""
+
         def __init__(self, sub: Annotated[SubDepClass, Depends()]):
             self.sub = sub
 
     @app.post("/dep")
     def endpoint(id: DepClass = Depends()) -> bool:
+        assert isinstance(id.sub, SubDepClass)
+        return True
+
+    @dataclass
+    class DepDataclass:
+        sub: Annotated[SubDepClass, Depends()]
+
+    @app.post("/dep2")
+    def endpoint2(dep: Annotated[DepDataclass, Depends()]):
+        assert isinstance(dep.sub, SubDepClass)
         return True
 
     with TestClient(app) as client:
-        r = client.post("/dep")
-        assert r.status_code == 200
-        invocation = r.json()
-        assert invocation is True
+        for url in ["/dep", "/dep2"]:
+            r = client.post(url)
+            assert r.status_code == 200
+            invocation = r.json()
+            assert invocation is True
 
 
 def test_invocation_id():
diff --git a/tests/test_example_thing.py b/tests/test_example_thing.py
diff --git a/tests/test_property.py b/tests/test_property.py
diff --git a/tests/test_server_cli.py b/tests/test_server_cli.py
