[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Python coding style

From: Jean Abou Samra
Subject: Re: Python coding style
Date: Wed, 1 Jul 2020 18:55:26 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0

Hi Carl,

Thanks for your reply.

Le 01/07/2020 à 17:05, Carl Sorensen a écrit :
Hi Jean,

On Wed, Jul 1, 2020 at 6:03 AM Jean Abou Samra < <>> wrote:

    Hi everybody,

    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
    following points
    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 ( 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/ outputs 1687 lines of tips. Excerpts:

scripts/ W1401: Anomalous backslash in string: '\+'. String constant might be missing an r prefix. (anomalous-backslash-in-string)
scripts/ C0326: Exactly one space required around assignment
      ^ (bad-whitespace)
scripts/ C0326: No space allowed before bracket
def error (msg):
          ^ (bad-whitespace)
scripts/ W0301: Unnecessary semicolon (unnecessary-semicolon) scripts/ W0311: Bad indentation. Found 6 spaces, expected 8 (bad-indentation)
scripts/ C0326: Exactly one space required after comma
def dump_header (outf,hdr):
                     ^ (bad-whitespace)
scripts/ C0325: Unnecessary parens after 'if' keyword (superfluous-parens) scripts/ C0325: Unnecessary parens after 'return' keyword (superfluous-parens)
scripts/ C0326: No space allowed around bracket
    a = re.sub ( '_', ' _ ', a)        # _ to ' _ '
               ^ (bad-whitespace)
scripts/ C0301: Line too long (105/100) (line-too-long)
scripts/ W0622: Redefining built-in 'str' (redefined-builtin)

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
    built-in type
    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, 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++ loop.

Why not. I agree that `string` would be somehow superfluous for at least.

    - Use the standard naming conventions, especially CamelCase for class
    names (the
    current style being a mixture of CamelCase and sometimes
    First_word_capitalized_with_underscores). Write module-level
    constants in
    UPPERCASE_WITH_UNDERSCORES. Likewise, change things like
       class error (Exception): pass
       class MIDIConversionError(Exception):

    In CamelCase names that contain acronyms, capitalize all letters
    of the
    (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
    style and
    (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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]