[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Python coding style
Jean Abou Samra
Re: Python coding style
Wed, 1 Jul 2020 18:55:26 +0200
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0
Thanks for your reply.
Le 01/07/2020 à 17:05, Carl Sorensen a écrit :
On Wed, Jul 1, 2020 at 6:03 AM Jean Abou Samra <email@example.com
There is some discussion in !212 about coding style inside our Python
The Contributor's Guide (10.5.1) clearly states that "Python code
use PEP 8". So, I'd like to be sure everyone agrees on the
which are part of applying this PEP:
- Remove spaces before the opening parenthesis in function calls and
e.g., f(x) instead of f (x).
I believe we should definitely follow PEP here. And ideally we should
have a code formatter that does this for us, so we don't rely on
people remembering that PEP is different from GNU C standarads.
Indeed, that was also the subject of a discussion in !190
(https://gitlab.com/lilypond/lilypond/-/merge_requests/190). Since then,
I looked a very bit into available Python linters and found Pylint which
looks great. It could be an ultimate step to use it in CI. (So Han-Wen,
unlike when using Mypy, you don't need to annotate the code you write in
order to check for bare typos.)
Running pylint on scripts/abc2ly.py outputs 1687 lines of tips. Excerpts:
scripts/abc2ly.py:159:0: W1401: Anomalous backslash in string: '\+'.
String constant might be missing an r prefix.
scripts/abc2ly.py:132:6: C0326: Exactly one space required around assignment
scripts/abc2ly.py:137:10: C0326: No space allowed before bracket
def error (msg):
scripts/abc2ly.py:164:0: W0301: Unnecessary semicolon
scripts/abc2ly.py:167:0: W0311: Bad indentation. Found 6 spaces,
expected 8 (bad-indentation)
scripts/abc2ly.py:218:21: C0326: Exactly one space required after comma
def dump_header (outf,hdr):
scripts/abc2ly.py:228:0: C0325: Unnecessary parens after 'if' keyword
scripts/abc2ly.py:650:0: C0325: Unnecessary parens after 'return'
scripts/abc2ly.py:654:15: C0326: No space allowed around bracket
a = re.sub ( '_', ' _ ', a) # _ to ' _ '
scripts/abc2ly.py:655:0: C0301: Line too long (105/100) (line-too-long)
scripts/abc2ly.py:1237:20: W0622: Redefining built-in 'str'
In the end, it says "Your code has been rated at 0.89/10.". (For
disclosure, I'm rather proud that in the branch where my current work on
abc2ly takes place, this bumps to 3.36/10.)
- Use `string` instead of `str` as an identifier − `str` being a
already. Also common is `s`, but the Contributor's Guide also says
that "Variable names should be complete words, rather than
(which is basically concerned with C++, but that particular rule
is, in my
opinion, valid for Python too). In particular, this applies to
I can agree that it is wrong to use 'str', since 'str' is a built in
type. In my opinion, for convertrules.py, we shoud just use 's', not
'string'. Even if we use the complete word 'string', there is no
special meaning in it (it's not a special string, it's just a generic
string). I believe it's analogous to using 'i' for an index in a c++
Why not. I agree that `string` would be somehow superfluous for
convertrules.py at least.
- Use the standard naming conventions, especially CamelCase for class
current style being a mixture of CamelCase and sometimes
First_word_capitalized_with_underscores). Write module-level
UPPERCASE_WITH_UNDERSCORES. Likewise, change things like
class error (Exception): pass
In CamelCase names that contain acronyms, capitalize all letters
(thus, the previous example doesn't read 'MidiConversionError').
I think we should follow the standard here.
There are many specific changes that could be done to improve
(such as replacing `ls = list(something); ls.sort()` with `ls =
etc.), but the above could be made in general commits fixing all
I would be supportive of changes to improve style and readability, but
might question if it's worth the time. However, I'd support anybody
who wants to scratch that itch.
Making things compatible with PEP8 is certainly a good idea.
What I plan is to apply changes that can be done in a semi-automatical
fashion to all Python files (in order to facilitate review). For other
types of improvement, I don't intend to do anything apart from cleaning
up a particular file if I need to touch it.
Also, to be honest, my time clearly isn't as valuable for the project as
that of a LilyPond developer, so I can spend it on small tasks like this
which are within my reach.