ImagingDataCommons / dicomweb-client

Python client for DICOMweb RESTful services
https://dicomweb-client.readthedocs.io
MIT License
113 stars 38 forks source link

flake8 failures using specified version of pytest-flake8==1.1.1 #98

Open sjswerdloff opened 2 months ago

sjswerdloff commented 2 months ago

Apple M1, MacOS 14.6, python 3.12.4

The requirements files don't (appear to) restrict the version of flake8, and for whatever reason: flake8==5.0.4

pytest-flake8 1.1.1 resulted in:

E AttributeError: module 'flake8.options.config' has no attribute 'ConfigFileFinder'

see: https://github.com/tholo/pytest-flake8/issues/100 so I updated to: pytest-flake8 1.2.2

Which addresses the AttributeError.

Trying to update flake8 resulted in many errors, so I reverted to 5.0.4

But even with flake8 5.0.4 and pytest-flake8 1.2.2, I get Flake8 errors.

pytest --flake8 -p no:warnings

This is the summary at the end of it: ===================================================== short test summary info ====================================================== FAILED src/dicomweb_client/file.py::flake-8::FLAKE8 FAILED src/dicomweb_client/uri.py::flake-8::FLAKE8 FAILED src/dicomweb_client/web.py::flake-8::FLAKE8 FAILED tests/conftest.py::flake-8::FLAKE8 FAILED tests/test_uri.py::flake-8::FLAKE8 FAILED tests/test_web.py::flake-8::FLAKE8 ============================================ 6 failed, 269 passed, 16 skipped in 35.66s ============================================

The following identifies the types of errors that are found (probably didn't need to do sort and uniq before the cut) grep dicomweb-client flake8.out | sort | uniq | cut -d: -f4- | sort | uniq

E203 whitespace before ':' E231 missing whitespace after ',' E231 missing whitespace after ':' E702 multiple statements on one line (semicolon) W604 backticks are deprecated, use 'repr()'

My fundamental concern is that if I try to submit a PR for a feature (e.g. #37 ), first I have to pass the flake8 test. Which might fail with a PR of no content at this point. If there's a specific choice for flake8 that will pass the tests, and you want to stick with that, that's fine, I'll make sure my environment matches up (or, better, have it in the requirements_test.txt file). Alternatively, black+flake8+isort, or ruff seem to be the current approach (I prefer black+flake8+isort, brains-off and it goes in my pre-commit configuration files).

CPBridge commented 1 month ago

Hi @sjswerdloff I'm having a hard time recreating this issue. Regardless of the version of flake8 and pytest-flake8, I do not get any flake8 errors. Can you send the locations of these supposed errors to see whether they are even valid?