All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [RFC] Adopt a coding style for Python scripts
@ 2017-04-13 23:44 Ricardo Martincoski
  2017-04-14 17:14 ` Arnout Vandecappelle
  0 siblings, 1 reply; 10+ messages in thread
From: Ricardo Martincoski @ 2017-04-13 23:44 UTC (permalink / raw)
  To: buildroot

Samuel, Maxime, Thomas, All,

First of all, I don't want to start a flame war.

I would like to know what you think about moving from A to D (or C or B) below.

A) keep using the implicit coding style for Python that we use now in Buildroot;

B) adopt a pre-existing Python coding style;
   The advantages of using a pre-existing one are: documenting on the manual
   takes a single sentence; for some coding styles there are automatic checkers
   to help during development/review.
   Of course there are coding style guides others than PEP8. But I don't know
   much about them.
   Do some of you use another coding style for Python? What are its advantages?

C) adopt the recommendation PEP8 [1] as coding style;

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).

What I am *NOT* proposing:
- use Python for every script;
- adapt all current Python scripts as the first step;
- enforce, starting now, 0 warnings from pep8 [2] before merging a patch;
- use an automatic formatter, like autopep8 [3];

My *personal* reasoning for using PEP8 [1] and pep8 [2] (outside Buildroot) is:
"Some people that wrote much more Python code than me already thought and
discussed about this to came up with this recommendation"
"I am lazy so I use the tool so I don't need to read the recommendation too
often"

[1] https://www.python.org/dev/peps/pep-0008/
[2] https://pypi.python.org/pypi/pep8
[3] https://pypi.python.org/pypi/autopep8

Regards,
Ricardo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-13 23:44 [Buildroot] [RFC] Adopt a coding style for Python scripts Ricardo Martincoski
@ 2017-04-14 17:14 ` Arnout Vandecappelle
  2017-04-19 17:00   ` Ricardo Martincoski
  2017-04-19 19:34   ` Thomas De Schampheleire
  0 siblings, 2 replies; 10+ messages in thread
From: Arnout Vandecappelle @ 2017-04-14 17:14 UTC (permalink / raw)
  To: buildroot



On 14-04-17 01:44, Ricardo Martincoski wrote:
> Samuel, Maxime, Thomas, All,
> 
> First of all, I don't want to start a flame war.
> 
> I would like to know what you think about moving from A to D (or C or B) below.
> 
> A) keep using the implicit coding style for Python that we use now in Buildroot;

 Note about this coding style: it is absolutely not formal, and most likely not
followed consistently. It's a combination of the coding style that the various
contributors are used to, with some things inherited from non-Python Buildroot
coding style. I think the single line between functions falls in the latter
category.


> B) adopt a pre-existing Python coding style;
>    The advantages of using a pre-existing one are: documenting on the manual
>    takes a single sentence; for some coding styles there are automatic checkers
>    to help during development/review.
>    Of course there are coding style guides others than PEP8. But I don't know
>    much about them.
>    Do some of you use another coding style for Python? What are its advantages?
> 
> C) adopt the recommendation PEP8 [1] as coding style;
> 
> 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.


> What I am *NOT* proposing:
> - use Python for every script;
> - adapt all current Python scripts as the first step;
> - enforce, starting now, 0 warnings from pep8 [2] before merging a patch;
> - use an automatic formatter, like autopep8 [3];

 Nothing wrong with autopep8 - at least if it is used to generate patches, not
as a pre-commit hook of course.


 The first step is to add the one line to the manual, somewhere in the
Contributing section.

 Regards,
 Arnout

> 
> My *personal* reasoning for using PEP8 [1] and pep8 [2] (outside Buildroot) is:
> "Some people that wrote much more Python code than me already thought and
> discussed about this to came up with this recommendation"
> "I am lazy so I use the tool so I don't need to read the recommendation too
> often"
> 
> [1] https://www.python.org/dev/peps/pep-0008/
> [2] https://pypi.python.org/pypi/pep8
> [3] https://pypi.python.org/pypi/autopep8
> 
> Regards,
> Ricardo
> 
> 
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-14 17:14 ` Arnout Vandecappelle
@ 2017-04-19 17:00   ` Ricardo Martincoski
  2017-04-20  7:17     ` Thomas Petazzoni
  2017-04-19 19:34   ` Thomas De Schampheleire
  1 sibling, 1 reply; 10+ messages in thread
From: Ricardo Martincoski @ 2017-04-19 17:00 UTC (permalink / raw)
  To: buildroot

Arnout,

On Fri, Apr 14, 2017 at 02:14 PM, Arnout Vandecappelle <arnout@mind.be> wrote:

> On 14-04-17 01:44, Ricardo Martincoski wrote:
>> 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 can look into this integration.

>> What I am *NOT* proposing:
>> - use Python for every script;
>> - adapt all current Python scripts as the first step;
>> - enforce, starting now, 0 warnings from pep8 [2] before merging a patch;
>> - use an automatic formatter, like autopep8 [3];
> 
>  Nothing wrong with autopep8 - at least if it is used to generate patches, not
> as a pre-commit hook of course.

Good point. I agree.

>  The first step is to add the one line to the manual, somewhere in the
> Contributing section.

As first iteration I will send a blob to the 'Developer Guide > Coding style'
section. It's not a strong opinion, I just was unable to find a good place in
the Contributing section. In the review I can move that if you or others think
it does not fit well.

Regards,
Ricardo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-14 17:14 ` Arnout Vandecappelle
  2017-04-19 17:00   ` Ricardo Martincoski
@ 2017-04-19 19:34   ` Thomas De Schampheleire
  2017-04-20  6:56     ` Ricardo Martincoski
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas De Schampheleire @ 2017-04-19 19:34 UTC (permalink / raw)
  To: buildroot

2017-04-14 19:14 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
>
>
> On 14-04-17 01:44, Ricardo Martincoski wrote:
>> Samuel, Maxime, Thomas, All,
>>
>> First of all, I don't want to start a flame war.
>>
>> I would like to know what you think about moving from A to D (or C or B) below.
>>
>> A) keep using the implicit coding style for Python that we use now in Buildroot;
>
>  Note about this coding style: it is absolutely not formal, and most likely not
> followed consistently. It's a combination of the coding style that the various
> contributors are used to, with some things inherited from non-Python Buildroot
> coding style. I think the single line between functions falls in the latter
> category.
>
>
>> B) adopt a pre-existing Python coding style;
>>    The advantages of using a pre-existing one are: documenting on the manual
>>    takes a single sentence; for some coding styles there are automatic checkers
>>    to help during development/review.
>>    Of course there are coding style guides others than PEP8. But I don't know
>>    much about them.
>>    Do some of you use another coding style for Python? What are its advantages?
>>
>> C) adopt the recommendation PEP8 [1] as coding style;
>>
>> 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.
I haven't used pep8 so don't know if it can be tweaked.

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'.

/Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-19 19:34   ` Thomas De Schampheleire
@ 2017-04-20  6:56     ` Ricardo Martincoski
  2017-04-23 21:41       ` Arnout Vandecappelle
  0 siblings, 1 reply; 10+ messages in thread
From: Ricardo Martincoski @ 2017-04-20  6:56 UTC (permalink / raw)
  To: buildroot

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@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".

> 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.


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@top of file
41    E501 line too long (80 > 79 characters)
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'
Only the F[0-9]* warnings are added compared to pep8 --statistics.

And here some comparison among the output of individual commands:
-----------------------------
$ cat 1.py
import os
a = 1
print "a"

def function1():
    b = 1
$ pep8 1.py 
1.py:5:1: E302 expected 2 blank lines, found 1

$ pep8 --ignore E302 1.py

$ pycodestyle 1.py 
1.py:5:1: E302 expected 2 blank lines, found 1

$ pycodestyle --ignore E302 1.py

$ pyflakes 1.py 
1.py:1: 'os' imported but unused
1.py:6: local variable 'b' is assigned to but never used

$ pyflakes3 1.py 
1.py:3:9: invalid syntax
print "a"
        ^

$ flake8 1.py 
1.py:1:1: F401 'os' imported but unused
1.py:5:1: E302 expected 2 blank lines, found 1
1.py:6:5: F841 local variable 'b' is assigned to but never used

$ flake8 --ignore E302 1.py 
1.py:1:1: F401 'os' imported but unused
1.py:6:5: F841 local variable 'b' is assigned to but never used
-----------------------------

(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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-19 17:00   ` Ricardo Martincoski
@ 2017-04-20  7:17     ` Thomas Petazzoni
  2017-04-23 21:34       ` Arnout Vandecappelle
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2017-04-20  7:17 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 19 Apr 2017 14:00:53 -0300, Ricardo Martincoski wrote:

> > On 14-04-17 01:44, Ricardo Martincoski wrote:  
> >> 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 can look into this integration.

I have nothing against option (D), but I'm wondering what is the
relation with the checkpackage script. checkpackage checks packages,
written in Kconfig and Make, what is the relation with complying with
the Python PEP8 recommendation?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-20  7:17     ` Thomas Petazzoni
@ 2017-04-23 21:34       ` Arnout Vandecappelle
  2017-04-24  1:25         ` Ricardo Martincoski
  0 siblings, 1 reply; 10+ messages in thread
From: Arnout Vandecappelle @ 2017-04-23 21:34 UTC (permalink / raw)
  To: buildroot



On 20-04-17 09:17, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 19 Apr 2017 14:00:53 -0300, Ricardo Martincoski wrote:
> 
>>> On 14-04-17 01:44, Ricardo Martincoski wrote:  
>>>> 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 can look into this integration.
> 
> I have nothing against option (D), but I'm wondering what is the
> relation with the checkpackage script. checkpackage checks packages,
> written in Kconfig and Make, what is the relation with complying with
> the Python PEP8 recommendation?

 Actually, check-package checks files, not packages. It looks at the filename
and if it is one it recognizes, it runs the appropriate checker on it. So a
checker that applies to files ending with .py could certainly be added.

 Regards,
 Arnout

> 
> Best regards,
> 
> Thomas
> 

-- 
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-20  6:56     ` Ricardo Martincoski
@ 2017-04-23 21:41       ` Arnout Vandecappelle
  2017-04-24  1:28         ` Ricardo Martincoski
  0 siblings, 1 reply; 10+ messages in thread
From: Arnout Vandecappelle @ 2017-04-23 21:41 UTC (permalink / raw)
  To: buildroot



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@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-23 21:34       ` Arnout Vandecappelle
@ 2017-04-24  1:25         ` Ricardo Martincoski
  0 siblings, 0 replies; 10+ messages in thread
From: Ricardo Martincoski @ 2017-04-24  1:25 UTC (permalink / raw)
  To: buildroot

Arnout, Thomas P,

On Sun, Apr 23, 2017 at 06:34 PM, Arnout Vandecappelle wrote:

> On 20-04-17 09:17, Thomas Petazzoni wrote:
>> Hello,
>> 
>> On Wed, 19 Apr 2017 14:00:53 -0300, Ricardo Martincoski wrote:
>> 
>>>> On 14-04-17 01:44, Ricardo Martincoski wrote:  
>>>>> 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 can look into this integration.
>> 
>> I have nothing against option (D), but I'm wondering what is the
>> relation with the checkpackage script. checkpackage checks packages,
>> written in Kconfig and Make, what is the relation with complying with
>> the Python PEP8 recommendation?
> 
>  Actually, check-package checks files, not packages. It looks at the filename
> and if it is one it recognizes, it runs the appropriate checker on it. So a
> checker that applies to files ending with .py could certainly be added.

It is feasible, probably using the library file-magic because not all Python
scripts in the tree use .py extension.

But maybe it is too much, at least for now. 

Before submitting a patch for a package one can use check-package.
Before submitting a patch for a Python script one can use pycodestyle (pep8).

It's not really needed to have a unified script to check both a package and a
Python script.
They are in different directories and are sent in separate patches anyway.

Regards,
Ricardo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [RFC] Adopt a coding style for Python scripts
  2017-04-23 21:41       ` Arnout Vandecappelle
@ 2017-04-24  1:28         ` Ricardo Martincoski
  0 siblings, 0 replies; 10+ messages in thread
From: Ricardo Martincoski @ 2017-04-24  1:28 UTC (permalink / raw)
  To: buildroot

Arnout,

On Sun, Apr 23, 2017 at 06:41 PM, Arnout Vandecappelle wrote:

> 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@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.

Sure. We don't have a problem with 2 empty lines.

Sorry the bad example I gave. What I meant to say is that the tool can be
tweaked but only by toggling on/off which warnings are generated.

>>> 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 :-)

OK. I hope I understand your suggestion correctly.
I will send a patch for option C (PEP8 recommendation, no tools).
Anyway it can be changed during review/apply if needed.

Regards,
Ricardo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-04-24  1:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-13 23:44 [Buildroot] [RFC] Adopt a coding style for Python scripts Ricardo Martincoski
2017-04-14 17:14 ` Arnout Vandecappelle
2017-04-19 17:00   ` Ricardo Martincoski
2017-04-20  7:17     ` Thomas Petazzoni
2017-04-23 21:34       ` Arnout Vandecappelle
2017-04-24  1:25         ` Ricardo Martincoski
2017-04-19 19:34   ` Thomas De Schampheleire
2017-04-20  6:56     ` Ricardo Martincoski
2017-04-23 21:41       ` Arnout Vandecappelle
2017-04-24  1:28         ` Ricardo Martincoski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.