-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pre-commit hooks and Github action for linting and formatting #168
Add pre-commit hooks and Github action for linting and formatting #168
Conversation
This omits the just deleted /data directory. This file and corresponding variables no longer be used at all once mlexchange#160 is addressed.
Questions
I looked at Tiled's pre-commit hooks (bluesky/tiled/blob/main/.pre-commit-config.yaml) as an example. There are a number not necessarily Python-related hooks that may be sensible to include as well, e.g. removing trailing white-space, uniform ending of files, warning about debug statements, etc. Should we also include these? |
Yes. I think these are python, actually. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I added a couple of minor comments.
- name: Set up Python 3.9.16 | ||
uses: actions/setup-python@v3 | ||
with: | ||
python-version: '3.9.16' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use 3.11?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A higher version makes sense, yes.
However, noticed that our Docker image has python:3.9
as a base image as well and thus would like to address that in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I added a couple of minor comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me!
for trimming trailing white spaces, fixing ends of files, and checking for broken symlinks, case and merge conflicts, valid python and yaml
This PR adds back the Github actions for linting and formatting that used to be part of .github/workflows/staging-deploy.yml but were removed in #158 and adds and applies corresponding pre-commit hooks and settings for
black
,isort
, andflake8
, following black's compatibility guidelines.Additional Notes
The
test-and-lint
action is applied only on pull requests, not commits, and running the hooks thus can be omitted with the--no-verify
flag for commits with work in progress.To setup pre-commit hooks run
pip install -r requirements-dev.txt
, followed bypre-commit install
. Aside from during commits, the hooks can also be run withpre-commit run --all-files
.Some selected lines have been marked with
# noqa:
to ignore flake8 errors. These are the dash component imports as well as the long lines with cmd specifications.Additionally closes #167, as the sample data directory will be created if it doesn't exists when running
python3 utils/download_sample_data.py