ancient convert rules

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

ancient convert rules

Jonas Hahnfeld
For https://gitlab.com/lilypond/lilypond/-/issues/6024, I've been
looking at python/convertrules.py and wonder if we really need all
ancient rules starting from version 0.1.9. Right now, a majority of
these won't even apply because Python 3 is much pickier about bad
escape codes in the regular expressions. Example:
    re.sub('\\musicalpitch', '\\pitch', s)
is wrong because \\ only escapes for the string and neither \m nor \p
are correct escapes in a regular expression. For this case, it's easy
to fix with raw strings and I think I was able to resolve most errors
so that all rules are able to run, but I've no way to guarantee that my
edits are correct.

To make the story short: Can we maybe instead drop any rules older than
2.12.0? Its last minor release 2.12.3 is more than 10 years ago.

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

James Lowe-9
On 30/08/2020 14:38, Jonas Hahnfeld wrote:
> To make the story short: Can we maybe instead drop any rules older than
> 2.12.0? Its last minor release 2.12.3 is more than 10 years ago.

I don't know.

While it is more than 10 years ago, I seem to recall a lot of stuff on
Mutopia where there are LP files to use are from much older than 2010.

I think at the very least we should perhaps 'flag' anything (either as a
warning or as an error) for users that might have needed to convert
something that we have removed because of backward compatibility if they
tried to use such a convert-ly file. Rather than look like it has worked
until they try to compile it with the later code and then get an error
and open a bug report (if you see what I mean).

James


Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Benkő Pál
FWIW, I ran a grep on my files, and the oldest version is 2.4.4; there
are also several 2.9, 2.10 and 2.11 ones.  I admit I rarely touch old
files, but sometimes that happens.

James Lowe <[hidden email]> ezt írta (időpont: 2020. aug. 30., V, 16:32):

>
> On 30/08/2020 14:38, Jonas Hahnfeld wrote:
> > To make the story short: Can we maybe instead drop any rules older than
> > 2.12.0? Its last minor release 2.12.3 is more than 10 years ago.
>
> I don't know.
>
> While it is more than 10 years ago, I seem to recall a lot of stuff on
> Mutopia where there are LP files to use are from much older than 2010.
>
> I think at the very least we should perhaps 'flag' anything (either as a
> warning or as an error) for users that might have needed to convert
> something that we have removed because of backward compatibility if they
> tried to use such a convert-ly file. Rather than look like it has worked
> until they try to compile it with the later code and then get an error
> and open a bug report (if you see what I mean).
>
> James
>
>

Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Carl Sorensen
In reply to this post by Jonas Hahnfeld
On Sun, Aug 30, 2020 at 9:02 AM Jonas Hahnfeld <[hidden email]> wrote:

> For https://gitlab.com/lilypond/lilypond/-/issues/6024, I've been
> looking at python/convertrules.py and wonder if we really need all
> ancient rules starting from version 0.1.9. Right now, a majority of
> these won't even apply because Python 3 is much pickier about bad
> escape codes in the regular expressions. Example:
>     re.sub('\\musicalpitch', '\\pitch', s)
> is wrong because \\ only escapes for the string and neither \m nor \p
> are correct escapes in a regular expression. For this case, it's easy
> to fix with raw strings and I think I was able to resolve most errors
> so that all rules are able to run, but I've no way to guarantee that my
> edits are correct.
>
> To make the story short: Can we maybe instead drop any rules older than
> 2.12.0? Its last minor release 2.12.3 is more than 10 years ago.
>

What if we kept a legacy convert.ly that went from the origin to, say
2.12.0 that was still Python 2 based?  And then had the new, Python 3-based
convert.ly start from 2.12.0?

This would provide a way for anybody who needed to get old source code up
to the current standard to do so, and would eliminate the burden of
converting the whole file to Python 3.

Thanks,

Carl
Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Jean ABOU SAMRA
In reply to this post by Jonas Hahnfeld
Hi Jonas,

Le 30/08/2020 à 15:38, Jonas Hahnfeld a écrit :

> For https://gitlab.com/lilypond/lilypond/-/issues/6024, I've been
> looking at python/convertrules.py and wonder if we really need all
> ancient rules starting from version 0.1.9. Right now, a majority of
> these won't even apply because Python 3 is much pickier about bad
> escape codes in the regular expressions. Example:
>      re.sub('\\musicalpitch', '\\pitch', s)
> is wrong because \\ only escapes for the string and neither \m nor \p
> are correct escapes in a regular expression. For this case, it's easy
> to fix with raw strings and I think I was able to resolve most errors
> so that all rules are able to run, but I've no way to guarantee that my
> edits are correct.

Ah, we were doing overlapping work, thanks for letting us know! I'll let
you go ahead.

> On 30/08/2020 14:38, Jonas Hahnfeld wrote:
>> To make the story short: Can we maybe instead drop any rules older than
>> 2.12.0? Its last minor release 2.12.3 is more than 10 years ago.
>
> I don't know.
>
> While it is more than 10 years ago, I seem to recall a lot of stuff on
> Mutopia where there are LP files to use are from much older than 2010.

A quick searchin Mutopia's repository reveals that the oldest \version
statement
there is 2.4.0. That release dates back to 2004. I think it would be
fine to drop
rules prior to that version, or maybe 2.0.

> I think at the very least we should perhaps 'flag' anything (either as
> a warning or as an error) for users that might have needed to convert
> something that we have removed because of backward compatibility if
> they tried to use such a convert-ly file. Rather than look like it has
> worked until they try to compile it with the later code and then get
> an error and open a bug report (if you see what I mean).

It should be made to error out when conversion from a version older that
2.4.0 or
whatever is attempted, I guess. Does that address your concern?

Best,
Jean

Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

David Kastrup
In reply to this post by Jonas Hahnfeld
Jonas Hahnfeld <[hidden email]> writes:

> For https://gitlab.com/lilypond/lilypond/-/issues/6024, I've been
> looking at python/convertrules.py and wonder if we really need all
> ancient rules starting from version 0.1.9. Right now, a majority of
> these won't even apply because Python 3 is much pickier about bad
> escape codes in the regular expressions. Example:
>     re.sub('\\musicalpitch', '\\pitch', s)
> is wrong because \\ only escapes for the string and neither \m nor \p
> are correct escapes in a regular expression. For this case, it's easy
> to fix with raw strings and I think I was able to resolve most errors
> so that all rules are able to run, but I've no way to guarantee that my
> edits are correct.
>
> To make the story short: Can we maybe instead drop any rules older than
> 2.12.0? Its last minor release 2.12.3 is more than 10 years ago.

Music tends to stick around really really long once entered.  Admittedly
old convert-ly rules tended to be a lot less thorough than what we tend
to do now.

Doing

dak@lola:/usr/local/tmp/The-Mutopia-Project$ git grep -h '\\version'|sort -u|less

gives me something starting with

\version "1.9.8"
\version "2.10"
\version "2.10"
  \version "2.10.0"
\version "2.10.0"
\version "2.10.0"
\version "2.10.10"
%\version "2.10.10"
\version "2.10.10"
\version "2.10.12"
\version "2.10.14"
\version "2.10.14"

and also containing

\version "2.2.0"
\version "2.4.0"
\version "2.4.1"
\version "2.4.2"
\version "2.4.2"
\version "2.4.6"
\version "2.5.21"
\version "2.6.0"
\version "2.6.0"
\version "2.6.0"
\version "2.6.2"
\version "2.6.3"
\version "2.6.3"
\version "2.6.3"  % necessary for upgrading to future LilyPond versions.
 \version "2.6.4"
\version "2.6.4"
\version "2.6.4"

and so on.  Stuff like that will not likely convert nicely, but at least
having a start seems like common courtesy.

--
David Kastrup

Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Joram Noeck
In reply to this post by Jonas Hahnfeld
Hi Jonas,

some comments to that:

1. I (hopefully) fixed all the escaping issues like #6024 in my merge
request: https://gitlab.com/lilypond/lilypond/-/merge_requests/363
The python re syntax is clear, it just requires the right amout of
backslashes or r-prefixes;

  re.sub(r'\\command', r'\command', s)  or
  re.sub('\\\\command', '\\command', s)

2. pylint spots the clearly wrong ones (e.g. \m) but not the valid ones
(e.g. \n in \numbers or \t in \times). I would gladly compare your fixes
to mine.

3. We might want to cleanup convertrules.py. And I doubt that all
conversions work flawlessly for ancient versions all the way to 2.21.
But IMHO this issue is not the right excuse to drop possibly working
functionality.

4. Mutopia uses very old versions. There was an effort in 2014 - 2015 to
update all Mutopia scores to a version > 2.0.0. So version 0 or 1 should
not be used any more (one exception). But 2.2.0 is also very old.



Details for Mutopia:

Update Mutopia pieces with LilyPond versions < 2.0
https://github.com/MutopiaProject/MutopiaProject/milestone/5

Apparently, one work (KV487) is missing from that effort.
2.16 and 2.18 are the most frequent versions in Mutopia:

    766 \version "2.16.0"
    736 \version "2.18.0"
    553 \version "2.18.2"
    519 \version "2.16.1"
    300 \version "2.14.2"
    186 \version "2.10.33"
    184 \version "2.12.2"
    176 \version "2.10.3"
    156 \version "2.6.0"    !!!

Besides KV487 with version 1.9.8, the oldest versions are (with number
of occurances):

      1 \version "2.2.0"
     41 \version "2.4.0"
      1 \version "2.4.1"
      7 \version "2.4.2"
      1 \version "2.4.6"
      4 \version "2.5.21"
    156 \version "2.6.0"
     12 \version "2.6.2"
      9 \version "2.6.3"
     51 \version "2.6.4"
      4 \version "2.6.4.3"
      5 \version "2.6.5"
      1 \version "2.7.13"
      1 \version "2.7.24"
     37 \version "2.7.29"
      1 \version "2.7.30"
      3 \version "2.7.38"
      1 \version "2.7.39"
     58 \version "2.7.40"
     46 \version "2.8.0"
     23 \version "2.8.1"
      5 \version "2.8.2"
      1 \version "2.8.3"
     11 \version "2.8.4"
     58 \version "2.8.5"
      2 \version "2.8.6"
      2 \version "2.8.7"
      4 \version "2.8.8"
      1 \version "2.9.16"
      1 \version "2.9.18"
      4 \version "2.9.2"
      9 \version "2.9.9"


tl;dr: I would not drop conversion rules unless we know that they fail.

Cheers,
Joram

Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Joram Noeck

> The python re syntax is clear, it just requires the right amout of
> backslashes or r-prefixes

I definitely overlooked how often there are *too many* backslashes in
the code. So there are a lot more changes requried than what is in merge
request 363:

https://gitlab.com/lilypond/lilypond/-/merge_requests/363

Cheers,
Joram

Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Jean ABOU SAMRA
In reply to this post by David Kastrup
Greetings everybody,

Le 30/08/2020 à 21:06, David Kastrup a écrit :

> Jonas Hahnfeld <[hidden email]> writes:
>
>> For https://gitlab.com/lilypond/lilypond/-/issues/6024, I've been
>> looking at python/convertrules.py and wonder if we really need all
>> ancient rules starting from version 0.1.9. Right now, a majority of
>> these won't even apply because Python 3 is much pickier about bad
>> escape codes in the regular expressions. Example:
>>      re.sub('\\musicalpitch', '\\pitch', s)
>> is wrong because \\ only escapes for the string and neither \m nor \p
>> are correct escapes in a regular expression. For this case, it's easy
>> to fix with raw strings and I think I was able to resolve most errors
>> so that all rules are able to run, but I've no way to guarantee that my
>> edits are correct.
>>
>> To make the story short: Can we maybe instead drop any rules older than
>> 2.12.0? Its last minor release 2.12.3 is more than 10 years ago.
> Music tends to stick around really really long once entered.  Admittedly
> old convert-ly rules tended to be a lot less thorough than what we tend
> to do now.
>
> Doing
>
> dak@lola:/usr/local/tmp/The-Mutopia-Project$ git grep -h '\\version'|sort -u|less
>
> gives me something starting with
>
> \version "1.9.8"

For the record, I finally figured out why we had different results: I
had been
searching only the ftp/ directory. Naturally, what you did is more accurate.

> What if we kept a legacy convert.ly that went from the origin to, say
> 2.12.0 that was still Python 2 based?  And then had the new, Python 3-based
> convert.ly start from 2.12.0?
>
> This would provide a way for anybody who needed to get old source code up
> to the current standard to do so, and would eliminate the burden of
> converting the whole file to Python 3.
>
> Thanks,
>
> Carl

Maybe the solution is just to mention somewhere in the documentation of
convert-ly that if the user wants to upgrade from older that 2.12 or
whatever,
they can use the convert-ly from LilyPond 2.18?


> I definitely overlooked how often there are*too many*  backslashes in
> the code. So there are a lot more changes requried than what is in merge
> request 363:

Also, you fixed invalid escape sequences -- thanks for that! --, but
pylint can't
spot cases where there aren't many actual backslash characters in the
resulting string
to escape backslashes in the regular expression (the second level of
escaping), like in Jonas'
original example. This is why it seems all ancient rules would require a
decent
amount of work to be made to work with Python 3, if I understand correctly.

Cheers,
Jean

Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Jonas Hahnfeld
In reply to this post by Joram Noeck
Am Sonntag, den 30.08.2020, 21:36 +0200 schrieb Joram Noeck:

> Hi Jonas,
>
> some comments to that:
>
> 1. I (hopefully) fixed all the escaping issues like #6024 in my merge
> request: https://gitlab.com/lilypond/lilypond/-/merge_requests/363
>
> The python re syntax is clear, it just requires the right amout of
> backslashes or r-prefixes;
>
>   re.sub(r'\\command', r'\command', s)  or
>   re.sub('\\\\command', '\\command', s)
You need two backslashes in the replacement string, or you get
   re.error: bad escape \c at position 0

re.sub(r'\\commandA', r'\\commandB', r'\command')
works correctly.

> I definitely overlooked how often there are *too many* backslashes in
> the code. So there are a lot more changes requried than what is in merge
> request 363:

There is no such thing as too many backslashes due to how re processes
the expressions and replacements, and there are many missing.


> tl;dr: I would not drop conversion rules unless we know that they fail.

All right, this means we need to fix all rules then.

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Jonas Hahnfeld
In reply to this post by Jean ABOU SAMRA
Am Sonntag, den 30.08.2020, 22:12 +0200 schrieb Jean Abou Samra:

> > What if we kept a legacy convert.ly that went from the origin to, say
> > 2.12.0 that was still Python 2 based?  And then had the new, Python 3-based
> > convert.ly start from 2.12.0?
> >
> > This would provide a way for anybody who needed to get old source code up
> > to the current standard to do so, and would eliminate the burden of
> > converting the whole file to Python 3.
> >
> > Thanks,
> >
> > Carl
>
> Maybe the solution is just to mention somewhere in the documentation of
> convert-ly that if the user wants to upgrade from older that 2.12 or
> whatever, they can use the convert-ly from LilyPond 2.18?
If we want to maintain all old rules, I think it's better to have them
stay one script. Python 2 will hopefully go away at some point, if the
rules are needed they should be fixed.


> > I definitely overlooked how often there are*too many*  backslashes in
> > the code. So there are a lot more changes requried than what is in merge
> > request 363:
>
> Also, you fixed invalid escape sequences -- thanks for that! --, but
> pylint can't
> spot cases where there aren't many actual backslash characters in the
> resulting string
> to escape backslashes in the regular expression (the second level of
> escaping), like in Jonas'
> original example. This is why it seems all ancient rules would require a
> decent
> amount of work to be made to work with Python 3, if I understand correctly.
I think it's not that bad, I went through 24 instances of this.
What's giving me more headaches right now is the rule for 2.5.13 to
remove \encoding. As far as I understand, it tries to re-encode the
whole file which won't work easily because the content is already read
in UTF-8 mode. As far as I understand, \encoding "latin1" basically
meant binary mode in LilyPond? I guess I have to look this up in git.

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Jonas Hahnfeld
Am Sonntag, den 30.08.2020, 22:37 +0200 schrieb Jonas Hahnfeld:

> Am Sonntag, den 30.08.2020, 22:12 +0200 schrieb Jean Abou Samra:
> > > What if we kept a legacy convert.ly that went from the origin to, say
> > > 2.12.0 that was still Python 2 based?  And then had the new, Python 3-based
> > > convert.ly start from 2.12.0?
> > >
> > > This would provide a way for anybody who needed to get old source code up
> > > to the current standard to do so, and would eliminate the burden of
> > > converting the whole file to Python 3.
> > >
> > > Thanks,
> > >
> > > Carl
> >
> > Maybe the solution is just to mention somewhere in the documentation of
> > convert-ly that if the user wants to upgrade from older that 2.12 or
> > whatever, they can use the convert-ly from LilyPond 2.18?
>
> If we want to maintain all old rules, I think it's better to have them
> stay one script. Python 2 will hopefully go away at some point, if the
> rules are needed they should be fixed.
>
>
> > > I definitely overlooked how often there are*too many*  backslashes in
> > > the code. So there are a lot more changes requried than what is in merge
> > > request 363:
> >
> > Also, you fixed invalid escape sequences -- thanks for that! --, but
> > pylint can't
> > spot cases where there aren't many actual backslash characters in the
> > resulting string
> > to escape backslashes in the regular expression (the second level of
> > escaping), like in Jonas'
> > original example. This is why it seems all ancient rules would require a
> > decent
> > amount of work to be made to work with Python 3, if I understand correctly.
>
> I think it's not that bad, I went through 24 instances of this.
> What's giving me more headaches right now is the rule for 2.5.13 to
> remove \encoding. As far as I understand, it tries to re-encode the
> whole file which won't work easily because the content is already read
> in UTF-8 mode. As far as I understand, \encoding "latin1" basically
> meant binary mode in LilyPond? I guess I have to look this up in git.
I think I found a solution that works equally well as it did when
running convert-ly with Python 2. Please see the second commit of
https://gitlab.com/lilypond/lilypond/-/merge_requests/365 for what I
tested.

Jonas

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ancient convert rules

Benkő Pál
Hi Jonas,

> I think I found a solution that works equally well as it did when
> running convert-ly with Python 2. Please see the second commit of
> https://gitlab.com/lilypond/lilypond/-/merge_requests/365 for what I
> tested.

I tested this branch (my first try with GitLab, I hope I got it right)
with some old examples of mine, and your branch at least creates
something usable, as opposed to current master, which just errors out.

thanks,
Pal