Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion music21/_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
'''
from __future__ import annotations

__version__ = '9.7.1'
__version__ = '9.7.2a4'

def get_version_tuple(vv):
v = vv.split('.')
Expand Down
2 changes: 1 addition & 1 deletion music21/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<class 'music21.base.Music21Object'>

>>> music21.VERSION_STR
'9.7.1'
'9.7.2a4'

Alternatively, after doing a complete import, these classes are available
under the module "base":
Expand Down
7 changes: 5 additions & 2 deletions music21/musicxml/m21ToXml.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from music21 import chord
from music21 import common
from music21.common.enums import AppendSpanners
from music21.common.numberTools import opFrac
from music21 import defaults
from music21 import duration
from music21 import dynamics
Expand Down Expand Up @@ -3277,7 +3278,9 @@ def parseFlatElements(
self.parseOneElement(obj, AppendSpanners.NORMAL)

for n in notesForLater:
if n.isRest and n.style.hideObjectOnPrint and n.duration.type == 'inexpressible':
if (n.isRest
and n.style.hideObjectOnPrint
and n.duration.type in ('inexpressible', 'complex')):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Greg -- 'complex' durations don't exist in m21ToXml unless makeNotation=False. I don't want fixes in here that cause different code paths for the two -- a complex duration in a stream at this point is an error.

# Prefer a gap in stream, to be filled with a <forward> tag by
# fill_gap_with_forward_tag() rather than raising exceptions
continue
Expand Down Expand Up @@ -3322,7 +3325,7 @@ def parseFlatElements(
else:
# if necessary, jump to end of the measure.
if self.offsetInMeasure < firstPassEndOffsetInMeasure:
self.moveForward(firstPassEndOffsetInMeasure)
self.moveForward(opFrac(firstPassEndOffsetInMeasure - self.offsetInMeasure))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this part of the same bug fix or a separate bug fix -- it doesn't seem to have anything to do with complex or not complex types? Thus is it actually being tested by the new tests below? I don't think so, so please test it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unrelated. I'll try to write a test for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the record: this is a bug I introduced back when I was implementing SpannerAnchors in the first place. I assumed the wrong thing about what offset should be passed to moveForward. The end result was that MusicXML write from SpannerAnchor-y scores was pretty busted. And I only noticed now because we're making SpannerAnchors now during MusicXML read, so the bug is triggered.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Still working on this test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just pushed this new test.


self.currentVoiceId = None

Expand Down
21 changes: 21 additions & 0 deletions music21/musicxml/test_m21ToXml.py
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,27 @@ def test_instrumentDoesNotCreateForward(self):
self.assertTrue(tree.findall('.//note'))
self.assertFalse(tree.findall('.//forward'))

def test_complexHiddenRestDoesNotCrashButInsteadCreatesForward(self):
'''
Complex hidden rests were raising an exception. Now they create a forward tag instead.
'''
complexRest = note.Rest()
complexRest.quarterLength = 5.0
complexRest.style.hideObjectOnPrint = True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah -- these are hidden rests already -- okay -- that was an important part.

n = note.Note()
n.quarterLength = 1.0
m = stream.Measure()
m.append(complexRest)
m.append(n)
p = stream.Part()
p.append(m)
s = stream.Score()
s.append(p)
tree = self.getET(s, makeNotation=False)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So all of this only applies if makeNotation is False?????? I'm flabbergasted -- tempted to remove it as an option. There is a reason why we make notation before showing -- to prevent crashes like this. Im really not interested in debugging anything else that comes forward only in makeNotation=False mode.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll try to avoid fixing things specific to makeNotation=False in the future. Sorry about that!

self.assertEqual(len(tree.findall('.//forward')), 1)
self.assertEqual(len(tree.findall('.//note')), 1)
self.assertEqual(len(tree.findall('.//rest')), 0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we test that the forward tag moves forward 5 beats? It's little things like this that help make sure bugs don't creep in in the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I'm on it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just pushed this test improvement.


def testOutOfBoundsExpressionDoesNotCreateForward(self):
'''
A metronome mark at an offset exceeding the bar duration was causing
Expand Down