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

Add pep8 check to CI #234

Closed
spencerahill opened this issue Nov 15, 2017 · 8 comments
Closed

Add pep8 check to CI #234

spencerahill opened this issue Nov 15, 2017 · 8 comments

Comments

@spencerahill
Copy link
Owner

Let's leave the style nitpicks to the robots. Should be easily able to adapt xarray's here: https://github.com/pydata/xarray/blob/master/.travis.yml#L94

@spencerahill
Copy link
Owner Author

I think the only modification required will be to replace master with develop in that line.

@spencerkclark
Copy link
Collaborator

Minor addition here -- I also like (as xarray does) running the test suite with the --verbose flag with pytest, so it would be great if we could add that in this same PR as well: https://github.com/pydata/xarray/blob/8267fdb1093bba3934a172cf71128470698279cd/.travis.yml#L93

@spencerkclark
Copy link
Collaborator

Let's leave the style nitpicks to the robots. Should be easily able to adapt xarray's here: https://github.com/pydata/xarray/blob/master/.travis.yml#L94

I think we may also need to add flake8 to our build environments.

@spencerahill
Copy link
Owner Author

@spencerkclark
Copy link
Collaborator

Indeed that is cleaner. If we go that route we may want to fix all the existing flake8 issues first :)

@micahkim23
Copy link
Collaborator

@spencerkclark Do you just mean fixing #238?

@spencerkclark
Copy link
Collaborator

@micahkim23 sorry for being a little vague; I still consider #238 as a separate issue. I meant that if we go with @spencerahill's suggestion we are no longer running flake8 on just the diff between the develop and the PR branches. Therefore if we want the CI to only flag changes to the existing code, we need to make sure the existing code is totally clean.

Currently it flags the following issues:

$ flake8 -j auto aospy
aospy/docs/conf.py:33:1: E402 module level import not at top of file
aospy/docs/conf.py:33:1: F401 'numpy' imported but unused
aospy/docs/conf.py:143:1: E402 module level import not at top of file
aospy/docs/conf.py:170:80: E501 line too long (80 > 79 characters)
aospy/aospy/automate.py:252:25: F841 local variable 'e' is assigned to but never used
aospy/aospy/automate.py:439:80: E501 line too long (96 > 79 characters)
aospy/aospy/__init__.py:2:1: F401 '.__config__.user_path' imported but unused
aospy/aospy/__init__.py:5:1: F401 '.var' imported but unused
aospy/aospy/__init__.py:7:1: F401 '.region' imported but unused
aospy/aospy/__init__.py:9:1: F401 '.run' imported but unused
aospy/aospy/__init__.py:11:1: F401 '.model' imported but unused
aospy/aospy/__init__.py:13:1: F401 '.proj' imported but unused
aospy/aospy/__init__.py:16:1: F401 '.calc.CalcInterface' imported but unused
aospy/aospy/__init__.py:16:1: F401 '.calc.Calc' imported but unused
aospy/aospy/__init__.py:17:1: F401 '.automate.submit_mult_calcs' imported but unused
aospy/aospy/__init__.py:18:1: F401 '.examples' imported but unused
aospy/aospy/model.py:153:80: E501 line too long (84 > 79 characters)
aospy/aospy/proj.py:5:1: F401 '.utils' imported but unused
aospy/aospy/calc.py:304:17: E129 visually indented line with same indent as next logical line
aospy/aospy/calc.py:686:17: E129 visually indented line with same indent as next logical line
aospy/aospy/calc.py:723:30: F821 undefined name 'PFULL_STR'
aospy/aospy/calc.py:772:1: E302 expected 2 blank lines, found 1
aospy/aospy/calc.py:783:1: E302 expected 2 blank lines, found 1
aospy/aospy/test/test_calc_basic.py:17:1: E302 expected 2 blank lines, found 1
aospy/aospy/test/test_calc_basic.py:32:1: E302 expected 2 blank lines, found 1
aospy/aospy/test/test_calc_basic.py:37:1: E302 expected 2 blank lines, found 1
aospy/aospy/test/test_calc_basic.py:129:1: E302 expected 2 blank lines, found 1
aospy/aospy/test/test_calc_basic.py:158:1: E302 expected 2 blank lines, found 1
aospy/aospy/test/test_calc_basic.py:179:17: E127 continuation line over-indented for visual indent
aospy/aospy/test/test_calc_basic.py:190:1: E305 expected 2 blank lines after class or function definition, found 1
aospy/aospy/test/__init__.py:6:9: F401 'pytest_catchlog' imported but unused
aospy/aospy/test/data/objects/examples.py:13:1: E305 expected 2 blank lines after class or function definition, found 1
aospy/aospy/utils/__init__.py:2:1: F401 '.io' imported but unused
aospy/aospy/utils/__init__.py:3:1: F401 '.times' imported but unused
aospy/aospy/utils/__init__.py:4:1: F401 '.vertcoord' imported but unused
aospy/aospy/utils/times.py:398:80: E501 line too long (107 > 79 characters)
aospy/aospy/examples/__init__.py:1:1: F401 '.example_obj_lib' imported but unused

Apparently some tools exist that can help automate this process, but I have not used them before:

@spencerahill
Copy link
Owner Author

I must say, I'm actually quite pleased that we had that few PEP8 violations in the codebase 😄

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

No branches or pull requests

3 participants