[Buildroot] [RFC] Adopt a coding style for Python scripts

Arnout Vandecappelle arnout at mind.be
Sun Apr 23 21:41:35 UTC 2017



On 20-04-17 08:56, Ricardo Martincoski wrote:
> Thomas,
> 
> I have a local patch for the docs for option D, but I will hold it for few more
> days to see if we get more input.
> 
> On Wed, Apr 19, 2017 at 04:34 PM, Thomas De Schampheleire wrote:
> 
>> 2017-04-14 19:14 GMT+02:00 Arnout Vandecappelle <arnout at mind.be>:
>>> On 14-04-17 01:44, Ricardo Martincoski wrote:
> [snip]
>>>> D) adopt the recommendation PEP8 [1] as coding style and the tool pep8 [2] as
>>>>    automatic checker for coding style before submitting patches;
>>>>    It checks for a subset of the recommendation (e.g. file naming is not
>>>>    checked).
>>>
>>>  For me, option D is great. It can also be added to the checkpackage script.
>>>
>>
>> I'm not against this either. However, sometimes I feel that pep8 is
>> being too pedantic. For example, there is a minimum variable name
>> length of 3. However, for regex searches, it is common to use the
>> variable 'm' to hold the match, and then use 'if m' or 'm.group(3)' or
>> whatever, which I feel is perfectly fine.
> 
> It seems more people think like you because pep8 tool does not check for that
> specifically (it's a subset from the recommendation), see my tests at the end.
> 
>> I haven't used pep8 so don't know if it can be tweaked.
> 
> The warnings can be suppressed by type.
> pep8 --ignore E302 1.py
> But AFAIK it can't be configured i.e. to check for 1 or 3 empty lines instead
> of 2. It's "check for 2" or "don't check for empty lines".

 I don't think we have a problem with the 2 empty lines. What I understand from
the previous discussion is that we're OK with changing the Buildroot to standard
to comply with pep8, also if that means adding an extra line between functions.


>> There also exist other tools btw, like flake8, which also check other
>> items than style. They too can be useful, but they too can be 'too
>> much'.
> 
> I haven't used flake8. I installed it after your e-mail.
> I usually use pep8 for style, pyflakes for common errors (i.e. unused import),
> and pyflakes3 for Python 3 compatibility (i.e. print without parenthesis).
> 
> Last year pep8 got renamed to pycodestyle to reduce confusion with PEP8.
> 
> So looking at [4] I had a nice surprise ...   
>  pycodestyle >= 2.0.0, < 2.1.0
>  pyflakes >= 0.8.0, != 1.2.0, != 1.2.1, != 1.2.2, < 1.3.0
>  mccabe >= 0.5.0, < 0.6.0
> If you use flake8 you already use pep8 = pycodestyle!
> 
> The only thing that worries me a bit is this warning from [5]:
> "In many ways, Flake8 is tied to the version of Python on which it runs."
> 
> 
> Do you (or somebody else) know if it can be a concern in our use case?
> 
> 
> Other than that I am OK to adopt PEP8 + flake8 instead of PEP8 + pep8.
> Let's call it option E.
> E) PEP8 + flake8
> Let's see what others think.

 Let's start with documenting PEP8 :-)

> 
> 
> Here is the output for all current Python scripts in Buildroot:
> $ flake8 --statistics $(find support/ -type f | xargs file | grep Python \
>   | sed -e 's,:.*,,g')
> 4     E101 indentation contains mixed spaces and tabs
> 2     E122 continuation line missing indentation or outdented
> 2     E127 continuation line over-indented for visual indent
> 2     E128 continuation line under-indented for visual indent
> 1     E129 visually indented line with same indent as next logical line
> 5     E201 whitespace after '['
> 7     E202 whitespace before ']'
> 3     E203 whitespace before ':'
> 1     E221 multiple spaces before operator
> 1     E225 missing whitespace around operator
> 16    E231 missing whitespace after ','
> 3     E261 at least two spaces before inline comment
> 54    E302 expected 2 blank lines, found 1
> 12    E305 expected 2 blank lines after class or function definition, found 1
> 6     E402 module level import not at top of file
> 41    E501 line too long (80 > 79 characters)
 This one is probably a bit too much to try to fix.
> 6     E502 the backslash is redundant between brackets
> 2     E711 comparison to None should be 'if cond is None:'
> 2     E713 test for membership should be 'not in'
> 15    F401 'checkpackagelib.ConsecutiveEmptyLines' imported but unused
> 1     F821 undefined name 'sys'
> 2     W191 indentation contains tabs
> 2     W391 blank line at end of file
> 3     W601 .has_key() is deprecated, use 'in'

 Otherwise looks like all of them are OK to change.

[snip]
> (my original e-mail used [3], so I incremented the number to avoid confusion)
> [4] http://flake8.pycqa.org/en/latest/faq.html#why-does-flake8-use-ranges-for-its-dependencies
> [5] http://flake8.pycqa.org/en/latest/
> 
> Regards,
> Ricardo
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF



More information about the buildroot mailing list