Fix mypy invocation in noxfile (#219)

When invoking `mypy`, the `src` directory (`frequenz.sdk` package) isn't
being checked because 2 bugs:

* The `src` path is excluded
* A transformation of `src` into a package name was a ill done
copy&paste from the channels repository, and said `frequenz.channels`
instead of `frequenz.sdk`

This commit fixes this issue by reworking how the conversion between
paths and packages needed by `mypy` is done. Now the conversion is more
generic and explicit, providing a list of paths and a mapping between
paths and packages.

This is a bit more verbose to declare but it can be reused more safely
and gives some extra flexibility, which can be handy when we move this
to a general tooling repo.

We also use package names for plain python files (like `noxfile.py`), so
we don't need to invoke `mypy` twice.

Also fixes most of the typing errors that sneaked in the SDK while the
type checking wasn't running, except for the `formula_enging` package
which has too many errors and will be addressed in separate PR.

Author: llucax

noxfile.py

@@ -50,13 +50,29 @@
         nox -R -e pytest_min -- -s -x tests/timeseries/test_logical_meter.py
 """
 
-from typing import List
+from __future__ import annotations
+
+from typing import Any, Iterable
 
 import nox
 import toml
 
+DEFAULT_PATH_PACKAGES = {
+    "benchmarks": "benchmarks",
+    "docs": "docs",
+    "examples": "examples",
+    "src": "frequenz.sdk",
+    "tests": "tests",
+    "noxfile.py": "noxfile",
+}
+"""A list of path to be used by default and its corresponding package name.
+
+The package name is needed for mypy, as it takes packages when full import
+checking needs to be done.
+"""
+
 
-def min_dependencies() -> List[str]:
+def min_dependencies() -> list[str]:
     """Extract the minimum dependencies from pyproject.toml.
 
     Raises:
@@ -74,7 +90,7 @@ def min_dependencies() -> List[str]:
     if not dependencies:
         raise RuntimeError(f"No dependencies found in file: {toml_file.name}")
 
-    min_deps: List[str] = []
+    min_deps: list[str] = []
     for dep in dependencies:
         min_dep = dep.split(",")[0]
         if any(op in min_dep for op in (">=", "==")):
@@ -84,7 +100,7 @@ def min_dependencies() -> List[str]:
     return min_deps
 
 
-def _source_file_paths(session: nox.Session) -> List[str]:
+def _source_file_paths(session: nox.Session) -> list[str]:
     """Return the file paths to run the checks on.
 
     If positional arguments are present in the nox session, we use those as the
@@ -98,14 +114,7 @@ def _source_file_paths(session: nox.Session) -> List[str]:
     """
     if session.posargs:
         return session.posargs
-    return [
-        "benchmarks",
-        "docs",
-        "examples",
-        "src",
-        "tests",
-        "noxfile.py",
-    ]
+    return list(DEFAULT_PATH_PACKAGES.keys())
 
 
 # Run all checks except `ci_checks` by default.  When running locally with just
@@ -131,7 +140,7 @@ def ci_checks_max(session: nox.Session) -> None:
     Args:
         session: the nox session.
     """
-    session.install(".[dev]")
+    session.install("-e", ".[dev]")
 
     formatting(session, False)
     mypy(session, False)
@@ -149,7 +158,7 @@ def formatting(session: nox.Session, install_deps: bool = True) -> None:
         install_deps: True if dependencies should be installed.
     """
     if install_deps:
-        session.install(".[format]")
+        session.install("-e", ".[format]")
 
     paths = _source_file_paths(session)
     session.run("black", "--check", *paths)
@@ -169,36 +178,24 @@ def mypy(session: nox.Session, install_deps: bool = True) -> None:
         # fast local tests with `nox -R -e mypy`.
         session.install("-e", ".[mypy]")
 
-    common_args = [
+    def _flatten(iterable: Iterable[Iterable[Any]]) -> Iterable[Any]:
+        return [item for sublist in iterable for item in sublist]
+
+    args = (
+        session.posargs
+        if session.posargs
+        else _flatten(("-p", p) for p in DEFAULT_PATH_PACKAGES.values())
+    )
+
+    session.run(
+        "mypy",
         "--install-types",
         "--namespace-packages",
         "--non-interactive",
         "--explicit-package-bases",
         "--strict",
-    ]
-
-    # If we have session arguments, we just use those...
-    if session.posargs:
-        session.run("mypy", *common_args, *session.posargs)
-        return
-
-    check_paths = _source_file_paths(session)
-    pkg_paths = [
-        path
-        for path in check_paths
-        if not path.startswith("src") and not path.endswith(".py")
-    ]
-    file_paths = [path for path in check_paths if path.endswith(".py")]
-
-    pkg_args = []
-    for pkg in pkg_paths:
-        if pkg == "src":
-            pkg = "frequenz.channels"
-        pkg_args.append("-p")
-        pkg_args.append(pkg)
-
-    session.run("mypy", *common_args, *pkg_args)
-    session.run("mypy", *common_args, *file_paths)
+        *args,
+    )
 
 
 @nox.session
@@ -231,7 +228,7 @@ def docstrings(session: nox.Session, install_deps: bool = True) -> None:
         install_deps: True if dependencies should be installed.
     """
     if install_deps:
-        session.install(".[docs-lint]")
+        session.install("-e", ".[docs-lint]")
 
     paths = _source_file_paths(session)
     session.run("pydocstyle", *paths)

pyproject.toml

@@ -33,7 +33,6 @@ dependencies = [
     "networkx >= 2.8, < 3",
     "pandas >= 1.5.2, < 2",
     "protobuf >= 4.21.6, < 5",
-    "pyarrow >= 10.0.1, < 11",
     "pydantic >= 1.9",
     "sympy >= 1.10.1, < 2",
     "toml >= 0.10",
@@ -59,6 +58,7 @@ docs-gen = [
 docs-lint = [
     "pydocstyle",
     "darglint",
+    "tomli",  # Needed by pydocstyle to read pyproject.toml
 ]
 format = [
     "black",
@@ -135,10 +135,19 @@ src_paths = ["src", "examples", "tests"]
 asyncio_mode = "auto"
 required_plugins = [ "pytest-asyncio", "pytest-mock" ]
 
+[tool.mypy]
+# This is a temporary hack, for details see:
+# * https://github.com/frequenz-floss/frequenz-sdk-python/pull/219
+# * https://github.com/frequenz-floss/frequenz-sdk-python/issues/226
+exclude = ["src/frequenz/sdk/timeseries/_formula_engine/"]
+
 [[tool.mypy.overrides]]
 module = [
     "grpc.aio",
-    "grpc.aio.*"
+    "grpc.aio.*",
+    # There is a stubs package available, but it's not working:
+    # https://github.com/eggplants/networkx-stubs/issues/1
+    "networkx",
 ]
 ignore_missing_imports = true
 

src/frequenz/sdk/_internal/asyncio.py

@@ -3,11 +3,14 @@
 
 """General purpose async tools."""
 
+from __future__ import annotations
+
 import asyncio
 from abc import ABC
+from typing import Any
 
 
-async def cancel_and_await(task: asyncio.Task) -> None:
+async def cancel_and_await(task: asyncio.Task[Any]) -> None:
     """Cancel a task and wait for it to finish.
 
     The `CancelledError` is suppresed, but any other exception will be propagated.

src/frequenz/sdk/actor/__init__.py

@@ -3,11 +3,12 @@
 
 """A base class for creating simple composable actors."""
 
+from ..timeseries._resampling import ResamplerConfig
 from ._channel_registry import ChannelRegistry
 from ._config_managing import ConfigManagingActor
 from ._data_sourcing import ComponentMetricRequest, DataSourcingActor
 from ._decorator import actor
-from ._resampling import ComponentMetricsResamplingActor, ResamplerConfig
+from ._resampling import ComponentMetricsResamplingActor
 
 __all__ = [
     "ChannelRegistry",

src/frequenz/sdk/actor/_resampling.py

@@ -105,7 +105,7 @@ async def run(self) -> None:
 
         # noqa: DAR401 error
         """
-        tasks_to_cancel: set[asyncio.Task] = set()
+        tasks_to_cancel: set[asyncio.Task[None]] = set()
         try:
             subscriptions_task = asyncio.create_task(
                 self._process_resampling_requests()

src/frequenz/sdk/actor/power_distributing/_battery_pool_status.py

@@ -54,7 +54,7 @@ class _BatteryStatusChannelHelper:
     battery_id: int
     """Id of the battery for which we should create channel."""
 
-    def __post_init__(self):
+    def __post_init__(self) -> None:
         self.name: str = f"battery-{self.battery_id}-status"
         channel = Broadcast[Status](self.name)
 

src/frequenz/sdk/actor/power_distributing/_battery_status.py

@@ -79,7 +79,7 @@ class _BlockingStatus:
     min_duration_sec: float
     max_duration_sec: float
 
-    def __post_init__(self):
+    def __post_init__(self) -> None:
         self.last_blocking_duration_sec: float = self.min_duration_sec
         self.blocked_until: Optional[datetime] = None
 
@@ -114,7 +114,7 @@ def block(self) -> float:
 
         return self.last_blocking_duration_sec
 
-    def unblock(self):
+    def unblock(self) -> None:
         """Unblock battery.
 
         This will reset duration of the next blocking timeout.
@@ -186,24 +186,26 @@ def __init__(  # pylint: disable=too-many-arguments
         self._max_data_age = max_data_age_sec
         # First battery is considered as not working.
         # Change status after first messages are received.
-        self._last_status = Status.NOT_WORKING
-        self._blocking_status = _BlockingStatus(1.0, max_blocking_duration_sec)
+        self._last_status: Status = Status.NOT_WORKING
+        self._blocking_status: _BlockingStatus = _BlockingStatus(
+            1.0, max_blocking_duration_sec
+        )
 
         inverter_id = self._find_adjacent_inverter_id(battery_id)
         if inverter_id is None:
             raise RuntimeError(f"Can't find inverter adjacent to battery: {battery_id}")
 
-        self._battery = _ComponentStreamStatus(
+        self._battery: _ComponentStreamStatus = _ComponentStreamStatus(
             battery_id, data_recv_timer=Timer(max_data_age_sec)
         )
-        self._inverter = _ComponentStreamStatus(
+        self._inverter: _ComponentStreamStatus = _ComponentStreamStatus(
             inverter_id, data_recv_timer=Timer(max_data_age_sec)
         )
 
         # Select needs receivers that can be get in async way only.
-        self._select = None
+        self._select: Select | None = None
 
-        self._task = asyncio.create_task(
+        self._task: asyncio.Task[None] = asyncio.create_task(
             self._run(status_sender, set_power_result_receiver)
         )
 

src/frequenz/sdk/actor/power_distributing/power_distributing.py

@@ -189,7 +189,7 @@ def __init__(
             max_data_age_sec=10.0,
         )
 
-    def _create_users_tasks(self) -> List[asyncio.Task[Empty]]:
+    def _create_users_tasks(self) -> List[asyncio.Task[None]]:
         """For each user create a task to wait for request.
 
         Returns:
@@ -298,6 +298,7 @@ async def run(self) -> None:
                 api, distribution, request.request_timeout_sec
             )
 
+            response: Success | PartialFailure
             if len(failed_batteries) > 0:
                 succeed_batteries = set(battery_distribution.keys()) - failed_batteries
                 response = PartialFailure(

src/frequenz/sdk/microgrid/_graph.py

@@ -139,7 +139,7 @@ def __init__(
             InvalidGraphError: if `components` and `connections` are not both `None`
                 and either of them is either `None` or empty
         """
-        self._graph = nx.DiGraph()
+        self._graph: nx.DiGraph = nx.DiGraph()
 
         if components is None and connections is None:
             return