Thanks for the feedback here and in the review.
I agree that we should aim for a style as close to PEP8 as possible.
However, I also didn't want to overshoot and my first goal was to get some
useful tooling set up, so that I don't have to constantly worry about
formatting. Once I had figured out some tooling, I thought I might as well
share it and solicit feedback.
Regarding the next steps, I'm open for anything really. I didn't know about
the --diff switch of flake8, that looks very useful. Even better of course
would be, if all python code could be converted to PEP8.
Here is a list of all PEP8 violations and their count, obtained with
"pycodestyle --statistics -qq tests":
9017 E111 indentation is not a multiple of four
902 E114 indentation is not a multiple of four (comment)
2 E116 unexpected indentation (comment)
24 E122 continuation line missing indentation or outdented
5 E124 closing bracket does not match visual indentation
105 E125 continuation line with same indent as next logical line
43 E127 continuation line over-indented for visual indent
1038 E128 continuation line under-indented for visual indent
7 E131 continuation line unaligned for hanging indent
13 E201 whitespace after '('
8 E202 whitespace before ']'
55 E203 whitespace before ':'
5 E211 whitespace before '['
5 E221 multiple spaces before operator
7 E222 multiple spaces after operator
9 E225 missing whitespace around operator
1 E227 missing whitespace around bitwise or shift operator
127 E231 missing whitespace after ':'
157 E251 unexpected spaces around keyword / parameter equals
20 E261 at least two spaces before inline comment
21 E265 block comment should start with '# '
1 E266 too many leading '#' for block comment
1 E271 multiple spaces after keyword
4 E301 expected 1 blank line, found 0
313 E302 expected 2 blank lines, found 1
16 E303 too many blank lines (2)
13 E305 expected 2 blank lines after class or function definition,
found 1
6 E306 expected 1 blank line before a nested definition, found 0
7 E402 module level import not at top of file
3800 E501 line too long (80 > 79 characters)
278 E502 the backslash is redundant between brackets
87 E701 multiple statements on one line (colon)
74 E703 statement ends with a semicolon
12 E711 comparison to None should be 'if cond is None:'
9 E712 comparison to False should be 'if cond is False:' or 'if not
cond:'
2 E713 test for membership should be 'not in'
2 E741 ambiguous variable name 'l'
1 W292 no newline at end of file
9 W391 blank line at end of file
2 W601 .has_key() is deprecated, use 'in'
19 W602 deprecated form of raising exception
If we take out the well known ones (indent, line width), it does not look
too far fetched to me to change it all to PEP8.
Thoughts?
On Tue, Jan 31, 2017 at 5:59 PM, Michael Brown <[email protected]> wrote:
> Thanks. I made some comments on the review, but I see now I should
> probably share my general view here.
>
> My general view is, if we are going to codify our Python style guide,
> I would rather we codify style conventions that are closer to standard
> Python style conventions, rather than codify what is currently done. I
> am willing to keep 2-space indents and 90-char lines, but I don't
> think anything else should be part of the conventions when those
> conventions involves ignoring PEP-008. My instinct tells me the Python
> conventions weren't conventions at all, but came up organically
> without regard to actually reading conventions or using tooling.
> Otherwise, we'd have already had a Python style guide, right?
>
> If the concern is "But there are too many noisy errors if I am editing
> an existing, large file, so we should ignore these anyway", something
> like this is possible:
>
> git diff | flake8 --diff
>
> This will only show PEP-008 problems on changed code, not whole files.
>
>
>
> On Mon, Jan 30, 2017 at 3:20 PM, Lars Volker <[email protected]> wrote:
> > Cool, thanks Michael for the reply. I added a section on Python to the
> Impala
> > Style Guide
> > <https://cwiki.apache.org/confluence/display/IMPALA/Impala+Style+Guide>.
> > Please feel free to edit it or let me know if I should make changes. I
> will
> > also send out a review to add a .pep8rc file to the repository.
> >
> > On Fri, Jan 27, 2017 at 11:56 PM, Michael Brown <[email protected]>
> wrote:
> >
> >> I prefer str.format() over the % operator, because:
> >>
> >> https://docs.python.org/2.7/library/stdtypes.html#str.format
> >>
> >> "This method of string formatting is the new standard in Python 3, and
> >> should be preferred to the % formatting described in String Formatting
> >> Operations in new code."
> >>
> >> Without an Impala Python style guide, I tend to use what I see on
> >> docs.python.org, modulo our 2-space indent and 90-char line policy.
> >>
> >>
> >> On Fri, Jan 27, 2017 at 2:44 PM, Lars Volker <[email protected]> wrote:
> >> > Hi All,
> >> >
> >> > do we have a strong preference for either old style or new style
> string
> >> > formatting in Python?
> >> >
> >> > "Hello %s!" % ("world") *vs* "Hello {0}!".format("world")
> >> >
> >> > The Impala Style Guide
> >> > <https://cwiki.apache.org/confluence/display/IMPALA/
> Impala+Style+Guide>
> >> doesn't
> >> > mention Python at all.
> >> >
> >> > Thanks, Lars
> >>
>
