From da5545734bcf730a9cbcae2949838a668e76523e Mon Sep 17 00:00:00 2001 From: per1234 Date: Wed, 20 Dec 2023 11:46:20 -0800 Subject: [PATCH 1/5] Add infrastructure for type checking in Python code Type annotations are provided in the project's Python code to indicate the types of objects. Previously no infrastructure was provided to leverage or validate these annotations. A task is added here to allow contributors to easily check for typing problems using the mypy tool. A new job is added to the "Check Python" GitHub Actions workflow to run the task on every push or pull request that modifies relevant files. --- .github/workflows/check-python-task.yml | 35 +++++++++++++++ Taskfile.yml | 11 +++++ poetry.lock | 59 ++++++++++++++++++++++++- pyproject.toml | 1 + 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check-python-task.yml b/.github/workflows/check-python-task.yml index 6d12a5f..9d7522a 100644 --- a/.github/workflows/check-python-task.yml +++ b/.github/workflows/check-python-task.yml @@ -15,6 +15,8 @@ on: - "**/poetry.lock" - "**/pyproject.toml" - "**/setup.cfg" + - ".mypy.ini" + - "mypy.ini" - "Taskfile.ya?ml" - "**/tox.ini" - "**.py" @@ -25,6 +27,8 @@ on: - "**/poetry.lock" - "**/pyproject.toml" - "**/setup.cfg" + - ".mypy.ini" + - "mypy.ini" - "Taskfile.ya?ml" - "**/tox.ini" - "**.py" @@ -120,3 +124,34 @@ jobs: - name: Check formatting run: git diff --color --exit-code + + types: + needs: run-determination + if: needs.run-determination.outputs.result == 'true' + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install Python + uses: actions/setup-python@v5 + with: + python-version: ${{ env.PYTHON_VERSION }} + + - name: Install Poetry + run: pip install poetry + + - name: Install Task + uses: arduino/setup-task@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + version: 3.x + + - name: Check types + uses: liskin/gh-problem-matcher-wrap@v3 + with: + linters: mypy + run: task python:check-types diff --git a/Taskfile.yml b/Taskfile.yml index 55af182..5bd239c 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -16,6 +16,7 @@ tasks: - task: markdown:lint - task: npm:validate - task: poetry:validate + - task: python:check-types - task: python:lint - task: python:test @@ -233,6 +234,16 @@ tasks: poetry run \ coverage report + python:check-types: + desc: Check types in Python files + deps: + - task: poetry:install-deps + cmds: + - | + poetry run \ + mypy \ + . + # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-python-task/Taskfile.yml python:format: desc: Format Python files diff --git a/poetry.lock b/poetry.lock index e89423f..b4c3abd 100644 --- a/poetry.lock +++ b/poetry.lock @@ -184,6 +184,52 @@ files = [ {file = "mccabe-0.7.0.tar.gz", hash = "sha256:348e0240c33b60bbdf4e523192ef919f28cb2c3d7d5c7794f74009290f236325"}, ] +[[package]] +name = "mypy" +version = "1.7.1" +description = "Optional static typing for Python" +optional = false +python-versions = ">=3.8" +files = [ + {file = "mypy-1.7.1-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:12cce78e329838d70a204293e7b29af9faa3ab14899aec397798a4b41be7f340"}, + {file = "mypy-1.7.1-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:1484b8fa2c10adf4474f016e09d7a159602f3239075c7bf9f1627f5acf40ad49"}, + {file = "mypy-1.7.1-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:31902408f4bf54108bbfb2e35369877c01c95adc6192958684473658c322c8a5"}, + {file = "mypy-1.7.1-cp310-cp310-musllinux_1_1_x86_64.whl", hash = "sha256:f2c2521a8e4d6d769e3234350ba7b65ff5d527137cdcde13ff4d99114b0c8e7d"}, + {file = "mypy-1.7.1-cp310-cp310-win_amd64.whl", hash = "sha256:fcd2572dd4519e8a6642b733cd3a8cfc1ef94bafd0c1ceed9c94fe736cb65b6a"}, + {file = "mypy-1.7.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:4b901927f16224d0d143b925ce9a4e6b3a758010673eeded9b748f250cf4e8f7"}, + {file = "mypy-1.7.1-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:2f7f6985d05a4e3ce8255396df363046c28bea790e40617654e91ed580ca7c51"}, + {file = "mypy-1.7.1-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:944bdc21ebd620eafefc090cdf83158393ec2b1391578359776c00de00e8907a"}, + {file = "mypy-1.7.1-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:9c7ac372232c928fff0645d85f273a726970c014749b924ce5710d7d89763a28"}, + {file = "mypy-1.7.1-cp311-cp311-win_amd64.whl", hash = "sha256:f6efc9bd72258f89a3816e3a98c09d36f079c223aa345c659622f056b760ab42"}, + {file = "mypy-1.7.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:6dbdec441c60699288adf051f51a5d512b0d818526d1dcfff5a41f8cd8b4aaf1"}, + {file = "mypy-1.7.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:4fc3d14ee80cd22367caaaf6e014494415bf440980a3045bf5045b525680ac33"}, + {file = "mypy-1.7.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:2c6e4464ed5f01dc44dc9821caf67b60a4e5c3b04278286a85c067010653a0eb"}, + {file = "mypy-1.7.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:d9b338c19fa2412f76e17525c1b4f2c687a55b156320acb588df79f2e6fa9fea"}, + {file = "mypy-1.7.1-cp312-cp312-win_amd64.whl", hash = "sha256:204e0d6de5fd2317394a4eff62065614c4892d5a4d1a7ee55b765d7a3d9e3f82"}, + {file = "mypy-1.7.1-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:84860e06ba363d9c0eeabd45ac0fde4b903ad7aa4f93cd8b648385a888e23200"}, + {file = "mypy-1.7.1-cp38-cp38-macosx_11_0_arm64.whl", hash = "sha256:8c5091ebd294f7628eb25ea554852a52058ac81472c921150e3a61cdd68f75a7"}, + {file = "mypy-1.7.1-cp38-cp38-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:40716d1f821b89838589e5b3106ebbc23636ffdef5abc31f7cd0266db936067e"}, + {file = "mypy-1.7.1-cp38-cp38-musllinux_1_1_x86_64.whl", hash = "sha256:5cf3f0c5ac72139797953bd50bc6c95ac13075e62dbfcc923571180bebb662e9"}, + {file = "mypy-1.7.1-cp38-cp38-win_amd64.whl", hash = "sha256:78e25b2fd6cbb55ddfb8058417df193f0129cad5f4ee75d1502248e588d9e0d7"}, + {file = "mypy-1.7.1-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:75c4d2a6effd015786c87774e04331b6da863fc3fc4e8adfc3b40aa55ab516fe"}, + {file = "mypy-1.7.1-cp39-cp39-macosx_11_0_arm64.whl", hash = "sha256:2643d145af5292ee956aa0a83c2ce1038a3bdb26e033dadeb2f7066fb0c9abce"}, + {file = "mypy-1.7.1-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:75aa828610b67462ffe3057d4d8a4112105ed211596b750b53cbfe182f44777a"}, + {file = "mypy-1.7.1-cp39-cp39-musllinux_1_1_x86_64.whl", hash = "sha256:ee5d62d28b854eb61889cde4e1dbc10fbaa5560cb39780c3995f6737f7e82120"}, + {file = "mypy-1.7.1-cp39-cp39-win_amd64.whl", hash = "sha256:72cf32ce7dd3562373f78bd751f73c96cfb441de147cc2448a92c1a308bd0ca6"}, + {file = "mypy-1.7.1-py3-none-any.whl", hash = "sha256:f7c5d642db47376a0cc130f0de6d055056e010debdaf0707cd2b0fc7e7ef30ea"}, + {file = "mypy-1.7.1.tar.gz", hash = "sha256:fcb6d9afb1b6208b4c712af0dafdc650f518836065df0d4fb1d800f5d6773db2"}, +] + +[package.dependencies] +mypy-extensions = ">=1.0.0" +typing-extensions = ">=4.1.0" + +[package.extras] +dmypy = ["psutil (>=4.0)"] +install-types = ["pip"] +mypyc = ["setuptools (>=50)"] +reports = ["lxml"] + [[package]] name = "mypy-extensions" version = "1.0.0" @@ -320,7 +366,18 @@ pytest = ">=5.0" [package.extras] dev = ["pre-commit", "pytest-asyncio", "tox"] +[[package]] +name = "typing-extensions" +version = "4.9.0" +description = "Backported and Experimental Type Hints for Python 3.8+" +optional = false +python-versions = ">=3.8" +files = [ + {file = "typing_extensions-4.9.0-py3-none-any.whl", hash = "sha256:af72aea155e91adfc61c3ae9e0e342dbc0cba726d6cba4b6c72c1f34e47291cd"}, + {file = "typing_extensions-4.9.0.tar.gz", hash = "sha256:23478f88c37f27d76ac8aee6c905017a143b0b1b886c3c9f66bc2fd94f9f5783"}, +] + [metadata] lock-version = "2.0" python-versions = "3.11.*" -content-hash = "3dc8c722f318ce66f471be75203697be676024db9e933b108ed367a3974f067a" +content-hash = "04f6b78adae1313647bc0dee8f7b130e264ccebd2100500e11de7bd1042ea103" diff --git a/pyproject.toml b/pyproject.toml index 5e884ef..b11f34e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,6 +18,7 @@ flake8 = "6.1.0" pep8-naming = "0.13.3" pytest = "7.4.3" pytest-mock = "3.12.0" +mypy = "1.7.1" [build-system] requires = ["poetry-core"] From df1bb53d0e0b8a43d80775baff6208fb29b87be0 Mon Sep 17 00:00:00 2001 From: per1234 Date: Wed, 20 Dec 2023 11:53:53 -0800 Subject: [PATCH 2/5] Remove invalid return type annotation from `http_request` method The annotation was invalid, resulting in mypy error: error: "dict" expects 2 type arguments, but 1 given [type-arg] The `headers` field of the dictionary returned by this function is a `http.client.HTTPMessage` class object, while the other field values are strings. I didn't find a way to specify this in a return type annotation and the current approach is to provide annotations on a "best effort" basis (vs making them universally mandatory) so I settled on removing the annotation entirely. --- reportsizedeltas/reportsizedeltas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 67b948c..fd63b3b 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -581,7 +581,7 @@ def get_json_response(self, url: str): except Exception as exception: raise exception - def http_request(self, url: str, data: str | None = None) -> dict[str]: + def http_request(self, url: str, data: str | None = None): """Make a request and return a dictionary: read -- the response info -- headers From 5a6a71294349d26093517a6607c42bfe65b210c0 Mon Sep 17 00:00:00 2001 From: per1234 Date: Wed, 20 Dec 2023 12:21:40 -0800 Subject: [PATCH 3/5] Correct type annotations The type of the `data` parameter of `urllib.request.Request` is bytes. Previously the type annotations for the method parameters used to pass the data passed via this parameter were the incorrect type `str`. --- reportsizedeltas/reportsizedeltas.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index fd63b3b..1321d11 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -581,7 +581,7 @@ def get_json_response(self, url: str): except Exception as exception: raise exception - def http_request(self, url: str, data: str | None = None): + def http_request(self, url: str, data: bytes | None = None): """Make a request and return a dictionary: read -- the response info -- headers @@ -599,7 +599,7 @@ def http_request(self, url: str, data: str | None = None): "url": response_object.geturl(), } - def raw_http_request(self, url: str, data: str | None = None): + def raw_http_request(self, url: str, data: bytes | None = None): """Make a request and return an object containing the response. Keyword arguments: From d99b485e2a6055a843796ff81aa465611e308005 Mon Sep 17 00:00:00 2001 From: per1234 Date: Wed, 20 Dec 2023 12:25:55 -0800 Subject: [PATCH 4/5] Rework code that changed variable type through reuse Some of the code uses practice of reusing a variable through a chain of processing its contents in some cases the variable type was changed over the course of the processing. This caused the type checking via mypy to fail. The code is reworked to avoid any changing of variable type. --- reportsizedeltas/reportsizedeltas.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/reportsizedeltas/reportsizedeltas.py b/reportsizedeltas/reportsizedeltas.py index 1321d11..819730a 100644 --- a/reportsizedeltas/reportsizedeltas.py +++ b/reportsizedeltas/reportsizedeltas.py @@ -516,9 +516,7 @@ def comment_report(self, pr_number: int, report_markdown: str) -> None: report_markdown -- Markdown formatted report """ print("::debug::Adding deltas report comment to pull request") - report_data = {"body": report_markdown} - report_data = json.dumps(obj=report_data) - report_data = report_data.encode(encoding="utf-8") + report_data = json.dumps(obj={"body": report_markdown}).encode(encoding="utf-8") url = "https://api.github.com/repos/" + self.repository_name + "/issues/" + str(pr_number) + "/comments" self.http_request(url=url, data=report_data) @@ -710,8 +708,7 @@ def get_page_count(link_header: str | None) -> int: # Get the pagination data for link in link_header.split(","): if link[-13:] == '>; rel="last"': - link = re.split("[?&>]", link) - for parameter in link: + for parameter in re.split("[?&>]", link): if parameter[:5] == "page=": page_count = int(parameter.split("=")[1]) break From ba7c42a7065a6265cc548c9b44e72568d1d830da Mon Sep 17 00:00:00 2001 From: per1234 Date: Wed, 20 Dec 2023 12:43:04 -0800 Subject: [PATCH 5/5] Add type annotations to test helper functions --- reportsizedeltas/tests/test_reportsizedeltas.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/reportsizedeltas/tests/test_reportsizedeltas.py b/reportsizedeltas/tests/test_reportsizedeltas.py index e3d6ea7..0a55847 100644 --- a/reportsizedeltas/tests/test_reportsizedeltas.py +++ b/reportsizedeltas/tests/test_reportsizedeltas.py @@ -19,8 +19,10 @@ def get_reportsizedeltas_object( - repository_name="FooOwner/BarRepository", sketches_reports_source="foo-artifact-name", token="foo token" -): + repository_name: str = "FooOwner/BarRepository", + sketches_reports_source: str = "foo-artifact-name", + token: str = "foo token", +) -> reportsizedeltas.ReportSizeDeltas: """Return a reportsizedeltas.ReportSizeDeltas object to use in tests. Keyword arguments: @@ -33,7 +35,7 @@ def get_reportsizedeltas_object( ) -def directories_are_same(left_directory, right_directory): +def directories_are_same(left_directory, right_directory) -> bool: """Check recursively whether two directories contain the same files. Based on https://stackoverflow.com/a/6681395