Improve how mypy is run (#128)

This PR mainly split `mypy` check for the source package and dev files.

`mypy` tends to have issues with checking source files inside a `src/`
directory. There are tricks[^1] but it seems like the only way to do it
is to run `mypy` on `.` using `MYPYPATH=src`, which will still check the
whole `.` directory (include whole virtualenvs if there are any), which
is not what we want. For now we've been using `-p` to check for packages
instead of files, which seemed to have done the trick, but we need to
pass directories that don't really have a package structure (like
benchmarks or examples or tests) as packages, which is also not great.

We try a different approach, we check the source code as a package
separately, and then check the development files (tests, examples,
etc.), which are passed as simple paths. This also opens up the door to
using more relaxed rules for these development files if needed in the
future.

To achieve this we also move the `packages` specification to the
`pyproject.toml` file, so now `mypy` can be called without any arguments
to check the source code package, making it easier to call it without
relaying in `nox`.

We also move all the options to the `pyproject.toml` file, so `mypy` can
be run consistently also outside of `nox`, and the stub dependencies now
need to be specified manually in the `pyproject.toml` file, so they can
be pinned.

This is also a step towards removing `nox` (#71).

[^1]:
https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules

Author: llucax

RELEASE_NOTES.md

@@ -2,18 +2,66 @@
 
 ## Summary
 
-<!-- Here goes a general summary of what this release is about -->
+This release replaces [`darglint`](https://github.com/terrencepreilly/darglint) (not maintained anymore) with [`pydoclint`](https://github.com/jsh9/pydoclint) which brings performance and checks improvements. It also adds basic `flake8` checks and `mypy` fixes.
 
 ## Upgrading
 
 - `flake8` basic checks are enabled now. Most are already covered by `pylint`, but there are a few differences, so you might need to fix your code if `flake8` find some issues.
 
-- `darglint` was replaced by `pydoclint`, `pydoclint` can find a few more issues than `darglint`, so your code might need adjusting.
+- `darglint`:
 
-- `darglint` is not used anymore, but if it is installed, it will make `flake8` run extremely slowly anyways, so it is extremely recommended to uninstall it (`pip uninstall darglint`) and rebuild you `nox` *venvs* if you use `-R`.
+  * Replaced by `pydoclint`, `pydoclint` can find a few more issues than `darglint`, so your code might need adjusting.
+
+  * It is recommended to remove the `darglint` configuration file `.darglint` and the `darglint` `pip` package, if it is kept installed, it will make `flake8` run extremely slowly even if not used: `pip uninstall darglint`) and rebuild you `nox` *venvs* if you use `-R`.
 
 - If you are upgrading without regenerating the cookiecutter templates, you'll need to adjust the dependencies accordingly.
 
+- `nox`: The `Config.package_args()` method was removed.
+
+- `mypy`
+
+    * Options must be specified in the `pyproject.toml` file, including the option `packages` specifying the package containing the source code of the project. The documentation was updated to suggest the recommended options.
+
+    * Dependencies on *stubs* for running the type check need to be specified manually in the `pyproject.toml` file, so they can be pinned (before they were installed automatically by `mypy`.
+
+    * The `mypy` `nox` session runs `mypy` 2 times, one without options to check the package including the sources, and one with the paths of development paths (`tests`, `benchmarks`, etc.).
+
+    To migrate existing projects:
+
+    1. Add the recommended options (previously specified in the default options object):
+
+        ```toml
+        [tool.mypy]
+        explicit_package_bases = true
+        namespace_packages = true
+        packages = ["<package.name>"]  # For example: "frequenz.repo.config" for this package
+        strict = true
+        ```
+
+    2. Find out which *stubs* were previously installed automatically by `mypy`:
+
+        ```sh
+        python -m venv tmp_venv
+        . tmp_venv/bin/activate
+        pip install -e .[dev-mypy]
+        mypy --install-types
+        deactivate
+        ```
+
+    3. Look at the list of packages it offers to install and answer "no".
+
+    4. Search for the latest package version for those packages in https://pypi.org/project/<package-name>/
+
+    5. Edit the `pyproject.toml` file to add those dependencies in `dev-mypy`, for example:
+
+        ```toml
+        [project.optional-dependencies]
+        dev-mypy = [
+        "mypy == 1.5.1",
+        "types-setuptools == 68.1.0.0",
+        # ...
+        ```
+
 ### Cookiecutter template
 
 - CI: The `nox` job now uses a matrix to run the different `nox` sessions in parallel. If you use branch projection with the `nox` job you need to update the rules to include each matrix job.
@@ -26,6 +74,8 @@
 
 - `darlint` was replaced by `pydoclint`, which is way faster and detect more issues.
 
+- `nox`: The `Config.path_args()` method now accepts two optional arguments to control with paths to include.
+
 ### Cookiecutter template
 
 - Now dependabot updates will be done weekly and grouped by *required* and *optional* for minor and patch updates (major updates are still done individually for each dependency).
@@ -34,12 +84,6 @@
 
   The output of `pip freeze` is printed to be able to more easily debug different behaviours between GitHub workflow runs and local runs.
 
-- See the general new features section.
+- `mypy`: Add a commented out `no-incremental` option, which makes the run slower but prevents some issues with `mypy` giving different results on different runs.
 
-## Bug Fixes
-
-<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
-
-### Cookiecutter template
-
-<!-- Here bug fixes for cookiecutter specifically -->
+- See the general new features section.

cookiecutter/{{cookiecutter.github_repo_name}}/pyproject.toml

@@ -64,7 +64,7 @@ dev-flake8 = [
   "flake8 == 6.1.0",
   "flake8-docstrings == 1.7.0",
   "flake8-pyproject == 1.2.3",  # For reading the flake8 config from pyproject.toml
-  "pydoclint == 0.3.0",
+  "pydoclint == 0.3.1",
   "pydocstyle == 6.3.0",
 ]
 dev-formatting = ["black == 23.7.0", "isort == 5.12.0"]
@@ -79,6 +79,9 @@ dev-mkdocs = [
 ]
 dev-mypy = [
   "mypy == 1.5.1",
+{%- if cookiecutter.type == "api" %}
+  "grpc-stubs == 1.53.0.2",
+{%- endif %}
   # For checking the noxfile, docs/ script, and tests
   "{{cookiecutter.pypi_package_name}}[dev-mkdocs,dev-noxfile,dev-pytest]",
 ]
@@ -155,6 +158,10 @@ disable = [
   # pylint's unsubscriptable check is buggy and is not needed because
   # it is a type-check, for which we already have mypy.
   "unsubscriptable-object",
+  # Checked by flake8
+  "line-too-long",
+  "unused-variable",
+  "unnecessary-lambda-assignment",
 ]
 
 [tool.pytest.ini_options]
@@ -165,6 +172,18 @@ required_plugins = ["pytest-asyncio", "pytest-mock"]
 {%- else %}
 testpaths = ["pytests"]
 {%- endif %}
+
+[tool.mypy]
+explicit_package_bases = true
+namespace_packages = true
+# This option disables mypy cache, and it is sometimes useful to enable it if
+# you are getting weird intermittent error, or error in the CI but not locally
+# (or vice versa). In particular errors saying that type: ignore is not
+# used but getting the original ignored error when removing the type: ignore.
+# See for example: https://github.com/python/mypy/issues/2960
+#no_incremental = true
+packages = ["{{cookiecutter.python_package}}"]
+strict = true
 {%- if cookiecutter.type != "api" %}
 
 [[tool.mypy.overrides]]

pyproject.toml

@@ -65,7 +65,7 @@ dev-flake8 = [
   "flake8 == 6.1.0",
   "flake8-docstrings == 1.7.0",
   "flake8-pyproject == 1.2.3",  # For reading the flake8 config from pyproject.toml
-  "pydoclint == 0.3.0",
+  "pydoclint == 0.3.1",
   "pydocstyle == 6.3.0",
 ]
 dev-formatting = ["black == 23.7.0", "isort == 5.12.0"]
@@ -79,7 +79,11 @@ dev-mkdocs = [
 ]
 dev-mypy = [
   "mypy == 1.5.1",
-  "types-setuptools >= 67.6.0, < 68", # Should match the global dependency
+  "types-setuptools == 68.1.0.0", # Should match the build dependency
+  "types-Markdown == 3.4.2.10",
+  "types-PyYAML == 6.0.12.11",
+  "types-babel == 2.11.0.15",
+  "types-colorama == 0.4.15.12",
   # For checking the noxfile, docs/ script, and tests
   "frequenz-repo-config[dev-mkdocs,dev-noxfile,dev-pytest]",
 ]
@@ -147,8 +151,24 @@ disable = [
   # pylint's unsubscriptable check is buggy and is not needed because
   # it is a type-check, for which we already have mypy.
   "unsubscriptable-object",
+  # Checked by flake8
+  "line-too-long",
+  "unused-variable",
+  "unnecessary-lambda-assignment",
 ]
 
+[tool.mypy]
+explicit_package_bases = true
+namespace_packages = true
+# This option disables mypy cache, and it is sometimes useful to enable it if
+# you are getting weird intermittent error, or error in the CI but not locally
+# (or vice versa). In particular errors saying that type: ignore is not
+# used but getting the original ignored error when removing the type: ignore.
+# See for example: https://github.com/python/mypy/issues/2960
+#no_incremental = true
+packages = ["frequenz.repo.config"]
+strict = true
+
 [[tool.mypy.overrides]]
 module = ["cookiecutter", "cookiecutter.*", "sybil", "sybil.*"]
 ignore_missing_imports = true

src/frequenz/repo/config/__init__.py

@@ -52,6 +52,12 @@
 nox.configure(config)
 ```
 
+!!! note
+
+    When possible, it is recommended to define options in the `pyproject.toml` file
+    (most tools can do this), so they can be consistently used even if the tool is used
+    outside of `nox`.
+
 If you need further customization or to define new sessions, you can use the
 following modules:
 
@@ -134,7 +140,7 @@
   "flake8 == 6.1.0",
   "flake8-docstrings == 1.7.0",
   "flake8-pyproject == 1.2.3",  # For reading the flake8 config from pyproject.toml
-  "pydoclint == 0.2.4",
+  "pydoclint == 0.3.1",
   "pydocstyle == 6.3.0",
 ]
 dev-formatting = ["black == 23.3.0", "isort == 5.12.0"]
@@ -167,6 +173,39 @@
 ]
 ```
 
+## `mypy` (static type checking)
+
+To configure `mypy` you can add the recommended options to the `pyproject.toml` file as
+follows:
+
+```toml
+[tool.mypy]
+explicit_package_bases = true
+namespace_packages = true
+packages = ["your_package_name"]  # Use the actual package name here
+strict = true
+```
+
+You can just call `mypy` to check the package of your sources or you can use `mypy
+tests` to check the tests, for example.
+
+You might also need to extra optional dependencies to install type checking stubs for
+some packages.  For example for API projects you need the `grpc-stubs` package:
+
+```toml
+[project.optional-dependencies]
+# ...
+dev-mypy = [
+    # ...
+    "grpc-stubs == 1.53.0.2",
+    # ...
+]
+```
+
+You can use `mypy --install-types` to install to get a list of missing stubs, `mypy`
+will list them for you and ask if you want to proceed with the installation.  You can
+answer no and copy the list of missing stubs to the `pyproject.toml` file.
+
 ## `mkdocs` (generating documentation)
 
 ### API reference generation

src/frequenz/repo/config/nox/config.py

@@ -13,7 +13,6 @@
 """
 
 import dataclasses as _dataclasses
-import itertools as _itertools
 from typing import Self, assert_never, overload
 
 import nox as _nox
@@ -114,7 +113,14 @@ def copy(self, /) -> Self:
             extra_paths=self.extra_paths.copy(),
         )
 
-    def path_args(self, session: _nox.Session, /) -> list[str]:
+    def path_args(
+        self,
+        session: _nox.Session,
+        /,
+        *,
+        include_sources: bool = True,
+        include_extra: bool = True,
+    ) -> list[str]:
         """Return the file paths to run the checks on.
 
         If positional arguments are present in the nox session, those are used
@@ -123,54 +129,22 @@ def path_args(self, session: _nox.Session, /) -> list[str]:
 
         Args:
             session: The nox session to use to look for command-line arguments.
+            include_sources: Whether to include the source paths or not.
+            include_extra: Whether to include the extra paths or not.
 
         Returns:
             The file paths to run the checks on.
         """
         if session.posargs:
             return session.posargs
 
-        return list(
-            str(p) for p in _util.existing_paths(self.source_paths + self.extra_paths)
-        )
-
-    def package_args(self, session: _nox.Session, /) -> list[str]:
-        """Return the package names to run the checks on.
-
-        If positional arguments are present in the nox session, those are used
-        as the file paths verbatim, and if not, all **existing** `source_paths`
-        are searched for python packges by looking for `__init__.py` files.
-        `extra_paths` are used as is, only converting the paths to python
-        package names (replacing `/` with `.` and removing the suffix `.pyi?`
-        if it exists.
-
-        Args:
-            session: The nox session to use to look for command-line arguments.
-
-        Returns:
-            The package names found in the `source_paths`.
-        """
-        if session.posargs:
-            return session.posargs
-
-        sources_package_dirs_with_roots = (
-            (p, _util.find_toplevel_package_dirs(p))
-            for p in _util.existing_paths(self.source_paths)
-        )
+        paths: list[str] = []
+        if include_sources:
+            paths.extend(self.source_paths)
+        if include_extra:
+            paths.extend(self.extra_paths)
 
-        source_packages = (
-            _util.path_to_package(pkg_path, root=root)
-            for root, pkg_paths in sources_package_dirs_with_roots
-            for pkg_path in pkg_paths
-        )
-
-        extra_packages = (
-            _util.path_to_package(p) for p in _util.existing_paths(self.extra_paths)
-        )
-
-        return list(
-            _util.deduplicate(_itertools.chain(source_packages, extra_packages))
-        )
+        return list(str(p) for p in _util.existing_paths(paths))
 
 
 _config: Config | None = None

src/frequenz/repo/config/nox/default.py

@@ -35,14 +35,7 @@
         "--diff",
         "--check",
     ],
-    mypy=[
-        "--install-types",
-        "--namespace-packages",
-        "--non-interactive",
-        "--explicit-package-bases",
-        "--strict",
-    ],
-    # SDK: pylint "--extension-pkg-whitelist=pydantic"
+    mypy=[],
     pytest=[
         "-W=all",
         "-vv",

src/frequenz/repo/config/nox/session.py

@@ -26,9 +26,9 @@ def ci_checks_max(session: nox.Session) -> None:
     session.install("-e", ".[dev]")
 
     formatting(session, False)
+    flake8(session, False)
     mypy(session, False)
     pylint(session, False)
-    flake8(session, False)
     pytest_max(session, False)
 
 
@@ -62,8 +62,22 @@ def mypy(session: nox.Session, install_deps: bool = True) -> None:
         session.install("-e", ".[dev-mypy]")
 
     conf = _config.get()
-    pkg_args = _util.flatten(("-p", p) for p in conf.package_args(session))
-    session.run("mypy", *conf.opts.mypy, *pkg_args)
+
+    # If we get CLI options, we run mypy on those, but still passing the
+    # configured options (they can be overridden by the CLI options).
+    if session.posargs:
+        session.run("mypy", *conf.opts.mypy, *session.posargs)
+        return
+
+    # We separate running the mypy checks into two runs, one is the default, as
+    # configured in `pyproject.toml`, which should run against the sources.
+    session.run("mypy", *conf.opts.mypy)
+
+    # The second run checks development files, like tests, benchmarks, etc.
+    # This is an attempt to minimize mypy internal errors.
+    session.run(
+        "mypy", *conf.opts.mypy, *conf.path_args(session, include_sources=False)
+    )
 
 
 @nox.session