Try a PEP8 style checker

We should look into using flake8 to do static analysis as part of our tests...

http://pypi.python.org/pypi/flake8/ http://dancallahan.info/journal/python-flake8/

added component::archived/stem owner::atagar priority::low resolution::implemented status::closed testing type::enhancement labels

Trac:
Keywords: N/A deleted, controller added

Oops, wrong keyword.

Trac:
Keywords: controller deleted, testing added

Damian,

Do you have a particular interest in flake8? Or will separate pep8 and PyFlakes work as well?

Trac:
Username: robinson

Nope, I don't have any strong feelings about flake8. Actually, I've never used it before - the only tool I've used for static analysis of python is pylint. This ticket is just to evaluate a style checker to see if it's helpful or more hassle than it's worth.

Revising the ticket summary.

Trac:
Summary: Use flake8 for tests to Try a PEP8 style checker

I have used pep80 and I do not think you will like what it tells you about the Stem source code. PEP8 has whitespace specifications much different from the current Stem practice (e.g. indents should be four spaces, should be two blank lines above/below class definitions, etc.) To keep the hassle down, I recommend looking for something (not pep8) that allows configurable specs.

Trac:
Username: robinson

Gotcha. I wouldn't mind getting a little more in line with PEP8 if it makes collaboration easier, but there's some things like the two space indentation that I'd be very reluctant to change.

I'm gonna resolve this for now since we don't really have a good story for what we want from static analysis. Personally I find the output of pylint to be useful and reasonably configurable, but even it gets confused by core pieces of stem's codebase (like enums).

Trac:
Status: new to closed
Resolution: N/A to wontfix

Reopening. This is understandably a pain point for new contributors so we really should change it. I'd be fine with replacing our style checker [1] with something that gets us closer to PEP8 compliance. After all, it's easier for me to change my habits and vimrc than ask it of the world. ;)

My one caveat to PEP8 is the four space indentations. From what I've seen python projects very widely on this. If active contributors really want us to move to four spaces then fine, but I'd like to have some discussion around that first.

I'm neck deep in the arm migration at present so it'll be a while before I can get to this. If stem's style really annoys you then you're more than welcome to take the lead on this!

[1] https://gitweb.torproject.org/stem.git/blob/HEAD:/test/check_whitespace.py

Trac:
Status: closed to reopened
Resolution: wontfix to N/A

Damian,

I recently checked out pylint, pyflakes, flake8, and pychecker (?). None of them impressed me enough to want to use on any of my projects.

I looked into configuring pep8 for Stem and I think I was too harsh before. The --ignore flag does let one turn off specific error-checking.

pep8 --ignore=E111,E121 run_tests.py

I was able to disable "E111 indentation is not a multiple of four" and "E121 continuation line indentation is not a multiple of four" messages with the above command, which reduced the errors/warnings on run_tests.py from 295 to 120.

pep8-py2.7 --ignore=E111,E121,W293 run_tests.py

Adding a warning "W293 blank line contains whitespace" to the ignore list, gets down to 42 messages (mainly "E501 line too long").

While the current coding style is not what I'm used to, it's not unreasonable. It does mean you, Damian, get to clean up submissions from new contributors until we each figure things out.

Trac:
Username: robinson

Thanks for looking into this, Sean! I'll try to look into swapping us over this next week (barring surprises).

Replying to atagar:

Reopening. This is understandably a pain point for new contributors so we really should change it. I'd be fine with replacing our style checker [1] with something that gets us closer to PEP8 compliance. After all, it's easier for me to change my habits and vimrc than ask it of the world. ;)

My one caveat to PEP8 is the four space indentations. From what I've seen python projects very widely on this. By complying with pep8, we would be helping maintain consistency among them. :p

If active contributors really want us to move to four spaces then fine, but I'd like to have some discussion around that first.

I don't mind the current style. But, I think it would be better for Stem to comply with PEP8 completely simply because it's The Convention To Confirm To.

This is now done...

https://gitweb.torproject.org/stem.git/commitdiff/d29ec4d12567a6af6f483400658d2d2d17769a5a?hp=d580192a95d0b323165cf22cfb8bbaca11af9600

Trac:
Status: reopened to closed
Resolution: N/A to implemented

closed