Re: musicxml: Use method to get number of Lyric (issue 573230043 by address@hidden)

[jonas . hahnfeld]

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.