Skip to content
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 xradar tests yeilds 514 warnings #273

Open
Steve-Roderick opened this issue Feb 14, 2025 · 1 comment
Open

flake8 xradar tests yeilds 514 warnings #273

Steve-Roderick opened this issue Feb 14, 2025 · 1 comment

Comments

@Steve-Roderick
Copy link
Contributor

Consulting CONTRIBUTING.md I see:

When you're done making changes, check that your changes pass flake8

I believe there are some latent warnings when running flake8 using the latest main branch.

$ flake8 xradar tests | wc -l
514

E501

A majority of warnings are E501

$ flake8 xradar tests  | grep 'E501 line too long' | wc -l
497

E203

$ flake8 xradar tests  | grep 'E203 line too long'

xradar/io/backends/common.py:84:71: E203 whitespace before ':'
xradar/io/backends/common.py:85:53: E203 whitespace before ':'
xradar/io/backends/furuno.py:507:30: E203 whitespace before ':'
xradar/io/backends/hpl.py:222:16: E203 whitespace before ':'
xradar/io/backends/iris.py:2597:36: E203 whitespace before ':'
xradar/io/backends/iris.py:2942:30: E203 whitespace before ':'
xradar/io/backends/iris.py:3411:33: E203 whitespace before ':'
xradar/io/backends/metek.py:183:38: E203 whitespace before ':'
xradar/io/backends/nexrad_level2.py:193:30: E203 whitespace before ':'
xradar/io/backends/nexrad_level2.py:304:38: E203 whitespace before ':'
xradar/io/backends/nexrad_level2.py:322:48: E203 whitespace before ':'
xradar/io/backends/nexrad_level2.py:329:41: E203 whitespace before ':'
xradar/io/backends/nexrad_level2.py:346:33: E203 whitespace before ':'
xradar/io/backends/rainbow.py:244:33: E203 whitespace before ':'
xradar/io/backends/rainbow.py:250:30: E203 whitespace before ':'
xradar/io/backends/rainbow.py:295:37: E203 whitespace before ':'

E402

tests/io/test_iris.py:295:1: E402 module level import not at top of file

There are obviously a couple paths forward but I leave this to the maintainers to prescribe their preferred path here. I am happy to hand bomb this (fix the outstanding warnings) if that is of interest.


I'll note that inside tox.ini, (for example), one can add a block like so:

...
[flake8]
max-line-length = 100
ignore = E203
...

^ But I believe adding this block may overwrite inherited flake8 configs.

@kmuehlbauer
Copy link
Collaborator

@Steve-Roderick Thanks for digging into this. Your contributions are much appreciated!

For our CI runs (and in pre-commit) we solely rely on black and ruff for linting.

I think we need to remove flake8 from our documentation. Instead we should advertise the usage of pre-commit.

For the usage of tox I have no suggestion how to move forward. For CI we rely on conda-forge to test on different python versions. And to be honest, if we do not test usage of tox we better should not advertise usage. Beside that, tox.ini is not up to date wrt python versions. Maybe we should remove it completely.

@openradar/xradar For the tox part this might need some discussion how to move forward. For the flake8 I'd suggest rmeoving it and advertise black/ruff instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants