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 automatic PEP8 checking to Travis #965

Closed
tmylk opened this issue Oct 20, 2016 · 13 comments
Closed

Add automatic PEP8 checking to Travis #965

tmylk opened this issue Oct 20, 2016 · 13 comments
Labels
difficulty medium Medium issue: required good gensim understanding & python skills testing Issue related with testing (code, documentation, etc)

Comments

@tmylk
Copy link
Contributor

tmylk commented Oct 20, 2016

A good example of this is sklearn

@tmylk tmylk added testing Issue related with testing (code, documentation, etc) difficulty easy Easy issue: required small fix labels Oct 20, 2016
@souravsingh
Copy link
Contributor

@tmylk I am interested in working on the issue.

@tmylk
Copy link
Contributor Author

tmylk commented Oct 20, 2016

Great. Let's add pep8, (not autopep8) to travis.yml. We don't care about long lines so it should be ignored. See this for an example https://gist.github.com/MichaelCurrie/802ce28c993ff2dd632c

@markroxor
Copy link
Contributor

@tmylk we can use pep8 but this will scan the complete repo and while working with the forks I have realised that there are way too many formatting issues. So the Travis build will fail until we rectify them all. So along with pep8 integration do you want that all the errors raised by pep8 should also be fixed under this PR?

@tmylk
Copy link
Contributor Author

tmylk commented Oct 21, 2016

@markroxor Yes, an initial autopep8 is definitely a part of this PR.

@souravsingh
Copy link
Contributor

I will be making a PR for autopep8 shortly

@tmylk
Copy link
Contributor Author

tmylk commented Oct 25, 2016

Ping @souravsingh . Thanks for taking this up!

@souravsingh
Copy link
Contributor

Apologies for not responding to the issue. I had to some commitments which came up suddenly and had to travel. I will finish up the issue once I get back( which should be by 28th)

@nishnik
Copy link

nishnik commented Dec 13, 2016

I think this issue is solved, isn't it ?

@tmylk
Copy link
Contributor Author

tmylk commented Dec 19, 2016

@nishnik It was moved to #973 by @souravsingh
your help in incorporating the coala suggestions into the code would be very welcome

@tmylk
Copy link
Contributor Author

tmylk commented Mar 27, 2017

Thanks to @SamriddhiJain adding Hanging Indent to pycodestyle in #1202, we can add a check to Travis.

Remaining items:

  • Fix existing 237 hanging indent violations either by hand or by adding hanging indent to autopep8 package
  • Fix other issues with autopep8
  • Add automatic pycodestyle check to travis
pycodestyle --statistics -qq --ignore=E501,E731,E12,W503 . 


9       E101 indentation contains mixed spaces and tabs
11      E111 indentation is not a multiple of four
8       E114 indentation is not a multiple of four (comment)
4       E116 unexpected indentation (comment)
237     E134 Not hanging indent
13      E201 whitespace after '('
20      E202 whitespace before ']'
19      E203 whitespace before ':'
1       E221 multiple spaces before operator
2       E222 multiple spaces after operator
9       E225 missing whitespace around operator
41      E226 missing whitespace around arithmetic operator
86      E231 missing whitespace after ','
33      E241 multiple spaces after ','
67      E251 unexpected spaces around keyword / parameter equals
227     E261 at least two spaces before inline comment
5       E262 inline comment should start with '# '
100     E265 block comment should start with '# '
3       E266 too many leading '#' for block comment
1       E271 multiple spaces after keyword
3       E301 expected 1 blank line, found 0
79      E302 expected 2 blank lines, found 1
169     E303 too many blank lines (3)
35      E305 expected 2 blank lines after class or function definition, found 0
2       E306 expected 1 blank line before a nested definition, found 0
5       E401 multiple imports on one line
29      E402 module level import not at top of file
3       E502 the backslash is redundant between brackets
49      E701 multiple statements on one line (colon)
1       E703 statement ends with a semicolon
1       E711 comparison to None should be 'if cond is not None:'
2       E713 test for membership should be 'not in'
35      E722 do not use bare except'
2       E741 ambiguous variable name 'l'
8       W191 indentation contains tabs
37      W291 trailing whitespace
3       W292 no newline at end of file
43      W293 blank line contains whitespace
12      W391 blank line at end of file

@tmylk
Copy link
Contributor Author

tmylk commented Apr 20, 2017

Automatic checks added in #1287. It doesn't include hanging indent yet.

Expecting it in a couple of months when PyCQA/pycodestyle#632 is merged in, and flake8 updates its' pycodestyle version.

@tmylk
Copy link
Contributor Author

tmylk commented May 22, 2017

It seems that PyCQA/pycodestyle#632 is not moving. An alternative is to use yapf from google. That has a lot of indent flags

@menshikh-iv menshikh-iv added good first issue Issue for new contributors (not required gensim understanding + very simple) difficulty medium Medium issue: required good gensim understanding & python skills and removed test before incubator difficulty easy Easy issue: required small fix good first issue Issue for new contributors (not required gensim understanding + very simple) labels Oct 16, 2017
@menshikh-iv
Copy link
Contributor

Done a long time ago (now PEP8 checked in the separate job in Travis)
https://github.com/RaRe-Technologies/gensim/blob/develop/.travis.yml#L15-L16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty medium Medium issue: required good gensim understanding & python skills testing Issue related with testing (code, documentation, etc)
Projects
None yet
Development

No branches or pull requests

5 participants