CI: pre-commit

[Borda]

Description

Enforcing the already defined styles such as Isort or pep8
Moreover, I would suggest enabling this nice bot that does the fixes for you within open PR
https://pre-commit.ci/

Status

Ready/Work in progress/Hold

Types of changes

• Non-breaking change (fix or new feature that would not break existing functionality).
• Breaking change (fix or new feature that would cause existing functionality to change).
• New tests added to cover the changes.
• Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
• Quick tests passed locally by running ./runtests.sh --quick --unittests.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[wyli]

thanks, this looks great, perhaps we don't need isort/black/flak8 here, those are enforced by runtests.sh, monai's codeformater ci, and github actions already. the rest are all very helpful.

[Borda]

monai's codeformater ci, and github actions already. the rest are all very helpful.

there is a certain limitation with using it in GHA, if a bot push to a PR it does not trigger any other actions so the user needs to do e "void" commit to re-start this action... This is the advantage of any external bot, after their edit in PR all GH a are properly triggered 🐰

runtests.sh

is nice, but persona I would say that have formatting in pre-commit is more transparent also it can be paired wth git so it is used for each commit automatically

[wyli]

this codebase has been using the /black command for a long time and I'd say the approach works well with our daily workflow and generates quite reliable results efficiently https://github.com/Project-MONAI/MONAI/blob/dev/CONTRIBUTING.md#automatic-code-formatting

in my mind this pre-commit.ci tool could be a complementary tool for us to run manually from time to time, fixing minor issues such as end-of-file-fixer check-executables-have-shebangs

[Borda]

this codebase has been using the /black

yes, in such a case, yes, but still as it is implemented in GHA, it would trigger nay consecutive action, right?

in my mind this pre-commit.ci tool could be a complementary tool

well yes, even though this is rather meant as continuous fixing, you can see how it works for example on TorchMetrics package
https://github.com/PyTorchLightning/metrics/pull/478/commits

[wyli]

@monai-bot is a GitHub user and it will trigger consecutive actions. the only caveat is that the PR author should tick Allow edits from maintainers

[Borda]

@monai-bot is a GitHub user and it will trigger consecutive actions. the only caveat is that the PR author should tick Allow edits from maintainers

ok, so shall I close this PR? 🐰

[wyli]

no, this one is still useful, if you please

• comment out line 38 - 63 in .pre-commit-config.yaml (as we handle these elsewhere), and
• reset monai/_version.py (which is an auto-generated python file)

I'll merge this PR. many thanks for the discussions here! cc @Project-MONAI/core-reviewers

[Borda]

this one is still useful, if you please

all you requested have been fixed :]

[wyli]

/build

[wyli]

The dev CI will use the latest black and flake8, but this pre-commit CI config will update the tool's version quarterly. It may cause some conflicting results at some point. Let's merge this for now and later may need to remove the black and flake8 config in the pre-commit CI.

[Borda]

pre-commit CI config will update the tool's version quarterly. It may cause some conflicting results at some point

you can set it weekly or monthly but I found it more reliable to have some stable version that scratching my head that yesterday all worked fine and this morning it does not pass... to find that some formatting package made release yesterday night which can be eventually buggy (as from experience it happed a few times and you need to ban the particular version)

later may need to remove the black and flake8 config in the pre-commit CI

I have already commented it out so black is not there