Don't ignore imports when running mypy

This allows for more strict type checking across dependencies
boundaries.

To be able to check types in a more strict way, we need to use packages
(-p) to avoid errors about having the same file in 2 packages
(src.frequenz.sdk and frequenz.sdk), so we need to translate
directory to package when running on `src`. Also we need to check
individual files separately.

A few type errors had to be fixed, most of them by adding type: ignore
because `grpc.aio` is missing stubs. Some other ignores were added to
tests for old data handling code that will probably be gone soon.

All ignores are in tests though, so it shouldn't be that bad.

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

Author: llucax

noxfile.py

@@ -14,6 +14,7 @@
 FMT_DEPS = ["black", "isort"]
 DOCSTRING_DEPS = ["pydocstyle", "darglint"]
 PYTEST_DEPS = ["pytest", "pytest-cov", "pytest-mock", "pytest-asyncio", "time-machine"]
+MYPY_DEPS = ["mypy", "pandas-stubs", "grpc-stubs"]
 
 
 def _source_file_paths(session: nox.Session) -> List[str]:
@@ -64,7 +65,7 @@ def ci_checks_max(session: nox.Session) -> None:
         session: the nox session.
     """
     session.install(
-        ".[docs]", "mypy", "pylint", "nox", *PYTEST_DEPS, *FMT_DEPS, *DOCSTRING_DEPS
+        ".[docs]", "pylint", "nox", *PYTEST_DEPS, *FMT_DEPS, *DOCSTRING_DEPS, *MYPY_DEPS
     )
 
     formatting(session, False)
@@ -101,35 +102,38 @@ def mypy(session: nox.Session, install_deps: bool = True) -> None:
     if install_deps:
         # install the package itself as editable, so that it is possible to do
         # fast local tests with `nox -R -e mypy`.
-        session.install("-e", ".[docs]", "mypy", "nox", *PYTEST_DEPS)
+        session.install("-e", ".[docs]", "nox", *MYPY_DEPS, *PYTEST_DEPS)
 
-    mypy_cmd = [
-        "mypy",
-        "--ignore-missing-imports",
+    common_args = [
         "--install-types",
         "--namespace-packages",
         "--non-interactive",
         "--explicit-package-bases",
-        "--follow-imports=silent",
         "--strict",
     ]
 
     # If we have session arguments, we just use those...
     if session.posargs:
-        session.run(*mypy_cmd, *session.posargs)
+        session.run("mypy", *common_args, *session.posargs)
         return
 
-    # Runs on the installed package
-    session.run(*mypy_cmd, "-p", "frequenz.sdk")
-
-    # Runs on the rest of the source folders
-    # Since we use other packages in the frequenz namespace, we need to run the
-    # checks for frequenz.sdk from the installed package instead of the src
-    # directory.
-    mypy_paths = [
-        path for path in _source_file_paths(session) if not path.startswith("src")
+    check_paths = _source_file_paths(session)
+    pkg_paths = [
+        path
+        for path in check_paths
+        if not path.startswith("src") and not path.endswith(".py")
     ]
-    session.run(*mypy_cmd, *mypy_paths)
+    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)
 
 
 @nox.session

pyproject.toml

@@ -99,3 +99,11 @@ src_paths = ["src", "examples", "tests"]
 [tool.pytest.ini_options]
 asyncio_mode = "auto"
 required_plugins = [ "pytest-asyncio", "pytest-mock" ]
+
+[[tools.mypy.overrides]]
+[[tool.mypy.overrides]]
+module = [
+    "grpc.aio",
+    "grpc.aio.*"
+]
+ignore_missing_imports = true

tests/test_data_handling/test_formula.py

@@ -87,9 +87,10 @@ def test_Formula() -> None:
     assert f7.symbols == set(["s1", "s2", "s3"])
 
     expected = pd.Series(index=df.index, data=[-2.0, -1.0, 0.0])
-    assert (
-        f7(**df.to_dict(orient="series")) == expected  # pylint: disable=not-a-mapping
-    ).all()
+    # We need to convert the keys as strings because they are some weird pandas
+    # type
+    symbols = {str(k): v for k, v in df.to_dict(orient="series").items()}
+    assert (f7(**symbols) == expected).all()
 
 
 def test_formula_with_broken_meter() -> None:

tests/test_data_handling/test_handle_historic_data.py

@@ -45,7 +45,7 @@ def mock_load_hd_read(
     for feature_generator in load_hd_settings.feature_generators:
         feature = feature_generator.feature
         assert isinstance(feature, str)
-        data.update({feature: np.full(len(timestamps), len(feature))})
+        data.update(feature=np.full(len(timestamps), len(feature)))  # type: ignore
 
     df_hist = pd.DataFrame(data)
     df_hist["timestamp"] = pd.to_datetime(df_hist.timestamp)
@@ -67,7 +67,7 @@ def test_load_compute_formula(mocker: MockerFixture) -> None:
             "ac_connection.total_power_active.power_supply.now",
             "ac_connection.total_power_active.power_consumption.now",
         ],
-        get_active_power,
+        get_active_power,  # type: ignore
         "power",
     )
     data_sampling_rate = 1.0

tests/test_microgrid/test_mock_api.py

@@ -18,68 +18,99 @@
 )
 from frequenz.api.microgrid.microgrid_pb2_grpc import MicrogridStub
 from google.protobuf.empty_pb2 import Empty
+from unittest.mock import Mock
 
 from . import mock_api
 
 
 def test_MockMicrogridServicer() -> None:
     api = mock_api.MockMicrogridServicer()
-    assert list(api.ListComponents(ComponentFilter(), None).components) == []
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == []
+    service_context_mock = Mock(spec=grpc.ServicerContext)
+    assert (
+        list(api.ListComponents(ComponentFilter(), service_context_mock).components)
+        == []
+    )
+    assert (
+        list(api.ListConnections(ConnectionFilter(), service_context_mock).connections)
+        == []
+    )
 
     # adding new components just appends them to the list:
     # duplicates are not prevented, as this may be wanted
     # behaviour of the mock
     api.add_component(0, ComponentCategory.COMPONENT_CATEGORY_METER)
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
-        Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER)
-    ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == []
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER)]
+    assert (
+        list(api.ListConnections(ConnectionFilter(), service_context_mock).connections)
+        == []
+    )
 
     api.add_component(0, ComponentCategory.COMPONENT_CATEGORY_BATTERY)
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == []
+    assert (
+        list(api.ListConnections(ConnectionFilter(), service_context_mock).connections)
+        == []
+    )
 
     api.add_component(0, ComponentCategory.COMPONENT_CATEGORY_METER)
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == []
+    assert (
+        list(api.ListConnections(ConnectionFilter(), service_context_mock).connections)
+        == []
+    )
 
     # similarly, duplicates are allowed when adding new connections
     api.add_connection(0, 0)
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == [
-        Connection(start=0, end=0)
-    ]
+    assert list(
+        api.ListConnections(ConnectionFilter(), service_context_mock).connections
+    ) == [Connection(start=0, end=0)]
 
     api.add_connection(7, 9)
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == [
+    assert list(
+        api.ListConnections(ConnectionFilter(), service_context_mock).connections
+    ) == [
         Connection(start=0, end=0),
         Connection(start=7, end=9),
     ]
 
     api.add_connection(0, 0)
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
         Component(id=0, category=ComponentCategory.COMPONENT_CATEGORY_METER),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == [
+    assert list(
+        api.ListConnections(ConnectionFilter(), service_context_mock).connections
+    ) == [
         Connection(start=0, end=0),
         Connection(start=7, end=9),
         Connection(start=0, end=0),
@@ -94,25 +125,33 @@ def test_MockMicrogridServicer() -> None:
             (999, ComponentCategory.COMPONENT_CATEGORY_BATTERY),
         ]
     )
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=9, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=99, category=ComponentCategory.COMPONENT_CATEGORY_INVERTER),
         Component(id=999, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == [
+    assert list(
+        api.ListConnections(ConnectionFilter(), service_context_mock).connections
+    ) == [
         Connection(start=0, end=0),
         Connection(start=7, end=9),
         Connection(start=0, end=0),
     ]
 
     # similarly `set_connections` overrides all the existing connections
     api.set_connections([(999, 9), (99, 19), (909, 101), (99, 91)])
-    assert list(api.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=9, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=99, category=ComponentCategory.COMPONENT_CATEGORY_INVERTER),
         Component(id=999, category=ComponentCategory.COMPONENT_CATEGORY_BATTERY),
     ]
-    assert list(api.ListConnections(ConnectionFilter(), None).connections) == [
+    assert list(
+        api.ListConnections(ConnectionFilter(), service_context_mock).connections
+    ) == [
         Connection(start=999, end=9),
         Connection(start=99, end=19),
         Connection(start=909, end=101),
@@ -121,20 +160,28 @@ def test_MockMicrogridServicer() -> None:
 
     # connection requests can be filtered
     assert list(
-        api.ListConnections(ConnectionFilter(starts=[999]), None).connections
+        api.ListConnections(
+            ConnectionFilter(starts=[999]), service_context_mock
+        ).connections
     ) == [Connection(start=999, end=9)]
     assert list(
-        api.ListConnections(ConnectionFilter(starts=[99]), None).connections
+        api.ListConnections(
+            ConnectionFilter(starts=[99]), service_context_mock
+        ).connections
     ) == [Connection(start=99, end=19), Connection(start=99, end=91)]
-    assert list(api.ListConnections(ConnectionFilter(ends=[19]), None).connections) == [
-        Connection(start=99, end=19)
-    ]
     assert list(
-        api.ListConnections(ConnectionFilter(ends=[101]), None).connections
+        api.ListConnections(
+            ConnectionFilter(ends=[19]), service_context_mock
+        ).connections
+    ) == [Connection(start=99, end=19)]
+    assert list(
+        api.ListConnections(
+            ConnectionFilter(ends=[101]), service_context_mock
+        ).connections
     ) == [Connection(start=909, end=101)]
     assert list(
         api.ListConnections(
-            ConnectionFilter(starts=[99, 999], ends=[9, 19]), None
+            ConnectionFilter(starts=[99, 999], ends=[9, 19]), service_context_mock
         ).connections
     ) == [Connection(start=999, end=9), Connection(start=99, end=19)]
 
@@ -148,11 +195,17 @@ def test_MockMicrogridServicer() -> None:
         ],
         connections=[(7, 8), (8, 9)],
     )
-    assert list(api_prealloc.ListComponents(ComponentFilter(), None).components) == [
+    assert list(
+        api_prealloc.ListComponents(ComponentFilter(), service_context_mock).components
+    ) == [
         Component(id=7, category=ComponentCategory.COMPONENT_CATEGORY_GRID),
         Component(id=9, category=ComponentCategory.COMPONENT_CATEGORY_LOAD),
     ]
-    assert list(api_prealloc.ListConnections(ConnectionFilter(), None).connections) == [
+    assert list(
+        api_prealloc.ListConnections(
+            ConnectionFilter(), service_context_mock
+        ).connections
+    ) == [
         Connection(start=7, end=8),
         Connection(start=8, end=9),
     ]
@@ -170,16 +223,16 @@ async def test_MockGrpcServer() -> None:
     server1 = mock_api.MockGrpcServer(servicer1, port=57809)
     await server1.start()
 
-    client = MicrogridStub(grpc.aio.insecure_channel("[::]:57809"))
+    client = MicrogridStub(grpc.aio.insecure_channel("[::]:57809"))  # type: ignore
 
-    components1 = await client.ListComponents(Empty())
+    components1 = await client.ListComponents(Empty())  # type: ignore
     assert list(components1.components) == [
         Component(id=1, category=ComponentCategory.COMPONENT_CATEGORY_GRID),
         Component(id=2, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=3, category=ComponentCategory.COMPONENT_CATEGORY_LOAD),
     ]
 
-    connections1 = await client.ListConnections(ConnectionFilter())
+    connections1 = await client.ListConnections(ConnectionFilter())  # type: ignore
     assert list(connections1.connections) == [
         Connection(start=1, end=2),
         Connection(start=2, end=3),
@@ -199,33 +252,33 @@ async def test_MockGrpcServer() -> None:
     server2 = mock_api.MockGrpcServer(servicer2, port=57809)
     await server2.start()
 
-    components2 = await client.ListComponents(Empty())
+    components2 = await client.ListComponents(Empty())  # type: ignore
     assert list(components2.components) == [
         Component(id=6, category=ComponentCategory.COMPONENT_CATEGORY_GRID),
         Component(id=77, category=ComponentCategory.COMPONENT_CATEGORY_METER),
         Component(id=888, category=ComponentCategory.COMPONENT_CATEGORY_EV_CHARGER),
         Component(id=9999, category=ComponentCategory.COMPONENT_CATEGORY_LOAD),
     ]
 
-    connections2 = await client.ListConnections(ConnectionFilter())
+    connections2 = await client.ListConnections(ConnectionFilter())  # type: ignore
     assert list(connections2.connections) == [
         Connection(start=6, end=77),
         Connection(start=6, end=888),
         Connection(start=77, end=9999),
     ]
 
-    connections2b = await client.ListConnections(ConnectionFilter(starts=[6]))
+    connections2b = await client.ListConnections(ConnectionFilter(starts=[6]))  # type: ignore
     assert list(connections2b.connections) == [
         Connection(start=6, end=77),
         Connection(start=6, end=888),
     ]
 
-    connections2c = await client.ListConnections(ConnectionFilter(ends=[9999]))
+    connections2c = await client.ListConnections(ConnectionFilter(ends=[9999]))  # type: ignore
     assert list(connections2c.connections) == [Connection(start=77, end=9999)]
 
     connections2d = await client.ListConnections(
         ConnectionFilter(starts=[6, 77], ends=[888, 9999])
-    )
+    )  # type: ignore
     assert list(connections2d.connections) == [
         Connection(start=6, end=888),
         Connection(start=77, end=9999),