Re: midi2ly: Fraction Reduction + 2 minor issues

2017-04-16 Thread Martin Tarenskeen



On Thu, 13 Apr 2017, Christopher Heckman wrote:


I have written a patch to fix this. Inside of the class Duration (line
129), replace the dump function with:


Hi Chris, thanks for trying to improve the midi2ly tool.

I like the idea and really appreciate your efforts, but I tried your patch 
and it doesn't seem to work (yet), at least not on my system. I can't 
convert a midifile after applying your patch. Did anyone try Christophers 
proposal succesfully? (I installed modified version of midi2ly as 
/usr/bin/midi2ly_ on my system)


Traceback (most recent call last):
  File "/usr/bin/midi2ly_", line 1241, in 
main ()
  File "/usr/bin/midi2ly_", line 1238, in main
convert_midi (f, o)
  File "/usr/bin/midi2ly_", line 1050, in convert_midi
s += t.dump (i)
  File "/usr/bin/midi2ly_", line 968, in dump
return dump_track (self.voices, i)
  File "/usr/bin/midi2ly_", line 881, in dump_track
s += '  ' + dump_voice (voice, skip)
  File "/usr/bin/midi2ly_", line 781, in dump_voice
lines[-1] = lines[-1] + dump_chord (ch[1])
  File "/usr/bin/midi2ly_", line 695, in dump_chord
s = s + dump (notes[0])
  File "/usr/bin/midi2ly_", line 684, in dump
return d.dump ()
  File "/usr/bin/midi2ly_", line 304, in dump
s = s + self.duration.dump ()
TypeError: cannot concatenate 'str' and 'NoneType' objects

--

MT

___
bug-lilypond mailing list
bug-lilypond@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-lilypond


Re: midi2ly: Fraction Reduction + 2 minor issues

2017-04-16 Thread Simon Albrecht

Dear fellow Bug Squad members,

unfortunately I can’t access my sourceforge account right now. Could 
anyone please open tracker issues for this (I think three separate ones 
would be in order)?



Am 14.04.2017 um 02:18 schrieb Christopher Heckman:

(1) When midi2ly is run, it will print fractions with large numerators
and denominators,

[…]

I have written a patch to fix this.


[…]

Am 15.04.2017 um 12:10 schrieb Simon Albrecht:

Am 14.04.2017 um 02:18 schrieb Christopher Heckman:


(2) In line 181 of midi2ly.py, the authors talk about how the 7th of a
minor scale should be raised, but this is not notated. The real issue
is that there are two types of minor scale involved, the harmonic
minor (1 2 b3 4 5 b6 7) and the natural minor (1 2 b3 4 5 b6 b7). The
reason that "C# is not put in the key signature of D minor" is that
the minor scale is the natural minor, not the harmonic minor.


I’d rather say that all these musicotheoretical musings don’t belong 
there at all. Instead one could just say:

# induce correct spelling of 7th scale degree in minor keys




(3) At line 677, there's a cryptic comment

# urg, this will barf at meter changes


i.e. an enhancement request to fix this

Best, Simon

___
bug-lilypond mailing list
bug-lilypond@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-lilypond


Re: midi2ly: Fraction Reduction + 2 minor issues

2017-04-16 Thread Simon Albrecht

Hi Chris,

we use to keep communication on-list apart from special cases where 
privacy is required.



Am 16.04.2017 um 10:45 schrieb Christopher Heckman:

On Sat, Apr 15, 2017 at 3:10 AM, Simon Albrecht  wrote:

Hi Chris,

thanks for your analyses and code.


Am 14.04.2017 um 02:18 schrieb Christopher Heckman:

(1) When midi2ly is run, it will print fractions with large numerators
and denominators

I have written a patch to fix this.


Could you provide this as a git-formatted patch, please?
(In case of doubt, see
)

I tried to do it. I'm not 100% sure it was submitted; I haven't used
git or anything like it before.


Well, there’s basically two options for how to proceed. Both require 
that you clone the git repository, add and commit your changes. (All of 
that should be explained in the CG , ‘Basic Git 
usage’ or so).

The first option is to then use
git format-patch HEAD
(or HEAD~1, HEAD~2… if you have made two or more commits)
and send the patch to the bug list, to be appended to a tracker issue.
()

Somewhat preferable is uploading the patch for review on Rietveld, as is 
done for all LilyPond development work. (see 
)


If getting acquainted with Git is too much overhead for you for now, you 
could use the lily-git user interface intended exactly for that. (CG 2.2)



(3) At line 677, there's a cryptic comment

# urg, this will barf at meter changes

Is there a brief explanation of the problem available?

Well, apparently this particular code isn’t prepared to deal with meter
changes in the input. What exactly is unclear about that?

Best, Simon

First question: Is the exact problem known, or did midi2ly crash (or
whatever -- that's another question), and someone said, "Oops, better
not do that"? Or ... did someone recognize that the bug would occur,
and put this here instead of trying to fix it?


I haven’t been involved in coding this at all, but to me the comment 
sounds like the author saw this limitation but didn’t want to fix it 
right away.



Are there meter changes where midi2ly does not crash?


That would be up to the person tackling the limitation to figure out.


Has anyone tried to fix it?


Probably not. If there is not already a tracker issue at 
, definitely not.


We’d certainly welcome your contributions and fixes to these bugs!

Best, Simon

___
bug-lilypond mailing list
bug-lilypond@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-lilypond


Re: midi2ly: Fraction Reduction + 2 minor issues

2017-04-15 Thread Simon Albrecht

Hi Chris,

thanks for your analyses and code.


Am 14.04.2017 um 02:18 schrieb Christopher Heckman:

(1) When midi2ly is run, it will print fractions with large numerators
and denominators

…

I have written a patch to fix this.


Could you provide this as a git-formatted patch, please?
(In case of doubt, see 
)



(2) In line 181 of midi2ly.py, the authors talk about how the 7th of a
minor scale should be raised, but this is not notated. The real issue
is that there are two types of minor scale involved, the harmonic
minor (1 2 b3 4 5 b6 7) and the natural minor (1 2 b3 4 5 b6 b7). The
reason that "C# is not put in the key signature of D minor" is that
the minor scale is the natural minor, not the harmonic minor.


I’d rather say that all these musicotheoretical musings don’t belong 
there at all. Instead one could just say:

# induce correct spelling of 7th scale degree in minor keys


(3) At line 677, there's a cryptic comment

# urg, this will barf at meter changes

Is there a brief explanation of the problem available?


Well, apparently this particular code isn’t prepared to deal with meter 
changes in the input. What exactly is unclear about that?


Best, Simon

___
bug-lilypond mailing list
bug-lilypond@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-lilypond