STIR
STIR copied to clipboard
enforce Python style
#98 mostly talks about C++, but we need this for Python as well, which clang-format doesn't do.
Current status for STIR: #724 and #970 installed all/most infrastructure for clang-format, including pre-commit config. We still haven't run this, pending imminent merge of the TOF PR. Current doc on the process is at https://github.com/UCL/STIR/blob/master/documentation/devel/README.md
Some info from @casperdcl
black: Python & extremely opinionated by design (can only config line length) so I don't tend to use it at all yapf: Python & configurable isort: Python import sorting (compatible with above tools) Also worth mentioning static tools like flake8. I tend to put config in .pre-commit-config.yaml and pyproject.toml. I'd also recommend using https://pre-commit.ci which automatically does CI & opens correctional PRs (without needing any additional config).
Example file https://github.com/TomographicImaging/eqt/blob/main/.pre-commit-config.yaml.
Note that we run Codacy which runs Bandit, Prospector, Pylint, but this leaves manual intervention by the user, so it's better to do this via pre-commit of course.
Suggested process:
- PR adding hooks/config/doc, include link to https://github.com/google/yapf/blob/main/EDITOR%20SUPPORT.md in our EDITOR_SUPPORT.md (and others?)
- merge TOF PR to master
- PR run
pre-commit
andgit commit --author="pre-commit-format <[email protected]>"
- use https://pre-commit.ci
- tell everone this will now be enforced, but recommend to set editors accordingly. Ideally we also clean-up/confirm the process by @ashgillman in https://github.com/UCL/STIR/pull/724#issuecomment-848435651 and its follow-up
- example links are already in "I tend to put config in .pre-commit-config.yaml and pyproject.toml" which has both C++ & Python (unlike
eqt
) - step (3) "run
pre-commit && git commit
" isn't required as (4) https://pre-commit.ci will open a PR authored by the bot anyway
- step (3) "run
pre-commit && git commit
" isn't required as (4) https://pre-commit.ci will open a PR authored by the bot anyway
great. I guess after enabling pre-commit.ci, we create a PR only updates the doc, or maybe it'll run first on master
. that'll be obvious.
I frequently use Black
and MyPy
in .pre-commit-config.yaml
black: Python & extremely opinionated by design (can only config line length) so I don't tend to use it at all
I don't see black being opinionated and strict as a bad thing, it enforces uniformity as default.
It is also worth mentioning MyPy
for static typing. Though, I am not sure it would be useful for STIR as most of the python usages are small script.