[Lilypond-auto] [LilyIssues-auto] [testlilyissues:issues] Re: #4751 Impo

David,

Perhaps somehow we're looking at different versions of musicxml.py?
Just prior to writing this message I did "git pull origin" and checked
out my staging branch, which git says "is up-to-date with
'origin/staging'". I'm looking at the file python/musicxml.py in my
lilypond-git directory. I'll call this the "Philomelos" file.

I'll go through all the changes recorded for issue 4781,
codereview.appspot.com/286480043 and list the relevant lines of this
file. Of the five changed blocks, as I see it, there is only one
questionable issue.

The first change adds a line to the method
Music_xml_node.init. Here is the method in the uploaded version of
the file, and the last line is the one added in issue 4781:

def __init__(self):
    Xml_node.__init__(self)
    self.duration = Rational(0)
    self.start = Rational(0)
    self.converted = False
    self.voice_id = None;

Next, there are changes to the Measure_element.get_voice_id method. The
method in the Philomelos file looks like this:

def get_voice_id(self):
    voice_id = self.get_maybe_exist_named_child('voice')
    if voice_id:
        return voice_id.get_text()
    else:
        return None

To me this looks equivalent to the issue-4681 version of the
file. There are two differences. One is that the Philomelos version
uses the attribute named voice_id where the issue-4681 version uses a
local variable is named voice. The second difference is that the
Philomelos version returns None if voice_id is undefined, the same as
returning voice_id. Note that voice_id is initialized to None as a
result of the first change above.

The third change is in the method Part.interpret, where a dozen or so
lines are added. These lines are in the Philomelos version of the file
exactly where I would expect them. Here they are with a little bit of
context before and after:

            measure_start_moment = now
            measure_position = Rational(0)

        voice_id = None;
        assign_to_next_voice = []
        for n in m.get_all_children ():
            # assign a voice to all measure elements
            if (n.get_name() == 'backup'):
                voice_id = None;

            if isinstance(n, Measure_element):
                if n.get_voice_id ():
                    voice_id = n.get_voice_id ()
                    for i in assign_to_next_voice:
                        i.voice_id = voice_id
                    assign_to_next_voice = []
                else:
                    if voice_id:
                        n.voice_id = voice_id
                    else:
                        assign_to_next_voice.append (n)

            # figured bass has a duration, but applies to the next note
            # and should not change the current measure position!
            if isinstance(n, FiguredBass):
                n._divisions = factor.denominator()
                n._when = now
                n._measure_position = measure_position
                continue

The fourth change is the difficult one. This makes a few changes to
the method Part.extract_voices. I now remember looking at this and
deciding to retain the Philomelos version, assuming the regression
tests would expose an error if there was one, and they did not. I was
being conservative because of the extent of the changes introduced by
the Philomelos project. Perhaps I dropped a ball here, but I don't
understand the code well enough to prove it one way or the
other. Perhaps pkx166h can give an opinion about this.

The fifth change is a change in the base class of the Direction class.
The Philomelos version uses the Music_xml_node class as the base class
and the issue-4781 version uses the Measure_element class. But,
Measure_element derives from Music_xml_node, and Measure_element
doesn't override anything in Music_xml_node, so I believe that these
are equivalent.

John

On Jun 11, 2016, at 4:37 AM, David Kastrup address@hidden wrote:

Seriously? The patch in the tracker consists of 5 changes. Of these changes, 1 is still in staging. 3 changes clearly have been reverted to the previous state. 1 change occurs in such heavily modified surroundings that I cannot tell whether or not its effects have been preserved.

So definitely more than half the changes are missing. I don't understand how this looks different to you.

[issues:#4751] Import Philomelos enhancements to musicxml2ly

Status: Fixed
Labels: Fixed_2_19_44
Created: Sun Jan 24, 2016 02:27 AM UTC by John Gourlay
Last Updated: Fri Jun 10, 2016 08:56 PM UTC
Owner: John Gourlay

Import Philomelos enhancements to musicxml2ly from github.com/Philomelos/lilypond-musicxml2ly-dev. This includes the following:
- New command line option --transpose c TOPITCH.
- Added the sound tempo recognition for midi output.
- Added support for standalone sound elements.
- Added the --shift-meter option to make the music look faster/slower.
- Implemented recognition of stem values "up" and "down".
- Added support for ChordNames transposition.
- Added the command line options --tc / --tabclef [tab|moderntab]
to be able to switch between the two styles of the tab clef.
- Added transpose support for FretDiagrams.
- Allow only "moderntab" and "tab" for tab_clef value.
- Added the command line options --sn / --string-numbers [t|f] to activate
(default) | deactivate the string number stencil.
- Added conversion of <frame> elements to a separate FretBoards voice.
- No longer use the staff tuning option.
- Added --no-stem-direction option to ignore stem directions from MusicXML.
- Added <credit> elements recognition.
- Added page layout handling options.
- Fixed the issue with spaces/brackets in filename.
- Recognize the print-lyric attribute.
- Colored noteheads.
- Allow both <fermata>angled</fermata> and <fermata type="angled"></fermata>
and convert them correctly.
- Color-attribute ignored. Color of notehead and stem can be set with
hexadecimal strings. Color of stem is set even if
conversion_settings.convert_stem_direction is False.
- Include articulate.ly if options.midi == True.
- If a <instrument-sound>-tag is present, attempt to map its value to a
corresponding Lilypond midi-instrument and assign it to the correct staff.
- Automatically include the philomelos tagline in the header.
- Fix numerous bugs.

Sent from sourceforge.net because address@hidden is subscribed to https://sourceforge.net/p/testlilyissues/issues/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/testlilyissues/admin/issues/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

From:	Auto mailings of changes to Lily Issues
Subject:	[Lilypond-auto] [LilyIssues-auto] [testlilyissues:issues] Re: #4751 Import Philomelos enhancements to musicxml2ly
Date:	Sat, 11 Jun 2016 17:36:54 +0000