Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 70 additions & 36 deletions ly/musicxml/lymus2musxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,25 +91,34 @@ def __init__(self):
self.phrslurnr = 0
self.mark = False
self.pickup = False
self.handlers = {}
self.set_command_handlers()

def get_handler(self, category, element):
"""Return a handler function of a given category, for a given element,
retrieved from the self.handlers dictionary.
For example: self.get_handler['Command']['\\rest']
If a handler for the given element is not defined, return None."""
return self.handlers[category].get(element, None)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any plans to expand this to more categories besides commands?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, once I'm more familiar with it there will probably more of that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are of course other examples of multiple elif statements. But I'm not yet convinced that this is helpful as a general restructuring. The goal should be to make it easier to add new functionality and to maintain the existing code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked again yet at the code of this PR concretely, but I came across the issue when looking in other corners. I'm not convinced that my suggestion is the best implementation because it may seem to add some level of complexity that is contrary to the goal of making maintenance and development easier. But I'm now even more convinced that it would be good to have a dictionary based infrastructure for a number of things to replace the elif chains.

One example is the End() function that provides endings for a number of different elements.
Another example I came across is Set(). I would like to start supporting specific context properties. This would also require handlers for each supported property.

Somehow I have the feeling that it would be the most natural approach to not use the token as the dictionary key but the element's class, and then instantiate an object of a corresponding class, which would handle the request. Such a could be maintained in its own file, so there's less penalty for implementing lots of special cases (e.g. context properties, element types to be ended ...). Maybe it would even make sense to make this subclasses of the classes in ly.music.items?

However, this is more a random musing than a concrete suggestion. Maybe this won't work because there is no really common interface (i.e. does this need arguments or not, how many, etc).

OTOH I'm thinking if this couldn't be a way to provide an opportunity for people to support their \userCommands. A base path could be passed as an argument, and if there is a certain file it will be included ... No idea if that would work, but it could prove very helpful.


def parse_text(self, ly_text, filename=None):
"""Parse the LilyPond source specified as text.

If you specify a filename, it can be used to resolve \\include commands
correctly.

"""
doc = ly.document.Document(ly_text)
doc.filename = filename
self.parse_document(doc)

def parse_document(self, ly_doc, relative_first_pitch_absolute=False):
"""Parse the LilyPond source specified as a ly.document document.

If relative_first_pitch_absolute is set to True, the first pitch in a
\relative expression without startpitch is considered to be absolute
(LilyPond 2.18+ behaviour).

"""
# The document is copied and the copy is converted to absolute mode to
# facilitate the export. The original document is unchanged.
Expand Down Expand Up @@ -479,46 +488,71 @@ def Set(self, cont_set):
elif cont_set.context() in group_contexts:
self.mediator.set_by_property(cont_set.property(), val, group=True)

def Command(self, command):
r""" \bar, \rest etc """
excls = ['\\major', '\\minor', '\\dorian', '\\bar']
if command.token == '\\rest':
self.mediator.note2rest()
elif command.token == '\\numericTimeSignature':
self.numericTime = True
elif command.token == '\\defaultTimeSignature':
def set_command_handlers(self):
"""Define handler functions for Command elements."""

# local functions
def breathe():
art = type('',(object,),{"token": "\\breathe"})()
self.Articulation(art)

def default():
if self.tupl_span:
self.mediator.unset_tuplspan_dur()
self.tupl_span = False
elif self.mark:
self.mark = False

def default_time_signature():
self.numericTime = False
elif command.token.find('voice') == 1:
self.mediator.set_voicenr(command.token[1:], piano=self.piano_staff)
elif command.token == '\\glissando':

def glissando():
try:
self.mediator.new_gliss(self.override_dict["Glissando.style"])
except KeyError:
self.mediator.new_gliss()
elif command.token == '\\startTrillSpan':
self.mediator.new_trill_spanner()
elif command.token == '\\stopTrillSpan':
self.mediator.new_trill_spanner("stop")
elif command.token == '\\ottava':
self.ottava = True
elif command.token == '\\mark':

def mark():
self.mark = True
self.mediator.new_mark()
elif command.token == '\\breathe':
art = type('',(object,),{"token": "\\breathe"})()
self.Articulation(art)
elif command.token == '\\stemUp' or command.token == '\\stemDown' or command.token == '\\stemNeutral':

def numeric_time_signature():
self.numericTime = True

def ottava():
self.ottava = True

handlers = {
'\\break': self.mediator.add_break,
'\\breathe': breathe,
'\\compressFullBarRests': self.mediator.set_mult_rest,
'\\default': default,
'\\defaultTimeSignature': default_time_signature,
'\\mark': mark,
'\\numericTimeSignature': numeric_time_signature,
'\\glissando': glissando,
'\\ottava': ottava,
'\\rest': self.mediator.note2rest,
'\\startTrillSpan': self.mediator.new_trill_spanner,
'\\stopTrillSpan': lambda: self.mediator.new_trill_spanner("stop"),
}
self.handlers['Command'] = handlers

def Command(self, command):
r""" \bar, \rest etc """
excls = ['\\major', '\\minor', '\\dorian', '\\bar']

# Retrieve and call generic, argument-less handlers
handler = self.get_handler('Command', command.token)
if handler:
handler()
return

# Handle special cases or issue a warning
if command.token.find('voice') == 1:
self.mediator.set_voicenr(command.token[1:], piano=self.piano_staff)
elif command.token in ['\\stemUp', '\\stemDown', '\\stemNeutral']:
self.mediator.stem_direction(command.token)
elif command.token == '\\default':
if self.tupl_span:
self.mediator.unset_tuplspan_dur()
self.tupl_span = False
elif self.mark:
self.mark = False
elif command.token == '\\compressFullBarRests':
self.mediator.set_mult_rest()
elif command.token == '\\break':
self.mediator.add_break()
else:
if command.token not in excls:
print("Unknown command:", command.token)
Expand Down