From 49595125a2b358e3fd4f0d61081dad9e57605763 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Sun, 27 Oct 2024 17:20:35 -1000 Subject: [PATCH 1/3] Fix Stream splitByQuarterLengths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1735 Adds tests. Thanks to Giulio Guilherme de Souza Simão for the perfect bug report! --- music21/base.py | 64 ++++++++++++++++++++++++++++++++++------- music21/stream/base.py | 6 ++-- music21/stream/tests.py | 33 +++++++++++++++++++++ 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/music21/base.py b/music21/base.py index b85f2a9ac..8b93a2607 100644 --- a/music21/base.py +++ b/music21/base.py @@ -148,11 +148,11 @@ class ContextSortTuple(t.NamedTuple): class _SplitTuple(tuple): ''' >>> st = base._SplitTuple([1, 2]) - >>> st.spannerList = [3] + >>> st.spannerList = [expressions.Trill()] >>> st (1, 2) >>> st.spannerList - [3] + [] >>> a, b = st >>> a 1 @@ -160,13 +160,48 @@ class _SplitTuple(tuple): 2 >>> st.__class__ + + OMIT_FROM_DOCS + + Might have been a mistake to make an implicit return make sure that + normal tuple comparisons work, but that things do not hash the same. + + st2 has the same (1, 2) value as st1 but no spanners + + >>> st2 = base._SplitTuple([1, 2]) + >>> st == st2 + False + >>> c = set() + >>> c.add(st) + >>> st2 in c + False + + >>> st3 = base._SplitTuple([1, 2]) + >>> st2 == st3 + True + + >>> st_big = base._SplitTuple([1, 3]) + >>> st2 < st_big + True ''' def __new__(cls, tupEls): # noinspection PyTypeChecker return super(_SplitTuple, cls).__new__(cls, tuple(tupEls)) def __init__(self, tupEls): - self.spannerList = [] + self.spannerList: list[spanner.Spanner] = [] + + def __eq__(self, other): + if not isinstance(other, _SplitTuple): + return False + if self.spannerList != other.spannerList: + return False + return super().__eq__(other) + + def __hash__(self): + h1 = super().__hash__() + h2 = id(self.spannerList) + return hash((h1, h2)) # ----------------------------------------------------------------------------- # make subclass of set once that is defined properly @@ -1368,7 +1403,8 @@ def getContextByClass( context for b would be much harder to get without this method, since in order to do it, it searches backwards within the measure, finds that there's nothing there. It goes to the previous measure and searches - that one backwards until it gets the proper TimeSignature of 2/4: + inside that one from the end backwards until it gets the proper + TimeSignature of 2/4: >>> b.getContextByClass(meter.TimeSignature) @@ -1388,7 +1424,7 @@ class name: part. This is all you need to know for most uses. The rest of the docs are for advanced uses: - The method searches both Sites as well as associated objects to find a + The method searches Sites and also associated objects to find a matching class. Returns `None` if no match is found. A reference to the caller is required to find the offset of the object @@ -3064,7 +3100,7 @@ def splitAtQuarterLength( Split an Element into two Elements at a provided `quarterLength` (offset) into the Element. - Returns a specialized tuple that also has + Returns a specialized tuple (_SplitTuple) that also has a .spannerList element which is a list of spanners that were created during the split, such as by splitting a trill note into more than one trill. @@ -3110,7 +3146,6 @@ def splitAtQuarterLength( [>] - Make sure that ties and accidentals remain as they should be: >>> d = note.Note('D#4') @@ -3172,6 +3207,9 @@ def splitAtQuarterLength( ''' from music21 import chord from music21 import note + + st: _SplitTuple + quarterLength = opFrac(quarterLength) if quarterLength > self.duration.quarterLength: @@ -3317,11 +3355,15 @@ def splitByQuarterLengths( displayTiedAccidentals=False ) -> _SplitTuple: ''' - Given a list of quarter lengths, return a list of + Given a list of quarter lengths, return a "SplitTuple" of Music21Object objects, copied from this Music21Object, that are partitioned and tied with the specified quarter length list durations. + THe SplitTuple will also have a .spannerList which + contains a list of spanner created during the split, such as by splitting a trill + note into more than one trill. + TODO: unite into a "split" function -- document obscure uses. >>> n = note.Note() @@ -3341,13 +3383,15 @@ def splitByQuarterLengths( if len(quarterLengthList) == 1: # return a copy of self in a list return _SplitTuple([copy.deepcopy(self)]) - elif len(quarterLengthList) <= 1: + elif not quarterLengthList: raise Music21ObjectException( f'cannot split by this quarter length list: {quarterLengthList}.') - eList = [] + eList: list[Music21Object] = [] spannerList = [] # this does not fully work with trills over multiple splits yet. eRemain = copy.deepcopy(self) + + st: _SplitTuple for qlSplit in quarterLengthList[:-1]: st = eRemain.splitAtQuarterLength(qlSplit, addTies=addTies, diff --git a/music21/stream/base.py b/music21/stream/base.py index 09808e02e..73fb1ebe7 100644 --- a/music21/stream/base.py +++ b/music21/stream/base.py @@ -3221,7 +3221,7 @@ def splitAtQuarterLength(self, retainOrigin=True, addTies=True, displayTiedAccidentals=False, - searchContext=True): + searchContext=True) -> base._SplitTuple: ''' This method overrides the method on Music21Object to provide similar functionality for Streams. @@ -3259,7 +3259,7 @@ def splitAtQuarterLength(self, sRight.clef = copy.deepcopy(endClef) if quarterLength > sLeft.highestTime: # nothing to do - return sLeft, sRight + return base._SplitTuple([sLeft, sRight]) # use quarterLength as start time targets = sLeft.getElementsByOffset( @@ -3303,7 +3303,7 @@ def splitAtQuarterLength(self, sRight.insert(target.getOffsetBySite(sLeft) - quarterLength, target) sLeft.remove(target) - return sLeft, sRight + return base._SplitTuple([sLeft, sRight]) # -------------------------------------------------------------------------- def recurseRepr(self, diff --git a/music21/stream/tests.py b/music21/stream/tests.py index 4c9bb210a..db66dc817 100644 --- a/music21/stream/tests.py +++ b/music21/stream/tests.py @@ -54,6 +54,8 @@ from music21 import tie from music21 import variant +from music21.base import Music21Exception, _SplitTuple + from music21.musicxml import m21ToXml from music21.midi import translate as midiTranslate @@ -7964,6 +7966,37 @@ def testSplitAtQuarterLengthC(self): # sLeft.show() # sRight.show() + def testSplitByQuarterLengths(self): + ''' + Was not returning splitTuples before + ''' + m = Measure([ + note.Note(quarterLength=8.0) + ]) + + with self.assertRaisesRegex(Music21Exception, + 'cannot split by quarter length list whose sum is not equal'): + m.splitByQuarterLengths([1.0, 2.0]) + + parts = m.splitByQuarterLengths([1.0, 2.0, 5.0]) + self.assertIsInstance(parts, _SplitTuple) + self.assertEqual(len(parts), 3) + self.assertIsInstance(parts[0], Measure) + self.assertEqual(parts[0].quarterLength, 1.0) + self.assertEqual(len(parts[0]), 1) + self.assertEqual(parts[0][0].quarterLength, 1.0) + + self.assertEqual(parts[1].quarterLength, 2.0) + self.assertEqual(len(parts[1]), 1) + self.assertEqual(parts[1][0].quarterLength, 2.0) + + self.assertEqual(parts[2].quarterLength, 5.0) + self.assertEqual(len(parts[2]), 1) + self.assertEqual(parts[2][0].quarterLength, 5.0) + self.assertEqual(parts[2][0].duration.type, 'complex') + + + def testGracesInStream(self): ''' testing grace notes From 51bcb634d610b9786d1053ca3a7823d69a1eb916 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Sun, 27 Oct 2024 19:10:22 -1000 Subject: [PATCH 2/3] lint and mypy --- music21/base.py | 2 +- music21/figuredBass/resolution.py | 32 +++++++++++++++---------------- music21/stream/base.py | 11 +++++++---- music21/test/testLint.py | 3 ++- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/music21/base.py b/music21/base.py index 8b93a2607..870de23eb 100644 --- a/music21/base.py +++ b/music21/base.py @@ -188,7 +188,7 @@ def __new__(cls, tupEls): # noinspection PyTypeChecker return super(_SplitTuple, cls).__new__(cls, tuple(tupEls)) - def __init__(self, tupEls): + def __init__(self, tupEls: t.Any) -> None: self.spannerList: list[spanner.Spanner] = [] def __eq__(self, other): diff --git a/music21/figuredBass/resolution.py b/music21/figuredBass/resolution.py index c16c1f70d..9b05d08ff 100644 --- a/music21/figuredBass/resolution.py +++ b/music21/figuredBass/resolution.py @@ -114,11 +114,11 @@ def augmentedSixthToDominant( assert isinstance(fifth, pitch.Pitch) assert isinstance(other, pitch.Pitch) - howToResolve = [(lambda p: p.name == bass.name, '-m2'), - (lambda p: p.name == root.name, 'm2'), - (lambda p: p.name == fifth.name, '-m2'), - (lambda p: p.name == other.name and augSixthType == 3, 'd1'), - (lambda p: p.name == other.name and augSixthType == 2, '-m2')] + howToResolve = [(lambda p: p and bass and p.name == bass.name, '-m2'), + (lambda p: p and root and p.name == root.name, 'm2'), + (lambda p: p and fifth and p.name == fifth.name, '-m2'), + (lambda p: p and other and p.name == other.name and augSixthType == 3, 'd1'), + (lambda p: p and other and p.name == other.name and augSixthType == 2, '-m2')] return _resolvePitches(augSixthPossib, howToResolve) @@ -199,12 +199,12 @@ def augmentedSixthToMajorTonic( assert isinstance(fifth, pitch.Pitch) assert isinstance(other, pitch.Pitch) - howToResolve = [(lambda p: p.name == bass.name, '-m2'), - (lambda p: p.name == root.name, 'm2'), - (lambda p: p.name == fifth.name, 'P1'), - (lambda p: p.name == other.name and augSixthType == 1, 'M2'), - (lambda p: p.name == other.name and augSixthType == 2, 'A1'), - (lambda p: p.name == other.name and augSixthType == 3, 'm2')] + howToResolve = [(lambda p: p and bass and p.name == bass.name, '-m2'), + (lambda p: p and root and p.name == root.name, 'm2'), + (lambda p: p and fifth and p.name == fifth.name, 'P1'), + (lambda p: p and other and p.name == other.name and augSixthType == 1, 'M2'), + (lambda p: p and other and p.name == other.name and augSixthType == 2, 'A1'), + (lambda p: p and other and p.name == other.name and augSixthType == 3, 'm2')] return _resolvePitches(augSixthPossib, howToResolve) @@ -284,11 +284,11 @@ def augmentedSixthToMinorTonic( assert isinstance(fifth, pitch.Pitch) assert isinstance(other, pitch.Pitch) - howToResolve = [(lambda p: p.name == bass.name, '-m2'), - (lambda p: p.name == root.name, 'm2'), - (lambda p: p.name == fifth.name, 'P1'), - (lambda p: p.name == other.name and augSixthType == 1, 'm2'), - (lambda p: p.name == other.name and augSixthType == 3, 'd2')] + howToResolve = [(lambda p: p and bass and p.name == bass.name, '-m2'), + (lambda p: p and root and p.name == root.name, 'm2'), + (lambda p: p and fifth and p.name == fifth.name, 'P1'), + (lambda p: p and other and p.name == other.name and augSixthType == 1, 'm2'), + (lambda p: p and other and p.name == other.name and augSixthType == 3, 'd2')] return _resolvePitches(augSixthPossib, howToResolve) diff --git a/music21/stream/base.py b/music21/stream/base.py index 73fb1ebe7..cc8c7fb47 100644 --- a/music21/stream/base.py +++ b/music21/stream/base.py @@ -3230,6 +3230,8 @@ def splitAtQuarterLength(self, * Changed in v7: all but quarterLength are keyword only ''' + keySignatures: list[key.KeySignature] = [] + quarterLength = opFrac(quarterLength) if retainOrigin: sLeft = self @@ -3247,11 +3249,12 @@ def splitAtQuarterLength(self, if timeSignatures: sRight.keySignature = copy.deepcopy(timeSignatures[0]) if searchContext: - keySignatures = sLeft.getContextByClass(key.KeySignature) - if keySignatures is not None: - keySignatures = [keySignatures] + ksContext = sLeft.getContextByClass(key.KeySignature) + if ksContext is not None: + keySignatures = [ksContext] else: - keySignatures = sLeft.getElementsByClass(key.KeySignature) + keySignatures = list(sLeft.getElementsByClass(key.KeySignature)) + if keySignatures: sRight.keySignature = copy.deepcopy(keySignatures[0]) endClef = sLeft.getContextByClass(clef.Clef) diff --git a/music21/test/testLint.py b/music21/test/testLint.py index 0d5d37c1c..245aec948 100644 --- a/music21/test/testLint.py +++ b/music21/test/testLint.py @@ -48,7 +48,7 @@ def main(fnAccept=None, strict=False): ''' - `fnAccept` is a list of one or more files to test. Otherwise runs all. + `fnAccept` is a list of one or more files to test. Otherwise, runs all. ''' poolSize = common.cpus() @@ -75,6 +75,7 @@ def main(fnAccept=None, strict=False): 'too-many-lines', # yes, someday. 'too-many-return-statements', # we'll see 'too-many-instance-attributes', # maybe later + 'too-many-positional-arguments', # let's get this at least to max 6. 'inconsistent-return-statements', # would be nice 'protected-access', # this is an important one, but for now we do a lot of # x = copy.deepcopy(self); x._volume = ... which is not a problem... From 80eda7cd2b043c5ee3f48fd0630b2811af313da7 Mon Sep 17 00:00:00 2001 From: Michael Scott Asato Cuthbert Date: Sun, 27 Oct 2024 19:17:01 -1000 Subject: [PATCH 3/3] two places for lint --- .pylintrc | 1 + music21/test/testLint.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/.pylintrc b/.pylintrc index d823a13bc..b901d91df 100644 --- a/.pylintrc +++ b/.pylintrc @@ -69,6 +69,7 @@ disable= too-many-lines, # yes, someday too-many-return-statements, # we'll see too-many-instance-attributes, # maybe later + too-many-positional-arguments, # try to get down to 6 first... # no-self-use, # moved to optional extension. invalid-name, # these are good music21 names; fix the regexp instead... too-few-public-methods, # never remove or set to 1 diff --git a/music21/test/testLint.py b/music21/test/testLint.py index 245aec948..9f55a060e 100644 --- a/music21/test/testLint.py +++ b/music21/test/testLint.py @@ -12,6 +12,8 @@ from __future__ import annotations # this requires pylint to be installed and available from the command line +# Anything changed here also needs to be changed at .pylintrc + import argparse import os