Reviewers: dak, Message: On 2020/02/02 15:44:48, dak wrote: > > However as far as I understand the MusicXML specification, a <note> > must always have either a <pitch>, or <unpitched>, or <rest>. > > Wouldn't percussion notes generally be "unpitched"? What happens for such > notes?
Yes, they are in valid MusicXML. This is handled in initialize_unpitched_event(), see context for python/musicxml.py Description: Fix SyntaxWarning's Individual changes: 1) abc2ly: Fix SyntaxWarning This line still works in Python 3.8, but it throws a warning: > SyntaxWarning: "is" with a literal. Did you mean "=="? 2) musicxml2ly: Remove drummode This code is fundamentally broken: NoteEvent::ly_expression() and NoteEvent::chord_element_ly() miss a '%' to format the string. Instead the code "calls" the string which Python meets with a SyntaxWarning. Trying to hit this code path, musicxml only sets the drum_type attribute in Note::initialize_drum_event() called from Note::to_lily_object(). However as far as I understand the MusicXML specification, a <note> must always have either a <pitch>, or <unpitched>, or <rest>. If I try to construct an (invalid) input with only an instrument name, musicxml2ly breaks at another function, so this is really dead code. Please review this at https://codereview.appspot.com/559440043/ Affected files (+1, -42 lines): M python/musicexp.py M python/musicxml.py M scripts/abc2ly.py M scripts/musicxml2ly.py Index: python/musicexp.py diff --git a/python/musicexp.py b/python/musicexp.py index e6c70bc0d70467125e67036ad83454fc6ebc5442..7019cce2e73fe08fb233ec7cb0ce3b1115ca0081 100644 --- a/python/musicexp.py +++ b/python/musicexp.py @@ -1714,7 +1714,6 @@ class NoteEvent(RhythmicEvent): def __init__ (self): RhythmicEvent.__init__ (self) #self.pitch = None - self.drum_type = None self.cautionary = False self.forced_accidental = False @@ -1723,8 +1722,6 @@ class NoteEvent(RhythmicEvent): if self.pitch: str += self.pitch.lisp_expression () - elif self.drum_type: - str += "'drum-type '%s" % self.drum_type return str @@ -1744,9 +1741,6 @@ class NoteEvent(RhythmicEvent): return res + '%s%s%s' % (self.pitch.ly_expression (), self.pitch_mods(), self.duration.ly_expression ()) - elif self.drum_type: - return res + '%s%s' (self.drum_type, - self.duration.ly_expression ()) def chord_element_ly (self): # obtain all stuff that needs to be printed before the note: @@ -1754,8 +1748,6 @@ class NoteEvent(RhythmicEvent): if self.pitch: return res + '%s%s' % (self.pitch.ly_expression (), self.pitch_mods()) - elif self.drum_type: - return res + '%s%s' (self.drum_type) def print_ly (self, printer): @@ -1769,8 +1761,6 @@ class NoteEvent(RhythmicEvent): if self.pitch: self.pitch.print_ly (printer) printer (self.pitch_mods ()) - else: - printer (self.drum_type) self.duration.print_ly (printer) Index: python/musicxml.py diff --git a/python/musicxml.py b/python/musicxml.py index 704afcaa8fad36153547ba1d96666ede7b3a490d..dffc3a6300f1cb5823fbbdeba4f1dadef54823a9 100644 --- a/python/musicxml.py +++ b/python/musicxml.py @@ -855,18 +855,6 @@ class Note(Measure_element): event.pitch = pitch return event - def initialize_drum_event(self): - event = musicexp.NoteEvent() - drum_type = self.drumtype_dict.get(self.instrument_name) - if drum_type: - event.drum_type = drum_type - else: - self.message( - _("drum %s type unknown, please add to instrument_drumtype_dict") - % n.instrument_name) - event.drum_type = 'acousticsnare' - return event - def to_lily_object(self, convert_stem_directions=True, convert_rest_positions=True): @@ -880,8 +868,6 @@ class Note(Measure_element): event = self.initialize_unpitched_event() elif self.get_maybe_exist_typed_child(Rest): event = self.initialize_rest_event(convert_rest_positions) - elif self.instrument_name: - event = self.initialize_drum_event() else: self.message(_("cannot find suitable event")) Index: scripts/abc2ly.py diff --git a/scripts/abc2ly.py b/scripts/abc2ly.py index c6615c2a62d99e0edc5dca73ec3141ade618ebbf..135f2b590e83ea2c80292c41986f58fec8ec4cff 100644 --- a/scripts/abc2ly.py +++ b/scripts/abc2ly.py @@ -597,7 +597,7 @@ def stuff_append_back(stuff, idx, a): stuff.append (a) else: point = len(stuff[idx])-1 - while stuff[idx][point] is ' ': + while stuff[idx][point] == ' ': point = point - 1 point = point +1 stuff[idx] = stuff[idx][:point] + a + stuff[idx][point:] Index: scripts/musicxml2ly.py diff --git a/scripts/musicxml2ly.py b/scripts/musicxml2ly.py index 5aed38670c1185c9b538ce8ffffa7d4234463100..5267b81f9ce5bf2dd952ebd1e5ee7f990a03eacd 100755 --- a/scripts/musicxml2ly.py +++ b/scripts/musicxml2ly.py @@ -2190,7 +2190,6 @@ def extract_lyrics(voice, lyric_key, lyrics_dict): def musicxml_voice_to_lily_voice(voice): tuplet_events = [] - modes_found = {} lyrics = {} return_value = VoiceData() return_value.voicedata = voice @@ -2380,9 +2379,6 @@ def musicxml_voice_to_lily_voice(voice): # ignore lyrics for notes inside a slur, tie, chord or beam ignore_lyrics = is_tied or is_chord #or is_beamed or inside_slur - if main_event and hasattr(main_event, 'drum_type') and main_event.drum_type: - modes_found['drummode'] = True - ev_chord = voice_builder.last_event_chord(n._when) if not ev_chord: ev_chord = musicexp.ChordEvent() @@ -2623,20 +2619,12 @@ def musicxml_voice_to_lily_voice(voice): seq_music = musicexp.SequentialMusic() - if 'drummode' in list(modes_found.keys()): - ## \key <pitch> barfs in drummode. - ly_voice = [e for e in ly_voice - if not isinstance(e, musicexp.KeySignatureChange)] - seq_music.elements = ly_voice for k in list(lyrics.keys()): return_value.lyrics_dict[k] = musicexp.Lyrics() return_value.lyrics_dict[k].lyrics_syllables = lyrics[k] - if len(modes_found) > 1: - ly.warning(_('cannot simultaneously have more than one mode: %s') % list(modes_found.keys())) - if hasattr(options, 'shift_meter') and options.shift_meter: sd[-1].element = seq_music seq_music = sd[-1] @@ -2649,11 +2637,6 @@ def musicxml_voice_to_lily_voice(voice): seq_music = v return_value.ly_voice = seq_music - for mode in list(modes_found.keys()): - v = musicexp.ModeChangingMusicWrapper() - v.element = seq_music - v.mode = mode - return_value.ly_voice = v # create \figuremode { figured bass elements } if figured_bass_builder.has_relevant_elements: