diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index d9335857..e807d977 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,18 +2,66 @@ ## Summary - +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 = [""] # 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// + + 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 - - - -### Cookiecutter template - - +- See the general new features section. diff --git a/cookiecutter/{{cookiecutter.github_repo_name}}/pyproject.toml b/cookiecutter/{{cookiecutter.github_repo_name}}/pyproject.toml index a0b31683..5ef8e099 100644 --- a/cookiecutter/{{cookiecutter.github_repo_name}}/pyproject.toml +++ b/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]] diff --git a/pyproject.toml b/pyproject.toml index c4349ed6..fde8daf2 100644 --- a/pyproject.toml +++ b/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 diff --git a/src/frequenz/repo/config/__init__.py b/src/frequenz/repo/config/__init__.py index 6b87a611..b474c18f 100644 --- a/src/frequenz/repo/config/__init__.py +++ b/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 diff --git a/src/frequenz/repo/config/nox/config.py b/src/frequenz/repo/config/nox/config.py index 2a3a9021..814fae7b 100644 --- a/src/frequenz/repo/config/nox/config.py +++ b/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,6 +129,8 @@ 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. @@ -130,47 +138,13 @@ def path_args(self, session: _nox.Session, /) -> list[str]: 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 diff --git a/src/frequenz/repo/config/nox/default.py b/src/frequenz/repo/config/nox/default.py index 9da50e45..40498ce0 100644 --- a/src/frequenz/repo/config/nox/default.py +++ b/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", diff --git a/src/frequenz/repo/config/nox/session.py b/src/frequenz/repo/config/nox/session.py index 67b91988..c567d2e9 100644 --- a/src/frequenz/repo/config/nox/session.py +++ b/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 diff --git a/tests_golden/integration/test_cookiecutter_generation/actor/frequenz-actor-test/pyproject.toml b/tests_golden/integration/test_cookiecutter_generation/actor/frequenz-actor-test/pyproject.toml index a8ead418..628a70a3 100644 --- a/tests_golden/integration/test_cookiecutter_generation/actor/frequenz-actor-test/pyproject.toml +++ b/tests_golden/integration/test_cookiecutter_generation/actor/frequenz-actor-test/pyproject.toml @@ -46,7 +46,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"] @@ -132,6 +132,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] @@ -139,6 +143,18 @@ testpaths = ["tests", "src"] asyncio_mode = "auto" required_plugins = ["pytest-asyncio", "pytest-mock"] +[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.actor.test"] +strict = true + [[tool.mypy.overrides]] module = ["async_solipsism", "async_solipsism.*", "sybil", "sybil.*"] ignore_missing_imports = true diff --git a/tests_golden/integration/test_cookiecutter_generation/api/frequenz-api-test/pyproject.toml b/tests_golden/integration/test_cookiecutter_generation/api/frequenz-api-test/pyproject.toml index 8f387424..a767775a 100644 --- a/tests_golden/integration/test_cookiecutter_generation/api/frequenz-api-test/pyproject.toml +++ b/tests_golden/integration/test_cookiecutter_generation/api/frequenz-api-test/pyproject.toml @@ -44,7 +44,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"] @@ -59,6 +59,7 @@ dev-mkdocs = [ ] dev-mypy = [ "mypy == 1.5.1", + "grpc-stubs == 1.53.0.2", # For checking the noxfile, docs/ script, and tests "frequenz-api-test[dev-mkdocs,dev-noxfile,dev-pytest]", ] @@ -126,11 +127,27 @@ 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] testpaths = ["pytests"] +[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.api.test"] +strict = true + [tool.setuptools.package-dir] "" = "py" diff --git a/tests_golden/integration/test_cookiecutter_generation/app/frequenz-app-test/pyproject.toml b/tests_golden/integration/test_cookiecutter_generation/app/frequenz-app-test/pyproject.toml index d3a88ce4..68d0ee32 100644 --- a/tests_golden/integration/test_cookiecutter_generation/app/frequenz-app-test/pyproject.toml +++ b/tests_golden/integration/test_cookiecutter_generation/app/frequenz-app-test/pyproject.toml @@ -45,7 +45,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"] @@ -131,6 +131,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] @@ -138,6 +142,18 @@ testpaths = ["tests", "src"] asyncio_mode = "auto" required_plugins = ["pytest-asyncio", "pytest-mock"] +[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.app.test"] +strict = true + [[tool.mypy.overrides]] module = ["async_solipsism", "async_solipsism.*", "sybil", "sybil.*"] ignore_missing_imports = true diff --git a/tests_golden/integration/test_cookiecutter_generation/lib/frequenz-test-python/pyproject.toml b/tests_golden/integration/test_cookiecutter_generation/lib/frequenz-test-python/pyproject.toml index 1ec8977b..94f44e21 100644 --- a/tests_golden/integration/test_cookiecutter_generation/lib/frequenz-test-python/pyproject.toml +++ b/tests_golden/integration/test_cookiecutter_generation/lib/frequenz-test-python/pyproject.toml @@ -42,7 +42,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"] @@ -128,6 +128,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] @@ -135,6 +139,18 @@ testpaths = ["tests", "src"] asyncio_mode = "auto" required_plugins = ["pytest-asyncio", "pytest-mock"] +[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.test"] +strict = true + [[tool.mypy.overrides]] module = ["async_solipsism", "async_solipsism.*", "sybil", "sybil.*"] ignore_missing_imports = true diff --git a/tests_golden/integration/test_cookiecutter_generation/model/frequenz-model-test/pyproject.toml b/tests_golden/integration/test_cookiecutter_generation/model/frequenz-model-test/pyproject.toml index e775b575..56fd0510 100644 --- a/tests_golden/integration/test_cookiecutter_generation/model/frequenz-model-test/pyproject.toml +++ b/tests_golden/integration/test_cookiecutter_generation/model/frequenz-model-test/pyproject.toml @@ -46,7 +46,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"] @@ -132,6 +132,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] @@ -139,6 +143,18 @@ testpaths = ["tests", "src"] asyncio_mode = "auto" required_plugins = ["pytest-asyncio", "pytest-mock"] +[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.model.test"] +strict = true + [[tool.mypy.overrides]] module = ["async_solipsism", "async_solipsism.*", "sybil", "sybil.*"] ignore_missing_imports = true