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

MusicXML export: Insert check against empty "sections" list #77

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

uliska
Copy link
Collaborator

@uliska uliska commented Nov 17, 2016

I don't know what this code actually is about, but adding this
check made a file compile that before caused a traceback because of
trying to pop from an empty list.

@PeterBjuhr Please have a look into this and tell me if I'm curing the right symptoms with that.
Actually I don't have any clue what this sections object is for.

Addresses #76

I don't know what this code actually is about, but adding this
check made a file compile that before caused a traceback because of
trying to pop from an empty list.
@PeterBjuhr
Copy link
Collaborator

This shouldn't happen so there's a risk that the output will be bad anyway. And for that reason I would prefer try/except over if. (Also some would argue that it's more pythonic :)

@uliska
Copy link
Collaborator Author

uliska commented Nov 19, 2016

Can you tell me what that "sections" actually refers to?

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

@PeterBjuhr
Copy link
Collaborator

In short it keeps tracks of different sections of music in the source. It works as a stack with the active section on top. When the section ends it is removed from the stack. And there should never be removed more sections than are added.

@uliska
Copy link
Collaborator Author

uliska commented Nov 19, 2016

Well, it did happen, and after that check it seemed to produce a file.
I didn't have the opportunity to check the results, though.

But do I understand you correctly that as this happened it more likely pointed to an issue elsewhere that I should investigate more closely?

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

@PeterBjuhr
Copy link
Collaborator

Of course I didn't mean to say that it can't happen. But as you say it's an indication that something went wrong somewhere. Therefore to my mind also an exception is more suitable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants