On Wed, Jul 1, 2020 at 10:55 AM Jean Abou Samra <j...@abou-samra.fr> wrote:
> 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 <j...@abou-samra.fr> wrote: > >> Hi everybody, >> >> There is some discussion in !212 about coding style inside our Python >> scripts. >> >> The Contributor's Guide (10.5.1) clearly states that "Python code should >> 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 >> definitions, >> 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: > <snip> pylint shows the problems, but doesn't fix them. I think that it would be much better to have an automatic formatter to fix whitespace errors than to just catch them with pylint. So I would expect the CI to do something like: RegularizeWhitespace(python.file) return(pylint(python.file)) IMO it's a waste of developer brain cells to try to make sure the whitespace rules for *any* coding standard are followed. Tools can (and should) do that. Expecting developers to make variable names follow standards is fine. I have no problem with pylint rejecting a piece of code because we have bad variable names. But that's because it's not appropriate (IMO) to have an automated tool changing variable names. In short, I think pylint is part of the solution (identifying bad python code), but is not a complete solution (ensuring whitespace is compliant with PEP8). Thanks, Carl