-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Flake8 is a little flaky #8255
Comments
Hi @garciadias, thanks for reporting! It seems there might be a mismatch in our results, and I suspect the difference could stem from the Python version. Different Python versions may trigger variations in rules. If running the checks within the MONAI container or using a lower Python version might cause these errors to disappear. However, upon reviewing the reported errors, some of them, like "E226: missing whitespace around arithmetic operator", appear straightforward to fix and should indeed be addressed.
|
Hi @KumoLiu, thank you for looking into it. I wouldn't mind fixing it myself, but I was worried about fixing it for me, and other people started getting errors. Many thanks! |
Yes, we have only tested the code on Python 3.9. Therefore, we should ensure that it works correctly for this version at the very least. For support on higher versions, we can defer it for now and focus on fixing low-risk formatting issues only. MONAI/.github/workflows/pythonapp.yml Line 32 in e604d18
Thanks! |
I am refactoring some tests as described in #8231, and I noticed an odd issue with Flake 8. If I run
./runtests.sh --codeformat
I get quite some errors, but if I run./runtests.sh --autofix
or./runtests.sh --ruff
there are no complaints.I understand
Flake8
is not run in./runtests.sh --autofix
or./runtests.sh --ruff
, but it is run in./runtests.sh -f -u --net --coverage
, which you recommend running before submitting a PR. I imagine this is a disagreement betweenFlake8
andruff
. It seems to me that we should either runFlake8
on all cases or never run it.The issues raised by
Flake8
are not coming from my changes but are in the code in thedev
branch. Would you like me to solve those or ignore theFlake8
errors?See below:
./runtests.sh --codeformat
./runtests.sh --autofix
./runtests.sh --ruff
The text was updated successfully, but these errors were encountered: