Re: Run formatting tools

[Jean Abou Samra]

Le 26/09/2020 à 10:26, Jonas Hahnfeld a écrit :

Am Freitag, den 25.09.2020, 21:14 +0200 schrieb Jean Abou Samra:

Le 25/09/2020 à 16:39, Jonas Hahnfeld a écrit :


Am Freitag, den 25.09.2020, 16:01 +0200 schrieb Jean Abou Samra:

Le 25/09/2020 à 15:48, Jonas Hahnfeld a écrit :

Anyway, running black on all Python files gives the following error:

  error: cannot format 
/code/LilyPond/src/scripts/auxiliar/translations-status.py: INTERNAL ERROR: 
Black produced different code on the second pass of the formatter.

   I can't reproduce this. What is your version of Black? (Mine is 19.3b0.) 
What line length did you use?

The latest, 20.8b1. Happens with both the default and -l 78.

Seems to be a known, recently introduced bug in Black.It's easy
to work around it using --fast.

This will just skip the sanity check that future runs keep the
formatting. I don't consider this a valid workaround because we *will*
run the tool again later on.


Actually, subsequent runs do keep the same formatting, but I don't 
understand why.


Jean

Re: Run formatting tools

[Jonas Hahnfeld]

Am Freitag, den 25.09.2020, 21:14 +0200 schrieb Jean Abou Samra:
> Le 25/09/2020 à 16:39, Jonas Hahnfeld a écrit :
> 
> > Am Freitag, den 25.09.2020, 16:01 +0200 schrieb Jean Abou Samra:
> > > Le 25/09/2020 à 15:48, Jonas Hahnfeld a écrit :
> > > > Anyway, running black on all Python files gives the following error:
> > > > 
> > > >  error: cannot format 
> > > > /code/LilyPond/src/scripts/auxiliar/translations-status.py: INTERNAL 
> > > > ERROR: Black produced different code on the second pass of the 
> > > > formatter.
> > > 
> > >   I can't reproduce this. What is your version of Black? (Mine is 
> > > 19.3b0.) What line length did you use?
> > > 
> > > The latest, 20.8b1. Happens with both the default and -l 78.
> 
> Seems to be a known, recently introduced bug in Black.It's easy
> to work around it using --fast.

This will just skip the sanity check that future runs keep the
formatting. I don't consider this a valid workaround because we *will*
run the tool again later on.

Jonas


signature.asc
Description: This is a digitally signed message part

Re: Run formatting tools

[Jean Abou Samra]

Le 25/09/2020 à 16:39, Jonas Hahnfeld a écrit :


Am Freitag, den 25.09.2020, 16:01 +0200 schrieb Jean Abou Samra:

Le 25/09/2020 à 15:48, Jonas Hahnfeld a écrit :

Anyway, running black on all Python files gives the following error:

 error: cannot format 
/code/LilyPond/src/scripts/auxiliar/translations-status.py: INTERNAL ERROR: 
Black produced different code on the second pass of the formatter.

  I can't reproduce this. What is your version of Black? (Mine is 19.3b0.) What 
line length did you use?

The latest, 20.8b1. Happens with both the default and -l 78.


Seems to be a known, recently introduced bug in Black.It's easy
to work around it using --fast.


Also, it seems to change every occurrence of single quotes to double
quotes unless there are already double quotes. My impression is that
single quotes are used more widespread, and the diff is massive at
56 files changed, 10819 insertions(+), 7704 deletions(-)

  https://github.com/psf/black#testimonials
Dusty Phillips, writer:


Black is opinionated so you don't have to be.


That's both the advantage and the drawback.

Personally I don't find the diff to be a huge problem
since the autopep8 reformatting already modified the blame.

If it changes around about every quoted string, things can go wrong
very quickly because of the weird escaping rules. We had a similar
issue with the import reordering and I personally don't want to track
down such issue once more.


To be clear, I was not asking you to take any action.

Best,
Jean

Re: Run formatting tools

[Jonas Hahnfeld]

Am Freitag, den 25.09.2020, 16:01 +0200 schrieb Jean Abou Samra:
> Le 25/09/2020 à 15:48, Jonas Hahnfeld a écrit :
> > Anyway, running black on all Python files gives the following error:
> > 
> > error: cannot format 
> > /code/LilyPond/src/scripts/auxiliar/translations-status.py: INTERNAL ERROR: 
> > Black produced different code on the second pass of the formatter.
>  I can't reproduce this. What is your version of Black? (Mine is 19.3b0.) 
> What line length did you use? 

The latest, 20.8b1. Happens with both the default and -l 78.

> > Also, it seems to change every occurrence of single quotes to double
> > quotes unless there are already double quotes. My impression is that
> > single quotes are used more widespread, and the diff is massive at
> > 56 files changed, 10819 insertions(+), 7704 deletions(-)
>  https://github.com/psf/black#testimonials
> Dusty Phillips, writer:
> 
> > Black is opinionated so you don't have to be.
> > 
> 
> That's both the advantage and the drawback.
> 
> Personally I don't find the diff to be a huge problem
> since the autopep8 reformatting already modified the blame.

If it changes around about every quoted string, things can go wrong
very quickly because of the weird escaping rules. We had a similar
issue with the import reordering and I personally don't want to track
down such issue once more.

Jonas


signature.asc
Description: This is a digitally signed message part

Re: Run formatting tools

[Jean Abou Samra]

Le 25/09/2020 à 15:48, Jonas Hahnfeld a écrit :

You could put this in a pyproject.toml file:

[tool.black]
line-length=78

Then the command would be just ``black .``

Not sure why 78 characters? PEP 8 says 79 characters for code or at
most 72 for doc-strings.


I don't know, just took this figure from Werner.


Anyway, running black on all Python files gives the following error:

 error: cannot format 
/code/LilyPond/src/scripts/auxiliar/translations-status.py: INTERNAL ERROR: 
Black produced different code on the second pass of the formatter.
I can't reproduce this. What is your version of Black? (Mine is 19.3b0.) 
What line length did you use?

Also, it seems to change every occurrence of single quotes to double
quotes unless there are already double quotes. My impression is that
single quotes are used more widespread, and the diff is massive at
56 files changed, 10819 insertions(+), 7704 deletions(-)

https://github.com/psf/black#testimonials

Dusty Phillips, writer 
:


   /Black/is opinionated so you don't have to be.

That's both the advantage and the drawback.

Personally I don't find the diff to be a huge problem
since the autopep8 reformatting already modified the blame.


I don't care enough of the Python formatting for now so I've removed
that commit from the mentioned MR. The obvious caveat is that we must
not run such large-scale formatting if we finally create a stable
branch (not discussed here), or apply the same run for the branch as
well.

Yeah, Python should be tackled in another MR if at all. Thanks
for raising this.

Best,
Jean

Re: Run formatting tools

[Jonas Hahnfeld]

Am Freitag, den 25.09.2020, 14:13 +0200 schrieb Jean Abou Samra:
> Le 25/09/2020 à 12:58, Han-Wen Nienhuys a écrit :
> > On Fri, Sep 25, 2020 at 9:36 AM Jonas Hahnfeld  wrote:
> > > Am Freitag, den 25.09.2020, 09:31 +0200 schrieb Werner LEMBERG:
> > > > > What puzzles me a bit is the magnitude of changes from autopep8 - I
> > > > > thought this tool was run in August? Maybe I'm missing something;
> > > > > FWIW I used the invocation "autopep8 -ia --ignore=E402" as
> > > > > documented in CG.
> > > > > 
> > > > > Thoughts, objections? Should I exclude running autopep8?
> > > > 
> > > > I was told from a Python professional developer that the best tool for
> > > > formatting python code be `black`.  Running
> > > > 
> > > >   black -l 78 ...
> > > > 
> > > > gives indeed very nice formatting (not tested with lilypond stuff,
> > > > though).
> > > 
> > > Hm, I thought we agreed on using autopep8 more than two months ago? I'm
> > > not very familiar with either tool but I'm against switching back and
> > > forth multiple times, that'll just cause churn in the code.
> > > 
> > 
> > IIRC I think the option used was -aa (2x -a = aggressive.)
> 
> It's the opposite, actually. I ran autopep8 wit the default level of
> aggressivity (no -a option), to be sure not to cause any regressions. Since
> then the right fixes to exclude were found and we put something different
> into the CG.
> 
> Black may be a good option. A good reason for using autopep8 back in the
> time was its fix for invalid escape sequences. Which turned out to be
> catastrophic. Since they are all fixed by now, and the interpreter will
> emit warnings during bytecode compilation if any new are added, I don't
> see any reason not to switch to Black. It's much less configurable, with
> just good defaults, and has become a standard tool nowadays as far as I
> can see (at least, it's in the PSF organisation on GitHub).
> 
> You could put this in a pyproject.toml file:
> 
> [tool.black]
> line-length=78
>
> Then the command would be just ``black .``

Not sure why 78 characters? PEP 8 says 79 characters for code or at
most 72 for doc-strings.

Anyway, running black on all Python files gives the following error:

error: cannot format 
/code/LilyPond/src/scripts/auxiliar/translations-status.py: INTERNAL ERROR: 
Black produced different code on the second pass of the formatter.

Also, it seems to change every occurrence of single quotes to double
quotes unless there are already double quotes. My impression is that
single quotes are used more widespread, and the diff is massive at
56 files changed, 10819 insertions(+), 7704 deletions(-)

I don't care enough of the Python formatting for now so I've removed
that commit from the mentioned MR. The obvious caveat is that we must
not run such large-scale formatting if we finally create a stable
branch (not discussed here), or apply the same run for the branch as
well.

Jonas


signature.asc
Description: This is a digitally signed message part

Re: Run formatting tools

[Jean Abou Samra]

Le 25/09/2020 à 12:58, Han-Wen Nienhuys a écrit :




On Fri, Sep 25, 2020 at 9:36 AM Jonas Hahnfeld > wrote:


Am Freitag, den 25.09.2020, 09:31 +0200 schrieb Werner LEMBERG:
> > What puzzles me a bit is the magnitude of changes from
autopep8 - I
> > thought this tool was run in August? Maybe I'm missing something;
> > FWIW I used the invocation "autopep8 -ia --ignore=E402" as
> > documented in CG.
> >
> > Thoughts, objections? Should I exclude running autopep8?
>
> I was told from a Python professional developer that the best
tool for
> formatting python code be `black`.  Running
>
>   black -l 78 ...
>
> gives indeed very nice formatting (not tested with lilypond stuff,
> though).

Hm, I thought we agreed on using autopep8 more than two months
ago? I'm
not very familiar with either tool but I'm against switching back and
forth multiple times, that'll just cause churn in the code.


IIRC I think the option used was -aa (2x -a = aggressive.)


It's the opposite, actually. I ran autopep8 wit the default level of
aggressivity (no -a option), to be sure not to cause any regressions. Since
then the right fixes to exclude were found and we put something different
into the CG.

Black may be a good option. A good reason for using autopep8 back in the
time was its fix for invalid escape sequences. Which turned out to be
catastrophic. Since they are all fixed by now, and the interpreter will
emit warnings during bytecode compilation if any new are added, I don't
see any reason not to switch to Black. It's much less configurable, with
just good defaults, and has become a standard tool nowadays as far as I
can see (at least, it's in the PSF organisation on GitHub).

You could put this in a pyproject.toml file:

[tool.black]
line-length  =  78

Then the command would be just ``black .``

Thanks,
Jean

PS: After a quick look, I indeed prefer Black's reformatting over 
autopep8's.

Re: Run formatting tools

[Han-Wen Nienhuys]

On Fri, Sep 25, 2020 at 9:36 AM Jonas Hahnfeld  wrote:

> Am Freitag, den 25.09.2020, 09:31 +0200 schrieb Werner LEMBERG:
> > > What puzzles me a bit is the magnitude of changes from autopep8 - I
> > > thought this tool was run in August? Maybe I'm missing something;
> > > FWIW I used the invocation "autopep8 -ia --ignore=E402" as
> > > documented in CG.
> > >
> > > Thoughts, objections? Should I exclude running autopep8?
> >
> > I was told from a Python professional developer that the best tool for
> > formatting python code be `black`.  Running
> >
> >   black -l 78 ...
> >
> > gives indeed very nice formatting (not tested with lilypond stuff,
> > though).
>
> Hm, I thought we agreed on using autopep8 more than two months ago? I'm
> not very familiar with either tool but I'm against switching back and
> forth multiple times, that'll just cause churn in the code.
>

IIRC I think the option used was -aa (2x -a = aggressive.)

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Run formatting tools

[Jonas Hahnfeld]

Am Freitag, den 25.09.2020, 09:31 +0200 schrieb Werner LEMBERG:
> > What puzzles me a bit is the magnitude of changes from autopep8 - I
> > thought this tool was run in August? Maybe I'm missing something;
> > FWIW I used the invocation "autopep8 -ia --ignore=E402" as
> > documented in CG.
> > 
> > Thoughts, objections? Should I exclude running autopep8?
> 
> I was told from a Python professional developer that the best tool for
> formatting python code be `black`.  Running
> 
>   black -l 78 ...
> 
> gives indeed very nice formatting (not tested with lilypond stuff,
> though).

Hm, I thought we agreed on using autopep8 more than two months ago? I'm
not very familiar with either tool but I'm against switching back and
forth multiple times, that'll just cause churn in the code.

Jonas


signature.asc
Description: This is a digitally signed message part

Re: Run formatting tools

[Werner LEMBERG]

> What puzzles me a bit is the magnitude of changes from autopep8 - I
> thought this tool was run in August? Maybe I'm missing something;
> FWIW I used the invocation "autopep8 -ia --ignore=E402" as
> documented in CG.
> 
> Thoughts, objections? Should I exclude running autopep8?

I was told from a Python professional developer that the best tool for
formatting python code be `black`.  Running

  black -l 78 ...

gives indeed very nice formatting (not tested with lilypond stuff,
though).


Werner

Run formatting tools

[Jonas Hahnfeld]

Hi all,

with the development slowing down, I'd like to run the formatting tools
on LilyPond's source code (fixcc.py, fixscm.py, autopep8). I've pushed
this as https://gitlab.com/lilypond/lilypond/-/merge_requests/422 and
the changes for fixcc.py and fixscm.py make sense to me.
What puzzles me a bit is the magnitude of changes from autopep8 - I
thought this tool was run in August? Maybe I'm missing something; FWIW
I used the invocation "autopep8 -ia --ignore=E402" as documented in CG.

Thoughts, objections? Should I exclude running autopep8?

Jonas


P.S.: Dan's work on voltas rebases cleanly on the formatting changes;
most files touched in !386 needed no formatting changes and the few
that did only have changes in different parts of the file that git can
handle automatically.


signature.asc
Description: This is a digitally signed message part