-
Notifications
You must be signed in to change notification settings - Fork 401
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
Store "numbered ending" within barline #1686
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi OmreeG -- thanks for the first contribution to the project. I have some concerns about the code, but more about the general concept here. What does this solve that isn't better solved by calling .getContextByClass(spanner.RepeatBracket).number
on the measure object? This way if the RepeatBracket's number is changed then the information propagates to all other sources.
Since it's a niche use case that hasn't come up for discussion before and there is an easy way to solve it, I'm inclined to reject the PR (But so grateful for the contribution!)
def setNumberedEnding(self, value): | ||
self.numberedEnding = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Python we don't generally use setters for things that can be set directly.
@@ -129,6 +129,8 @@ class Barline(base.Music21Object): | |||
classSortOrder = -5 | |||
|
|||
equalityAttributes = ('type', 'pause', 'location') | |||
|
|||
numberedEnding = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All new attributes need to be typed. numberedEnding: int | None = None
@@ -4825,6 +4825,7 @@ def xmlBarline(self, mxBarline): | |||
# barline objects also store ending objects, that mark begin | |||
# and end of repeat bracket designations | |||
mxEndingObj = mxBarline.find('ending') | |||
barline.setNumberedEnding(mxEndingObj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What type of variable is this storing? Is it a str? int?
When parsing musicxml files which contain numbered endings, it would be nice to be able to access them within the "Measure" object.
I believe one reasonable way to do this would be to store the numbered ending in the Barline
object, be it a "Repeat" or not. Then it is easily accessed by the "numberedEnding" field.