-
Notifications
You must be signed in to change notification settings - Fork 406
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
Minimize gaps produced by quantization algorithm #1540
Changes from 10 commits
98533df
f64ea43
517f046
fe5186a
d4ebe55
a7135d3
1ea8840
44450c4
65692c8
5c232c9
0206700
18b750f
df97c67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4027,6 +4027,8 @@ def procCompare(srcOffset, srcDur, dstOffset, dstDur, divList): | |
n = note.Note() | ||
n.quarterLength = srcDur[i] | ||
s.insert(srcOffset[i], n) | ||
# Must be sorted for quantizing to work optimally. | ||
s.sort() | ||
Comment on lines
+4030
to
+4031
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tiny bit confused by this addition -- Streams are autosorted by default. This seems to be a problem of quantize if it somehow bypasses the normal "sort before iterate/access" paradigm. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might have been an artifact from an earlier commit. Does look unnecessary now. 😅 |
||
|
||
s.quantize(divList, processOffsets=True, processDurations=True, inPlace=True) | ||
|
||
|
@@ -4071,6 +4073,18 @@ def procCompare(srcOffset, srcDur, dstOffset, dstDur, divList): | |
|
||
[8, 6]) # snap to 0.125 and 0.1666666 | ||
|
||
# User-reported example: contains overlap and tiny gaps | ||
# Parsing with fewer gaps in v.9, as long as stream is sorted | ||
# https://github.com/cuthbertLab/music21/issues/1536 | ||
procCompare([2.016, 2.026, 2.333, 2.646, 3.0, 3.323, 3.651], | ||
[0.123, 0.656, 0.104, 0.094, 0.146, 0.099, 0.141], | ||
|
||
[2, 2, F('7/3'), F('8/3'), 3.0, F('10/3'), F('11/3')], | ||
[F('1/3'), F('2/3'), F('1/3'), F('1/3'), | ||
F('1/3'), F('1/3'), 0.25], | ||
|
||
[4, 3]) | ||
|
||
def testQuantizeMinimumDuration(self): | ||
''' | ||
Notes (not rests!) of nonzero duration should retain a nonzero | ||
|
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.
okay, but generally add new attributes to a namedtuple at the end so anyone using
[0]
still gets the same result.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.
Yeah. Unfortunately we could no longer use it for sorting (unless there's something clever I'm not thinking of, like layering a sort function on top, but that's what I took the namedtuple to function as.)
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.
Generally I've used namedtuples because before it was a standard (anonymous) tuple, so people might still be using access by index or destructuring paradigms.
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.
ah, yes that's fine. You could sort with
key=lambda bqm: (bqm[-1], *bqm)
but yeah, too much trouble for backwards compatibility on an internal object.I did though change it to use min -- there's no need to sort the whole thing if you're just getting the minimum
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.
ah! That was old code back when the m21 team didn't know its own algorithmic complexity