search

mlexchange / mlex_highres_segmentation

A Dash interface for ML-based segmentation of user-annotated large multi-dimensional image data
Other
5 stars 4 forks source link

Add pre-commit hooks and Github action for linting and formatting #168

Closed Wiebke closed 9 months ago

Wiebke commented 9 months ago

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, and flake8, 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 by pre-commit install. Aside from during commits, the hooks can also be run with pre-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

Wiebke commented 9 months ago

Questions

black has a default line length of 88 characters. It will not auto-format comments, docstrings, or strings. The previous Github action in this repo had flake8 check for a line length of 127 characters with the comment that is in line with the Github code editor. This means effectively since black runs before flake8 in the pre-commit hooks, flake8 will only complain about long lines with comments, docstrings, or strings. Is that sensible?

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?

dylanmcreynolds commented 9 months ago

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. flake8 certainly complains about trailing white-space and uniform ending of files. I don't klnow if this will warn of breakpoint() statement, but it sure would be nice.