Fix the new pydoclint check DOC503

This check verifies that the `Raises` section of a method docstring
contains the same exception types present in `raise` statements in the
code.

Some checks were correctly failing because of wrong documentation or
typos, but some fail only because `pydoclint` is not smart enough to
figure out the type of variables, so one must always have `raise
ExceptionType` in the code to make the check pass. It also fails when
the exception is raised indirectly, but we want to document it anyways,
for example if raised by some utility function or private method.

For cases where this is possible, the code was changed to do explicit
`raise ExceptionType`, for other cases the check is simply disabled. In
other cases a `raise` was replaced with an `assert_never` to make the
check pass and also make sure a failure is detected by `mypy` instead of
at runtime.

Signed-off-by: Leandro Lucarella <[email protected]>

Author: llucax

src/frequenz/sdk/_internal/_channels.py

@@ -153,20 +153,20 @@ def get_or_create(self, message_type: type[T], key: str) -> Broadcast[T]:
 
         entry = self._channels[key]
         if entry.message_type is not message_type:
-            exception = ValueError(
+            error_message = (
                 f"Type mismatch, a channel for key {key!r} exists and the requested "
                 f"message type {message_type} is not the same as the existing "
                 f"message type {entry.message_type}."
             )
             if _logger.isEnabledFor(logging.DEBUG):
                 _logger.debug(
                     "%s at:\n%s",
-                    str(exception),
+                    error_message,
                     # We skip the last frame because it's this method, and limit the
                     # stack to 9 frames to avoid adding too much noise.
                     "".join(traceback.format_stack(limit=10)[:9]),
                 )
-            raise exception
+            raise ValueError(error_message)
 
         return typing.cast(Broadcast[T], entry.channel)
 

src/frequenz/sdk/actor/_background_service.py

@@ -129,7 +129,9 @@ def cancel(self, msg: str | None = None) -> None:
         for task in self._tasks:
             task.cancel(msg)
 
-    async def stop(self, msg: str | None = None) -> None:
+    # We need the noqa because pydoclint can't figure out `rest` is
+    # a `BaseExceptionGroup` instance.
+    async def stop(self, msg: str | None = None) -> None:  # noqa: DOC503
         """Stop this background service.
 
         This method cancels all running tasks spawned by this service and waits for them

src/frequenz/sdk/timeseries/_moving_window.py

@@ -9,7 +9,7 @@
 import math
 from collections.abc import Sequence
 from datetime import datetime, timedelta
-from typing import SupportsIndex, overload
+from typing import SupportsIndex, assert_never, overload
 
 import numpy as np
 from frequenz.channels import Broadcast, Receiver, Sender
@@ -282,7 +282,7 @@ def at(self, key: int | datetime) -> float:  # pylint: disable=invalid-name
             assert timestamp is not None
             return self._buffer[self._buffer.to_internal_index(timestamp)]
 
-        raise TypeError("Key has to be either a timestamp or an integer.")
+        assert_never(key)
 
     def window(
         self,
@@ -385,7 +385,10 @@ def __getitem__(self, key: datetime) -> float:
     def __getitem__(self, key: slice) -> ArrayLike:
         """See the main __getitem__ method."""
 
-    def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLike:
+    # We need the noqa because `IndexError` is raised indirectly by `at()` and `window()`
+    def __getitem__(  # noqa: DOC503
+        self, key: SupportsIndex | datetime | slice
+    ) -> float | ArrayLike:
         """
         Return a sub window of the `MovingWindow`.
 
@@ -400,12 +403,15 @@ def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLik
           where the bounds correspond to the slice bounds.
           Note that a half open interval, which is open at the end, is returned.
 
+        Note:
+            Slicing with a step other than 1 is not supported.
+
         Args:
             key: Either an integer or a timestamp or a slice of timestamps or integers.
 
         Raises:
             IndexError: when requesting an out of range timestamp or index
-            TypeError: when the key is not a datetime or slice object.
+            ValueError: when requesting a slice with a step other than 1
 
         Returns:
             A float if the key is a number or a timestamp.
@@ -422,10 +428,7 @@ def __getitem__(self, key: SupportsIndex | datetime | slice) -> float | ArrayLik
         if isinstance(key, SupportsIndex):
             return self.at(key.__index__())
 
-        raise TypeError(
-            "Key has to be either a timestamp or an integer "
-            "or a slice of timestamps or integers"
-        )
+        assert_never(key)
 
 
 # We need to register the class as a subclass of Sequence like this because

src/frequenz/sdk/timeseries/_resampling.py

@@ -801,7 +801,9 @@ async def _receive_samples(self) -> None:
             if sample.value is not None and not sample.value.isnan():
                 self._helper.add_sample(sample)
 
-    async def resample(self, timestamp: datetime) -> None:
+    # We need the noqa because pydoclint can't figure out that `recv_exception` is an
+    # `Exception` instance.
+    async def resample(self, timestamp: datetime) -> None:  # noqa: DOC503
         """Calculate a new sample for the passed `timestamp` and send it.
 
         The helper is used to calculate the new sample and the sender is used

src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_battery_power_formula.py

@@ -44,9 +44,8 @@ def generate(
             ComponentNotFound: if there are no batteries in the component graph, or if
                 they don't have an inverter as a predecessor.
             FormulaGenerationError: If a battery has a non-inverter predecessor
-                in the component graph.
-            FormulaGenerationError: If not all batteries behind a set of inverters
-                have been requested.
+                in the component graph, or if not all batteries behind a set of
+                inverters have been requested.
         """
         builder = self._get_builder(
             "battery-power", ComponentMetricId.ACTIVE_POWER, Power.from_watts

src/frequenz/sdk/timeseries/formula_engine/_formula_generators/_consumer_power_formula.py

@@ -49,7 +49,8 @@ def _are_grid_meters(self, grid_successors: set[Component]) -> bool:
             for successor in grid_successors
         )
 
-    def generate(self) -> FormulaEngine[Power]:
+    # We need the noqa here because `RuntimeError` is raised indirectly
+    def generate(self) -> FormulaEngine[Power]:  # noqa: DOC503
         """Generate formula for calculating consumer power from the component graph.
 
         Returns:

tests/actor/test_run_utils.py

@@ -35,7 +35,7 @@ async def _run(self) -> None:
         """Run the faulty actor.
 
         Raises:
-            CancelledError: the exception causes the actor to be cancelled
+            asyncio.CancelledError: the exception causes the actor to be cancelled
         """
         self.is_cancelled = True
         raise asyncio.CancelledError(f"Faulty Actor {self.name} failed")

tests/timeseries/_battery_pool/test_battery_pool.py

@@ -263,7 +263,7 @@ async def run_scenarios(
 
     Raises:
         TimeoutError: If metric update was not received.
-        AssertError: If received metric is not as expected.
+        AssertionError: If received metric is not as expected.
     """
     for idx, scenario in enumerate(scenarios):
         # Update data stream
@@ -278,19 +278,19 @@ async def run_scenarios(
         # Wait for result and check if received expected message
         try:
             msg = await asyncio.wait_for(receiver.receive(), timeout=waiting_time_sec)
-        except TimeoutError as err:
+        except TimeoutError:
             _logger.error("Test scenario %d failed with timeout error.", idx)
-            raise err
+            raise
 
         if scenario.expected_result is None:
             assert msg is None
             continue
 
         try:
             compare_messages(msg, scenario.expected_result)
-        except AssertionError as err:
+        except AssertionError:
             _logger.error("Test scenario: %d failed.", idx)
-            raise err
+            raise
 
 
 async def test_all_batteries_capacity(
@@ -404,7 +404,8 @@ def compare_messages(msg: Any, expected_msg: Any) -> None:
     assert msg_dict == expected_dict
 
 
-async def run_test_battery_status_channel(  # pylint: disable=too-many-arguments
+# pylint: disable-next=too-many-arguments,too-many-positional-arguments
+async def run_test_battery_status_channel(
     battery_status_sender: Sender[ComponentPoolStatus],
     battery_pool_metric_receiver: Receiver[T],
     all_batteries: set[int],

tests/utils/mock_microgrid_client.py

@@ -126,7 +126,8 @@ def component_graph(self) -> ComponentGraph:
         """
         return self._component_graph
 
-    async def send(self, data: ComponentData) -> None:
+    # We need the noqa because the `SenderError` is raised indirectly by `send()`.
+    async def send(self, data: ComponentData) -> None:  # noqa: DOC503
         """Send component data using channel.
 
         This simulates component sending data. Right now only battery and inverter
@@ -136,6 +137,7 @@ async def send(self, data: ComponentData) -> None:
             data: Data to be send
 
         Raises:
+            RuntimeError: if the type of data is not supported.
             SenderError: if the underlying channel was closed.
                 A [ChannelClosedError][frequenz.channels.ChannelClosedError] is
                 set as the cause.