From c64f7412a74be7797dcf930a30f6aa10c491b7de Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 14:23:09 +0200 Subject: [PATCH 01/13] Remove extensions from replay file We always want to use the template provided extensions, keeping our own list only makes it more likely to have errors when upgrading. Signed-off-by: Leandro Lucarella --- .cookiecutter-replay.json | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.cookiecutter-replay.json b/.cookiecutter-replay.json index 1fb8f89b..f1899973 100644 --- a/.cookiecutter-replay.json +++ b/.cookiecutter-replay.json @@ -13,17 +13,6 @@ "pypi_package_name": "frequenz-channels", "github_repo_name": "frequenz-channels-python", "default_codeowners": "@frequenz-floss/python-sdk-team", - "_extensions": [ - "jinja2_time.TimeExtension", - "local_extensions.default_codeowners", - "local_extensions.github_repo_name", - "local_extensions.keywords", - "local_extensions.pypi_package_name", - "local_extensions.python_package", - "local_extensions.src_path", - "local_extensions.title", - "local_extensions.as_identifier" - ], "_template": "gh:frequenz-floss/frequenz-repo-config-python" } } From 76114275daf798c9cb0b27ee107a913b8d125ce3 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 14:25:18 +0200 Subject: [PATCH 02/13] Add empty `Introduction` variable The new template use it to show some help about cookiecutter variables, so we include it to avoid prompts when upgrading. Signed-off-by: Leandro Lucarella --- .cookiecutter-replay.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.cookiecutter-replay.json b/.cookiecutter-replay.json index f1899973..c7810be6 100644 --- a/.cookiecutter-replay.json +++ b/.cookiecutter-replay.json @@ -1,5 +1,6 @@ { "cookiecutter": { + "Introduction": "", "type": "lib", "name": "channels", "description": "Channel implementations for Python", From 0f915fa4851ba886ead2271c46012bc3f0c1ad07 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:03:38 +0200 Subject: [PATCH 03/13] Bump repo-config version to v0.5.2 Signed-off-by: Leandro Lucarella --- pyproject.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 35b465de..e046cc6f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ requires = [ "setuptools == 67.7.2", "setuptools_scm[toml] == 7.1.0", - "frequenz-repo-config[lib] == 0.4.0", + "frequenz-repo-config[lib] == 0.5.2", ] build-backend = "setuptools.build_meta" @@ -49,14 +49,14 @@ dev-mkdocs = [ "mkdocs-material == 9.2.8", "mkdocs-section-index == 0.3.6", "mkdocstrings[python] == 0.23.0", - "frequenz-repo-config[lib] == 0.4.0", + "frequenz-repo-config[lib] == 0.5.2", ] dev-mypy = [ "mypy == 1.5.1", # For checking the noxfile, docs/ script, and tests "frequenz-channels[dev-mkdocs,dev-noxfile,dev-pytest]", ] -dev-noxfile = ["nox == 2023.4.22", "frequenz-repo-config[lib] == 0.4.0"] +dev-noxfile = ["nox == 2023.4.22", "frequenz-repo-config[lib] == 0.5.2"] dev-pylint = [ "pylint == 2.17.5", # For checking the noxfile, docs/ script, and tests From d1503a7162b1e7e1d8e395aa43676bb2198c16b4 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:04:09 +0200 Subject: [PATCH 04/13] Add common pylint options Signed-off-by: Leandro Lucarella --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index e046cc6f..aabfb7f4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -104,6 +104,9 @@ disable = [ # disabled because it conflicts with isort "wrong-import-order", "ungrouped-imports", + # pylint's unsubscriptable check is buggy and is not needed because + # it is a type-check, for which we already have mypy. + "unsubscriptable-object", ] [tool.pytest.ini_options] From 21e4624a3bb923afb82c23e89e025eeed14ae1c1 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:04:42 +0200 Subject: [PATCH 05/13] Add common source paths for isort Signed-off-by: Leandro Lucarella --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index aabfb7f4..922de0e9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,7 +90,7 @@ include = '\.pyi?$' [tool.isort] profile = "black" line_length = 88 -src_paths = ["src", "examples", "tests"] +src_paths = ["benchmarks", "examples", "src", "tests"] [tool.pylint.similarities] ignore-comments = ['yes'] From 0200914793461e284abae66c5bab97b9912bdceb Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:05:41 +0200 Subject: [PATCH 06/13] Add editorconfig file This is to have a common basic configuration that should work with most editors/IDEs. Signed-off-by: Leandro Lucarella --- .editorconfig | 26 ++++++++++++++++++++++++++ .github/labeler.yml | 1 + MANIFEST.in | 1 + 3 files changed, 28 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..c6c7f9e5 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,26 @@ +# EditorConfig is awesome: https://EditorConfig.org + +# top-most EditorConfig file +root = true + +# Unix-style newlines with a newline ending every file +[*] +end_of_line = lf +insert_final_newline = true + +# Set default charset, indent style and trimming of whitespace +[{.editorconfig,CODEOWNERS,LICENSE,*.{in,json,md,proto,py,pyi,toml,yaml,yml}}] +charset = utf-8 +indent_style = space +trim_trailing_whitespace = true + +# 4 space indentation +[*.{py,pyi}] +indent_size = 4 + +# 2 space indentation +[{.editorconfig,CODEOWNERS,LICENSE,*.{in,json,proto,toml,yaml,yml}}] +indent_size = 2 + +# No indentation size specified for *.md because different blocks have +# different indentation rules diff --git a/.github/labeler.yml b/.github/labeler.yml index 7a576fa5..cf342f6a 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -20,6 +20,7 @@ - "**/*.toml" - "**/*.yaml" - "**/*.yml" + - ".editorconfig" - ".git*" - ".git*/**" - "docs/*.py" diff --git a/MANIFEST.in b/MANIFEST.in index f537ce4c..86ba2956 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,5 +1,6 @@ exclude .gitignore exclude .darglint +exclude .editorconfig exclude noxfile.py exclude CODEOWNERS recursive-exclude .github * From a853bdc1a0402a5080729eca0e05a089c9197dbd Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:06:35 +0200 Subject: [PATCH 07/13] Use repo-config for linting examples in docstrings Now repo-config ships support to easily enable the collection and linting of code examples in docstrings, so we use that instead. Also make sure that we label the `conftest.py` file properly and we exclude it from the source distribution. Signed-off-by: Leandro Lucarella --- .github/labeler.yml | 2 + MANIFEST.in | 1 + pyproject.toml | 6 +- src/conftest.py | 211 ++------------------------------------------ 4 files changed, 10 insertions(+), 210 deletions(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index cf342f6a..edfdd7a2 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -13,6 +13,7 @@ - LICENSE "part:tests": + - "**/conftest.py" - "tests/**" "part:tooling": @@ -20,6 +21,7 @@ - "**/*.toml" - "**/*.yaml" - "**/*.yml" + - "**/conftest.py" - ".editorconfig" - ".git*" - ".git*/**" diff --git a/MANIFEST.in b/MANIFEST.in index 86ba2956..1a48ad56 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -3,6 +3,7 @@ exclude .darglint exclude .editorconfig exclude noxfile.py exclude CODEOWNERS +exclude src/conftest.py recursive-exclude .github * recursive-exclude tests * recursive-exclude benchmarks * diff --git a/pyproject.toml b/pyproject.toml index 922de0e9..b8873389 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,10 +67,8 @@ dev-pytest = [ "async-solipsism == 0.5", "hypothesis == 6.84.2", "pytest-asyncio == 0.21.1", + "frequenz-repo-config[extra-lint-examples] == 0.5.2", "pytest-mock == 3.11.1", - # For checking docs examples - "sybil == 5.0.3", - "pylint == 2.17.5", ] dev = [ "frequenz-channels[dev-mkdocs,dev-docstrings,dev-formatting,dev-mkdocs,dev-mypy,dev-noxfile,dev-pylint,dev-pytest]", @@ -110,7 +108,7 @@ disable = [ ] [tool.pytest.ini_options] -testpaths = ["tests", "src"] # src for docs examples +testpaths = ["tests", "src"] asyncio_mode = "auto" required_plugins = ["pytest-asyncio", "pytest-mock"] markers = [ diff --git a/src/conftest.py b/src/conftest.py index a5c6b352..2c6e0fcf 100644 --- a/src/conftest.py +++ b/src/conftest.py @@ -1,214 +1,13 @@ # License: MIT # Copyright © 2023 Frequenz Energy-as-a-Service GmbH -"""Pytest plugin to validate docstring code examples. +"""Validate docstring code examples. -Code examples are often wrapped in triple backticks (```) within our docstrings. +Code examples are often wrapped in triple backticks (```) within docstrings. This plugin extracts these code examples and validates them using pylint. """ +from frequenz.repo.config.pytest import examples +from sybil import Sybil -import ast -import os -import subprocess -from pathlib import Path - -from sybil import Example, Sybil -from sybil.evaluators.python import pad -from sybil.parsers.abstract.lexers import textwrap -from sybil.parsers.myst import CodeBlockParser - -PYLINT_DISABLE_COMMENT = ( - "# pylint: {}=unused-import,wildcard-import,unused-wildcard-import" -) - -FORMAT_STRING = """ -# Generated auto-imports for code example -{disable_pylint} -{imports} -{enable_pylint} - -{code}""" - - -def get_import_statements(code: str) -> list[str]: - """Get all import statements from a given code string. - - Args: - code: The code to extract import statements from. - - Returns: - A list of import statements. - """ - tree = ast.parse(code) - import_statements: list[str] = [] - - for node in ast.walk(tree): - if isinstance(node, (ast.Import, ast.ImportFrom)): - import_statement = ast.get_source_segment(code, node) - assert import_statement is not None - import_statements.append(import_statement) - - return import_statements - - -def path_to_import_statement(path: Path) -> str: - """Convert a path to a Python file to an import statement. - - Args: - path: The path to convert. - - Returns: - The import statement. - - Raises: - ValueError: If the path does not point to a Python file. - """ - # Make the path relative to the present working directory - if path.is_absolute(): - path = path.relative_to(Path.cwd()) - - # Check if the path is a Python file - if path.suffix != ".py": - raise ValueError("Path must point to a Python file (.py)") - - # Remove 'src' prefix if present - parts = path.parts - if parts[0] == "src": - parts = parts[1:] - - # Remove the '.py' extension and join parts with '.' - module_path = ".".join(parts)[:-3] - - # Create the import statement - import_statement = f"from {module_path} import *" - return import_statement - - -# We need to add the type ignore comment here because the Sybil library does not -# have type annotations. -class CustomPythonCodeBlockParser(CodeBlockParser): # type: ignore[misc] - """Code block parser that validates extracted code examples using pylint. - - This parser is a modified version of the default Python code block parser - from the Sybil library. - It uses pylint to validate the extracted code examples. - - All code examples are preceded by the original file's import statements as - well as an wildcard import of the file itself. - This allows us to use the code examples as if they were part of the original - file. - - Additionally, the code example is padded with empty lines to make sure the - line numbers are correct. - - Pylint warnings which are unimportant for code examples are disabled. - """ - - def __init__(self) -> None: - """Initialize the parser.""" - super().__init__("python") - - def evaluate(self, example: Example) -> None | str: - """Validate the extracted code example using pylint. - - Args: - example: The extracted code example. - - Returns: - None if the code example is valid, otherwise the pylint output. - """ - # Get the import statements for the original file - import_header = get_import_statements(example.document.text) - # Add a wildcard import of the original file - import_header.append( - path_to_import_statement(Path(os.path.relpath(example.path))) - ) - imports_code = "\n".join(import_header) - - # Dedent the code example - # There is also example.parsed that is already prepared, but it has - # empty lines stripped and thus fucks up the line numbers. - example_code = textwrap.dedent( - example.document.text[example.start : example.end] - ) - # Remove first line (the line with the triple backticks) - example_code = example_code[example_code.find("\n") + 1 :] - - example_with_imports = FORMAT_STRING.format( - disable_pylint=PYLINT_DISABLE_COMMENT.format("disable"), - imports=imports_code, - enable_pylint=PYLINT_DISABLE_COMMENT.format("enable"), - code=example_code, - ) - - # Make sure the line numbers are correct - source = pad( - example_with_imports, - example.line - imports_code.count("\n") - FORMAT_STRING.count("\n"), - ) - - # pylint disable parameters - pylint_disable_params = [ - "missing-module-docstring", - "missing-class-docstring", - "missing-function-docstring", - "reimported", - "unused-variable", - "no-name-in-module", - "await-outside-async", - ] - - response = validate_with_pylint(source, example.path, pylint_disable_params) - - if len(response) > 0: - response_concats = "\n".join(response) - return ( - f"Pylint validation failed for code example:\n" - f"{example_with_imports}\nOutput: {response_concats}" - ) - - return None - - -def validate_with_pylint( - code_example: str, path: str, disable_params: list[str] -) -> list[str]: - """Validate a code example using pylint. - - Args: - code_example: The code example to validate. - path: The path to the original file. - disable_params: The pylint disable parameters. - - Returns: - A list of pylint messages. - """ - try: - pylint_command = [ - "pylint", - "--disable", - ",".join(disable_params), - "--from-stdin", - path, - ] - - subprocess.run( - pylint_command, - input=code_example, - text=True, - capture_output=True, - check=True, - ) - except subprocess.CalledProcessError as exception: - output = exception.output - assert isinstance(output, str) - return output.splitlines() - - return [] - - -pytest_collect_file = Sybil( - parsers=[CustomPythonCodeBlockParser()], - patterns=["*.py"], -).pytest() +pytest_collect_file = Sybil(**examples.get_sybil_arguments()).pytest() From d5a04ef2fc8ad15d2712b2e1c8ed5608186ec7b6 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:06:54 +0200 Subject: [PATCH 08/13] ci: Checkout submodules This is not really used in this project, but it is included as part of the repo-config ci template, as it doesn't hurt either. Signed-off-by: Leandro Lucarella --- .github/workflows/ci.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index af407138..f15a93d2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -37,6 +37,8 @@ jobs: steps: - name: Fetch sources uses: actions/checkout@v4 + with: + submodules: true - name: Set up Python uses: actions/setup-python@v4 @@ -61,6 +63,8 @@ jobs: steps: - name: Fetch sources uses: actions/checkout@v4 + with: + submodules: true - name: Set up Python uses: actions/setup-python@v4 @@ -90,6 +94,8 @@ jobs: steps: - name: Fetch sources uses: actions/checkout@v4 + with: + submodules: true - name: Setup Git user and e-mail uses: frequenz-floss/setup-git-user@v2 @@ -167,6 +173,8 @@ jobs: - name: Fetch sources if: steps.mike-metadata.outputs.version uses: actions/checkout@v4 + with: + submodules: true - name: Setup Git user and e-mail if: steps.mike-metadata.outputs.version From ea766f996b9d5b262970d311ee15ab16ebbcca89 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:08:08 +0200 Subject: [PATCH 09/13] Add a workflow to make sure release notes are updated When a PR is changing files in the `proto` or `py` directories, this workflow will fail if the release notes weren't updated in the PR too, unless the label `cmd:skip-release-notes` is set in the PR, which signals that this PR doesn't need a release notes update. Signed-off-by: Leandro Lucarella --- .github/workflows/release-notes-check.yml | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/workflows/release-notes-check.yml diff --git a/.github/workflows/release-notes-check.yml b/.github/workflows/release-notes-check.yml new file mode 100644 index 00000000..e0b00ac9 --- /dev/null +++ b/.github/workflows/release-notes-check.yml @@ -0,0 +1,28 @@ +name: Release Notes Check + +on: + merge_group: + pull_request: + types: + # On by default if you specify no types. + - "opened" + - "reopened" + - "synchronize" + # For `skip-label` only. + - "labeled" + - "unlabeled" + + +jobs: + check-release-notes: + name: Check release notes are updated + runs-on: ubuntu-latest + steps: + - name: Check for a release notes update + if: github.event_name == 'pull_request' + uses: brettcannon/check-for-changed-files@4170644959a21843b31f1181f2a1761d65ef4791 # v1.2.0 + with: + file-pattern: "RELEASE_NOTES.md" + prereq-pattern: "src/**" + skip-label: "cmd:skip-release-notes" + failure-message: "Missing a release notes update. Please add one or apply the ${skip-label} label to the pull request" From 119826d0e47dc0edf6b8d12765bb4552a5f99ae0 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:08:23 +0200 Subject: [PATCH 10/13] Don't ship development files in the source distribution Signed-off-by: Leandro Lucarella --- MANIFEST.in | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 1a48ad56..cbd04173 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,9 +1,14 @@ -exclude .gitignore +exclude .cookiecutter-replay.json exclude .darglint exclude .editorconfig -exclude noxfile.py +exclude .gitignore exclude CODEOWNERS +exclude CONTRIBUTING.md +exclude mkdocs.yml +exclude noxfile.py exclude src/conftest.py recursive-exclude .github * -recursive-exclude tests * recursive-exclude benchmarks * +recursive-exclude docs * +recursive-exclude tests * +recursive-include py *.pyi From 6645654321012ad2681775cd915067c8efac3594 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:08:42 +0200 Subject: [PATCH 11/13] Add and sort mkdocs markdown extensions This also removes a duplicated entry for `pymdownx.superfences`. Signed-off-by: Leandro Lucarella --- mkdocs.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mkdocs.yml b/mkdocs.yml index 9a76cb20..38ca5e9d 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -68,9 +68,6 @@ markdown_extensions: anchor_linenums: true line_spans: __span pygments_lang_class: true - - pymdownx.superfences - - pymdownx.tasklist - - pymdownx.tabbed - pymdownx.keys - pymdownx.snippets: check_paths: true @@ -79,6 +76,8 @@ markdown_extensions: - name: mermaid class: mermaid format: "!!python/name:pymdownx.superfences.fence_code_format" + - pymdownx.tabbed + - pymdownx.tasklist - toc: permalink: "¤" From aec50d7cbcce65752d05e9c0ace4226f9c47fe59 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 17 Aug 2023 15:09:04 +0200 Subject: [PATCH 12/13] mkdocs: Add comment with more info about cross-linking Signed-off-by: Leandro Lucarella --- mkdocs.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/mkdocs.yml b/mkdocs.yml index 38ca5e9d..87185e49 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -103,6 +103,7 @@ plugins: show_root_members_full_path: true show_source: true import: + # See https://mkdocstrings.github.io/python/usage/#import for details - https://docs.python.org/3/objects.inv - https://typing-extensions.readthedocs.io/en/stable/objects.inv - search From e1e83444a1a9d14c6a999fdd771fdf1e865b1b42 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 18 Aug 2023 09:36:01 +0200 Subject: [PATCH 13/13] Bump setuptools dependency Signed-off-by: Leandro Lucarella --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index b8873389..902be050 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ [build-system] requires = [ - "setuptools == 67.7.2", + "setuptools == 68.1.0", "setuptools_scm[toml] == 7.1.0", "frequenz-repo-config[lib] == 0.5.2", ]