Implement Python type checking and docstring validation checks #73
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a Github Action and corresponding script to validate the use of docstrings and type hints in Python code. This action will be triggered on any pull request and it posts information about missing docstrings or type hints directly on the PR. The validation is performed by mypy and pydocstyle. This feature is intended to improve code quality and promote good coding practices.
Type Hinting and Docstring Checksmypypydocstyle |
There was a problem hiding this comment.
@maxachis thank you! one thing just occurred to me: what if we made these part of the checks which are pass/fail on PRs? One less comment/notification per PR, seems worth it to me—and I don't believe it prevents you pushing commits/opening PRs
josh-chamberlain
approved these changes
Apr 15, 2024
This commit deletes the script responsible for automatically commenting on Pull Requests with the results of 'mypy' and 'pydocstyle' checks. Additionally, references to this script have been removed from the 'python_checks.yml' GitHub workflow file. As a result, the GitHub actions workflow now only performs the checks without posting comments, simplifying the review process and reducing unnecessary noise in the PR discussion.
This commit simplifies the method of detecting modified Python files in the GitHub Actions workflow file 'python_checks.yml'. Instead of writing the output of 'git diff' command to the environment variable 'MODIFIED_FILES' using multiple lines, it now does so in a single line, thereby improving readability and maintaining cleanliness in the code.
The Python checks in the Github action workflow for mypy and pydocstyle have been updated to capture their respective exit codes. An additional step has been included to observe and react to those exit codes, terminating the workflow with a failure status if any errors are detected in either of the checks.
The python_checks.yml workflow has been updated to continue on error for mypy and pydocstyle checks. This ensures other steps in the workflow continue executing, even if these checks fail, enabling a complete overview of all potential issues.
|
@josh-chamberlain Done. Because I made changes, I won't also perform the merge, so you get to press the fun green button should you find the changes satisfactory✅ |
|
@maxachis looks like there is an error in checks still |
A condition has been added to the 'Check errors' operation in the Python Checks workflow. This guarantees that the operation will only run if there have been changes to files in the environment, preventing unnecessary execution.
@mbodeantor Thanks for the catch. I needed to make sure it checks for errors only if files have been modified. That's been resolved. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Description
mypyandpydocstyleon modified Python files and commenting the output on the associated pull request.Testing
Testing has already been demonstrated in the following PRs:
Performance