Improve how mypy is run

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