[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: musicxml: Use method to get number of Lyric (issue 573230043 by addr
From: |
jonas . hahnfeld |
Subject: |
Re: musicxml: Use method to get number of Lyric (issue 573230043 by address@hidden) |
Date: |
Mon, 11 Nov 2019 07:50:42 -0800 |
Reviewers: Dan Eble,
https://codereview.appspot.com/573230043/diff/559230043/python/musicxml.py
File python/musicxml.py (right):
https://codereview.appspot.com/573230043/diff/559230043/python/musicxml.py#newcode1243
python/musicxml.py:1243: nr = l.get_number()
On 2019/11/11 13:34:38, Dan Eble wrote:
I infer from your change summary that you verified that this is called
at some
point during the regression tests. Is that correct?
Yes, it originally showed up when porting to Python 3 (I'm currently
just dividing up the resulting diff). To be sure I just verified that
adding a sys.exit(1) instead of this line breaks the tests, so I think
this code path is indeed exercised by at least a few regression tests.
Description:
musicxml: Use method to get number of Lyric
Commit edb29ef3a ("Correct an inconsistency in the Lyric class")
renamed the method from get_number() to number(), which is shadowed
by the attribute itself and not used in the current code. Using a
method has the advantage that the code can cast to int() in a single
place which is needed for Python 3. Additionally, the code handles
the case if there is no 'number' attribute (should this ever happen).
Please review this at https://codereview.appspot.com/573230043/
Affected files (+7, -7 lines):
M python/musicxml.py
M scripts/musicxml2ly.py
Index: python/musicxml.py
diff --git a/python/musicxml.py b/python/musicxml.py
index
8a24204145bd0b24f23922111a06a709d33c3f4e..91cff75acfa8d3a57bb77a41c94d3faa6f93c01c
100644
--- a/python/musicxml.py
+++ b/python/musicxml.py
@@ -958,7 +958,7 @@ class Syllabic(Music_xml_node):
class Lyric(Music_xml_node):
- def number(self):
+ def get_number(self):
"""
Return the number attribute(if it exists) of the lyric element.
@@ -966,7 +966,7 @@ class Lyric(Music_xml_node):
@return: The value of the number attribute
"""
if hasattr(self, 'number'):
- return self.number
+ return int(self.number)
else:
return -1
@@ -1240,7 +1240,7 @@ class Musicxml_voice:
self.has_lyrics = len(lyrics) > 0
for l in lyrics:
- nr = l.number
+ nr = l.get_number()
if(nr > 0) and not(nr in self._lyrics):
self._lyrics.append(nr)
Index: scripts/musicxml2ly.py
diff --git a/scripts/musicxml2ly.py b/scripts/musicxml2ly.py
index
9b56ceeb8151ccc0883a89b7c15d061ba74875ff..f0ee2b57fb0d868d043b29c3ab56e41420a98141
100755
--- a/scripts/musicxml2ly.py
+++ b/scripts/musicxml2ly.py
@@ -2129,7 +2129,7 @@ def get_all_lyric_parts_in_voice(voice):
lyrics = elem.get_typed_children(musicxml.Lyric)
if lyrics:
for lyric in lyrics:
- index = lyric.number
+ index = lyric.get_number()
if not index in all_lyric_parts:
all_lyric_parts.append(index)
return all_lyric_parts
@@ -2156,17 +2156,17 @@ def extract_lyrics(voice, lyric_key, lyrics_dict):
def has_lyric_belonging_to_lyric_part(note, lyric_part_id):
lyric_elements = get_lyric_elements(note)
- lyric_numbers = [lyric.number for lyric in lyric_elements]
+ lyric_numbers = [lyric.get_number() for lyric in lyric_elements]
return any([lyric_number == lyric_part_id for lyric_number in
lyric_numbers])
for idx, elem in enumerate(voice._elements):
lyrics = get_lyric_elements(elem)
- lyric_keys = [lyric.number for lyric in lyrics]
+ lyric_keys = [lyric.get_number() for lyric in lyrics]
note_has_lyric_belonging_to_lyric_part = lyric_key in lyric_keys
# Current note has lyric with 'number' matching 'lyric_key'.
if note_has_lyric_belonging_to_lyric_part:
for lyric in lyrics:
- if lyric.number == lyric_key:
+ if lyric.get_number() == lyric_key:
text = musicxml_lyrics_to_text(lyric, None)
result.append(text)
# Note has any lyric.