musicxml changes: review

[Han-Wen Nienhuys]

>From 1adfed63faf737d14bcec8772589c0cec6b3e3e8 Mon Sep 17 00:00:00 2001
>From: Reinhold Kainhofer <address@hidden>
>Date: Fri, 14 Sep 2007 22:59:12 +0200
>Subject: [PATCH] MusicXML: Reset measure position on backup elements,
print => sys.stderr.write
>
>-) Replace all relevant (i.e. non-testing outputs; these are still in there
>   in the main() functions for testing) occurrences of print by
>   sys.stderr.write

You could consider  introducing a log function, which may come in
handy if you want implement a --silent flag, or redirect to a file.

>+                    if measure_position < 0:
>+                        # backup went beyond the measure start => reset to 0
>+                        now -= measure_position
>+                        measure_position = 0

Shouldn't this print a warning?

>--- a/python/musicxml.py
>+++ b/python/musicxml.py
>@@ -226,11 +226,13 @@ class Attributes (Measure_element):
>
>         try:
>             mxl = self.get_named_attribute ('time')
>-
>-            beats = mxl.get_maybe_exist_named_child ('beats')
>-            type = mxl.get_maybe_exist_named_child ('beat-type')
>-            return (int (beats.get_text ()),
>-                    int (type.get_text ()))
>+            if mxl:
>+                beats = mxl.get_maybe_exist_named_child ('beats')
>+                type = mxl.get_maybe_exist_named_child ('beat-type')
>+                return (int (beats.get_text ()),
>+                        int (type.get_text ()))
>+            else:
>+                return (4, 4)
>         except KeyError:
>             sys.stderr.write ('error: requested time signature, but time sig 
> unknown')
>             return (4, 4)

is there a more specific way to check for this?  This exception will
also catch and hide programming errors in the methods you are calling.

>Grace notes are normal <note> elements in XML, only with a <grace/>
>child element. I append these notes to a special array of EventChord
>and try to take care of the zero duration of grace notes (which
>come before the real note and thus also create the EventChord with
>duration zero!)

The idea behind the current design of musicxml2ly is that we have two
data structures, one for XML -mirroring the input file- and one for
.ly -mirroring the music expression structure.  musicexp.py should be
independent from musicxml2ly, so it could be used in other
applications too.

Since EventChord doesn't have an extra array, you're breaking the
abstraction. The same goes fro the way you deal with StaffGroups.

If you want to know how the abstraction works, have a look at the
output of

  \displayMusic MUSIC-EXPR

Could you evaluate whether you can do something along those lines? It
would mean that most of hte logic for handling graces should be in
musicxml2ly, rather than in musicxml.py or musicexp.py

If you want to really solve this, it might be that you have to create
a new in-memory intermediate representation of the music, that matches
the capabilities of musicxml. That might or might be too much work,
though, especially since the standard is not completely unambiguous.

>There is still one problem with things like beams or articulations on
>grace notes (they are printed after the whole cord (not inside the
>\grace!), but this is a general problem that musicxml2ly inserts
>slurs, ties, beams, articulations, etc. as separate events not assigned
>to any particular note/EventChord. The code should be changed to
>assign these ornaments/settings directly to the note or the chord they
>apply to. This would (1) get rid of e.g. triple [ with beamed chords,
>(2) make it possible to combine multiple MusicXML articulations into
>single lilypond articulations, (3) apply fingerings and ties to notes
>inside chords rather than the whole chord, and (4) fix the grace notes.

Note that normally this happens too, in

  c4(

the slur start is not attached to the note, but to the chord, ie.

  <c>4 (

and it even works if the slur start comes in somewhere else, ie.

  << c4
     { s4 ( }
     >>

>     d.dots = len (mxl_note.get_typed_children (musicxml.Dot))
>-    d.factor = mxl_note._duration / d.get_length ()
>+    # Grace notes by specification have duration 0, so no time modification
>+    # factor is possible. It even messes up the output with *0/1
>+    if not mxl_note.get_maybe_exist_typed_child (musicxml.Grace):
>+        d.factor = mxl_note._duration / d.get_length ()

note that inside lilypond, duration and time bookkeeping use tuples,

  (normal-time, grace-time)

to deal with this problem. It results in rather complicated logic
sometimes, but perhaps this helps you.

>+class Header:
>+    def __init__ (self):
>+        self.header_fields = {}
>+    def set_field (self, field, value):
>+        self.header_fields[field] = value
>+
>+    def print_ly (self, printer):
>+        printer.dump ("\header {")
>+        printer.newline ()
>+        for (k,v) in self.header_fields.items ():

Maybe you'd like to sort (eg. alfabetically?) the fields too?

>--- a/THANKS
>+++ b/THANKS
>@@ -19,7 +19,7 @@ CONTRIBUTORS
> Rune Zedeler
> Maximilian Albert
> Milan Zamazal
>-
>+Reinhold Kainhofer - musicxml2ly development
>

This can be alfabetical too. If you like, you can put yourself in
Development team, btw. I think your contributions warrant that.

>-) Extract lyrics syllables only for notes, but not for rests. In lilypond
>   a rest can have lyrics attached, in lilypond, the lyrics are assigned to
>        notes only. All lyrics assigned to rests in MusicXML will be lost!

actually, lyrics can be attached to rests too, but you shouldn't use
\lyricsto in that case. If you have the durations of the notes, you
can do

  \lyricmode  { quar4 ter4 eighth8 }

>+class StaffGroup:
>+    def __init__ (self, command = "StaffGroup"):
>+        self.stafftype = command

See my comment above about music representation. If it's too
complicated to really build this as music expressions, this should at
least derive from a SimultaneousMusic.

>             for [v, lyrics] in voices:

this looks a bit strange. I think the normal idiom is

  for v, lyrics in voices

or

  for (v, lyrics) in voices

>The sample files from Recordare contain a piece where the voice on the
>upper staff of a PianoStaff starts with a grace displayed in the second
>staff. So far, this put the whole voice on the second staff. Thus, grace
>notes are now ignored when determining the voice for a staff.

That seems silly? Isn't staff switching for grace notes handled in the
same way as staff switching for normal notes?



-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen