Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for #133 parsing and indexing for partial/irregular measures #256

Merged
merged 9 commits into from
Jun 7, 2023

Conversation

huispaty
Copy link
Collaborator

@huispaty huispaty commented Jun 5, 2023

Measure object now has 'number' and 'name' property:

  • number : always int starting from 1, representing a running count independent measure regularity/ volta endings
  • name : string, ID of a measure in a given musicxml score file, can be non-number in case of volta endings, irregular measures (i.e., pickup measures in the middle of the piece)

@huispaty huispaty added the bug Something isn't working label Jun 5, 2023
@huispaty huispaty requested review from fosfrancesco and sildater June 5, 2023 10:24
@CarlosCancino-Chacon CarlosCancino-Chacon changed the base branch from main to develop June 5, 2023 10:38
Copy link
Member

@CarlosCancino-Chacon CarlosCancino-Chacon left a comment

Choose a reason for hiding this comment

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

This looks like a nice addition! I have a couple of comments regarding extra commented lines and a formatting of the __str__ representation of the Measure.

partitura/io/importmusicxml.py Outdated Show resolved Hide resolved
partitura/io/importmusicxml.py Outdated Show resolved Hide resolved
@@ -456,13 +456,14 @@ def _parse_parts(document, part_dict):
# shift.applied = True


def _handle_measure(measure_el, position, part, ongoing, doc_order):
def _handle_measure(measure_el, position, part, ongoing, doc_order, measure_counter):
# print('_handle_measure') # NOTE del
"""

Choose a reason for hiding this comment

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

Maybe it would be worthwhile to document the function a little bit, since there is a new parameter (measure_counter). It would be nice to fully document all functions at some point 😉


def __str__(self):
return f"{super().__str__()} number={self.number}"
return f"{super().__str__():16s} number={self.number} name={self.name}"

Choose a reason for hiding this comment

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

Why is the addition of the spaces necessary (the :16s)? Is it because of a specific formatting required by pretty_print?

@manoskary manoskary linked an issue Jun 6, 2023 that may be closed by this pull request
@CarlosCancino-Chacon CarlosCancino-Chacon merged commit 093168a into develop Jun 7, 2023
@CarlosCancino-Chacon CarlosCancino-Chacon deleted the partial_measures branch June 7, 2023 08:37
@huispaty huispaty restored the partial_measures branch June 7, 2023 08:58
@huispaty huispaty deleted the partial_measures branch June 7, 2023 09:00
@manoskary manoskary mentioned this pull request Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Partial Measure Indexing
2 participants