Skip to content

Conversation

@igneus
Copy link
Collaborator

@igneus igneus commented Aug 10, 2024

This is an attempt to fix #153

As there were major differences between the expected and actual XML output, I regenerated all the XML examples with current python-ly.

@igneus

This comment was marked as outdated.

igneus added 2 commits August 10, 2024 14:35
The parser requires either a unicode string without encoding declaration
in the XML document or bytes input with encoding declaration.
@igneus igneus marked this pull request as ready for review August 10, 2024 12:45
@igneus
Copy link
Collaborator Author

igneus commented Aug 10, 2024

Tests pass now.

</note>
</measure>
<measure number="2">
<print new-system="yes" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

✔️ correct MusicXML equivalent of lily \break

<type>quarter</type>
</note>
</measure>
<measure number="3" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems to be correct - break.ly only has musical content for two measures.

</direction-type>
</direction>
</measure>
<measure number="2">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

✔️ correct - dynamics.ly doesn't specify \time, so the default 4/4 is used

<line>2</line>
</clef>
</attributes>
<direction placement="above">
Copy link
Collaborator Author

@igneus igneus Aug 10, 2024

Choose a reason for hiding this comment

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

❌ the markup does belong here, it shouldn't be removed

<direction placement="above">
<direction-type>
<words>poco più forte </words>
<words>intenso poco più forte </words>
Copy link
Collaborator Author

@igneus igneus Aug 10, 2024

Choose a reason for hiding this comment

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

❌ incorrect merge of two markups - only "poco più forte" belongs here, "intenso" should be in measure 2

</identification>
<part-list>
<part-group number="1" type="start">
<part-group type="start" number="1">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

✔️ mere attribute order change

</identification>
<part-list>
<part-group number="1" type="start">
<part-group type="start" number="1">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

✔️ mere attribute order change

<type>eighth</type>
</note>
</measure>
<measure number="1">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

❌ This is definitely incorrect. The original content was correct (or at least renders in the desired way in MuseScore).

<direction-type>
<rehearsal>A</rehearsal>
</direction-type>
</direction>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't tell for sure if the changes to this file are correct, but the result renders in MuseScore almost the same mark.ly renders in LilyPond. Just a trailing measure with a rest is added.

@igneus
Copy link
Collaborator Author

igneus commented Aug 10, 2024

At least for now I'm done here. Tests are updated. Two are failing - because of issues in the export implementation, not because of outdated test code and examples.

@PeterBjuhr PeterBjuhr merged commit b1dbdce into frescobaldi:master Sep 24, 2024
This was referenced Dec 7, 2024
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.

Broken tests for py3.{9..11}

2 participants