Will you use autopep8? If so, how will you check that it doesn't break something on an infrequently-used codepath?
On Tue, Jan 31, 2017 at 11:12 AM, Michael Brown <[email protected]> wrote: > I agree. > > On Tue, Jan 31, 2017 at 10:44 AM, Lars Volker <[email protected]> wrote: >> 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 >>> >> >>>
