From 021ab5b65d24be2aeed7893ad5cebd6d0bbeda2a Mon Sep 17 00:00:00 2001 From: Myke Cuthbert Date: Thu, 3 Dec 2020 15:00:02 -1000 Subject: [PATCH] Cleanup a few Stream things and try to get new coverage report --- music21/common/numberTools.py | 3 +- music21/stream/__init__.py | 86 +++++++++++++++++++++++------------ music21/stream/core.py | 61 +++++++++++++++++-------- 3 files changed, 100 insertions(+), 50 deletions(-) diff --git a/music21/common/numberTools.py b/music21/common/numberTools.py index 19904ed4ef..7fcc28b6b8 100644 --- a/music21/common/numberTools.py +++ b/music21/common/numberTools.py @@ -248,7 +248,6 @@ def opFrac(num): This is a performance critical operation. Do not alter it in any way without running many timing tests. - >>> from fractions import Fraction >>> defaults.limitOffsetDenominator 65535 @@ -298,7 +297,7 @@ def opFrac(num): elif num is None: return None - # class inheritance only check AFTER ifs... + # class inheritance only check AFTER ifs... this is redundant but highly optimized. elif isinstance(num, int): return num + 0.0 elif isinstance(num, float): diff --git a/music21/stream/__init__.py b/music21/stream/__init__.py index ca4a301f76..54da492100 100644 --- a/music21/stream/__init__.py +++ b/music21/stream/__init__.py @@ -215,8 +215,6 @@ def __init__(self, givenElements=None, *args, **keywords): self._unlinkedDuration = None self.autoSort = True - # should isFlat become readonly? - self.isFlat = True # does it have no embedded Streams # these should become part of style or something else... self.definesExplicitSystemBreaks = False @@ -986,8 +984,6 @@ def hasElementOfClass(self, className, forceFlat=False): Only a single class name can be given. - Possibly to be deprecated in v.6 - >>> s = stream.Stream() >>> s.append(meter.TimeSignature('5/8')) >>> s.append(note.Note('d-2')) @@ -996,12 +992,18 @@ def hasElementOfClass(self, className, forceFlat=False): True >>> s.hasElementOfClass('Measure') False + + To be deprecated in v.7 -- to be removed in version 8, use: + + >>> bool(s.getElementsByClass('TimeSignature')) + True + >>> bool(s.getElementsByClass('Measure')) + False + + forceFlat does nothing, while getElementsByClass can be done on recurse() ''' # environLocal.printDebug(['calling hasElementOfClass()', className]) - for e in self._elements: - if className in e.classSet: - return True - for e in self._endElements: + for e in self.elements: if className in e.classSet: return True return False @@ -1107,20 +1109,17 @@ def index(self, el): else: self._cache['index'] = {} - # TODO: possibly replace by binary search - if common.isNum(el): - objId = el - else: - objId = id(el) + objId = id(el) count = 0 + for e in self._elements: - if id(e) == objId: + if e is el: self._cache['index'][objId] = count return count count += 1 for e in self._endElements: - if id(e) == objId: + if e is el: self._cache['index'][objId] = count return count # this is the index count += 1 # cumulative indices @@ -1562,7 +1561,14 @@ def _replaceSpannerBundleForDeepcopy(self, new): # this must be done here, not when originally copying e.purgeOrphans(excludeStorageStreams=False) - def setElementOffset(self, element, offset, *, addElement=False, setActiveSite=True): + def setElementOffset( + self, + element: base.Music21Object, + offset: Union[int, float, Fraction, str], + *, + addElement=False, + setActiveSite=True + ): ''' Sets the Offset for an element, very quickly. @@ -1597,10 +1603,13 @@ def setElementOffset(self, element, offset, *, addElement=False, setActiveSite=T Changed in v5.5 -- also sets .activeSite for the element unless setActiveSite is False ''' + # Note: not documenting 'highestTime' is on purpose, since can only be done for + # elements already stored at end. Infinite loop. try: offset = opFrac(offset) except TypeError: - pass + if offset not in core.OFFSET_STRING_VALUES: # pragma: no cover + raise StreamException(f'Cannot set offset to {offset!r} for {element}') idEl = id(element) if not addElement and idEl not in self._offsetDict: @@ -1674,7 +1683,7 @@ def elementOffset(self, element, stringReturns=False): 'an entry for this object 0x%x is not stored in stream %s' % (id(element), self)) - if stringReturns is False and o in ('highestTime', 'lowestOffset', 'highestOffset'): + if stringReturns is False and o in core.OFFSET_STRING_VALUES: try: return getattr(self, o) except AttributeError: @@ -1846,12 +1855,15 @@ def insertIntoNoteOrChord(self, offset, noteOrChord, chordsOnly=False): ''' # could use duration of Note to get end offset span - targets = list(self.iter.getElementsByOffset( - offset, - offset + noteOrChord.quarterLength, # set end to dur of supplied - includeEndBoundary=False, - mustFinishInSpan=False, - mustBeginInSpan=True).notesAndRests) + targets = list( + self.iter.getElementsByOffset( + offset, + offset + noteOrChord.quarterLength, # set end to dur of supplied + includeEndBoundary=False, + mustFinishInSpan=False, + mustBeginInSpan=True + ).notesAndRests + ) removeTarget = None # environLocal.printDebug(['insertIntoNoteOrChord', [e for e in targets]]) if len(targets) == 1: @@ -1950,8 +1962,8 @@ def append(self, others): (6.0, 9.0) >>> notes2 = [] - Since notes are not embedded in Elements here, their offset - changes when they are added to a stream! + Notes' naive offsets will + change when they are added to a stream. >>> for x in range(3): ... n = note.Note('A-') @@ -2046,7 +2058,7 @@ def append(self, others): # we cannot keep the index cache here b/c we might self.coreElementsChanged(updateIsFlat=updateIsFlat) self.isSorted = storeSorted - self._setHighestTime(highestTime) # call after to store in cache + self._setHighestTime(opFrac(highestTime)) # call after to store in cache def storeAtEnd(self, itemOrList, ignoreSort=False): ''' @@ -7288,6 +7300,9 @@ def highestTime(self): 47.0 >>> r = q.flat + Changed in v6.5 -- highestTime can return a Fraction. + + OMIT_FROM_DOCS Make sure that the cache really is empty @@ -7295,6 +7310,19 @@ def highestTime(self): False >>> r.highestTime # 44 + 3 47.0 + + Can highestTime be a fraction? + + >>> n = note.Note('D') + >>> n.duration.quarterLength = 1/3 + >>> n.duration.quarterLength + Fraction(1, 3) + + >>> s = stream.Stream() + >>> s.append(note.Note('C')) + >>> s.append(n) + >>> s.highestTime + Fraction(4, 3) ''' # TODO(msc) -- why is cache 'HighestTime' and not 'highestTime'? # environLocal.printDebug(['_getHighestTime', 'isSorted', self.isSorted, self]) @@ -7306,7 +7334,7 @@ def highestTime(self): self._cache['HighestTime'] = 0.0 return 0.0 else: - highestTimeSoFar = Fraction(0, 1) + highestTimeSoFar = 0.0 # TODO: optimize for a faster way of doing this. # but cannot simply look at the last element because what if the penultimate # element, with a @@ -7318,7 +7346,7 @@ def highestTime(self): + e.duration.quarterLength) if candidateOffset > highestTimeSoFar: highestTimeSoFar = candidateOffset - self._cache['HighestTime'] = float(highestTimeSoFar) + self._cache['HighestTime'] = opFrac(highestTimeSoFar) return self._cache['HighestTime'] @property diff --git a/music21/stream/core.py b/music21/stream/core.py index 6ee50efe27..654d741049 100644 --- a/music21/stream/core.py +++ b/music21/stream/core.py @@ -22,35 +22,47 @@ All functions here will eventually begin with `.core`. ''' # pylint: disable=attribute-defined-outside-init - +from typing import List, Dict, Union, Tuple +from fractions import Fraction import unittest +from music21.base import Music21Object from music21 import spanner from music21 import tree from music21.exceptions21 import StreamException, ImmutableStreamException +OFFSET_STRING_VALUES = {'highestTime', 'lowestOffset', 'highestOffset'} class StreamCoreMixin: def __init__(self): # hugely important -- keeps track of where the _elements are # the _offsetDict is a dictionary where id(element) is the - # index and the index and the offset is the value. - self._offsetDict = {} + # index and the value is a tuple of offset and element. + # offsets can be floats, Fractions, or the special string 'highestTime' + self._offsetDict: Dict[int, Tuple[Union[float, Fraction, str], Music21Object]] = {} + # self._elements stores Music21Object objects. - self._elements = [] + self._elements: List[Music21Object] = [] # self._endElements stores Music21Objects found at # the highestTime of this Stream. - self._endElements = [] + self._endElements: List[Music21Object] = [] self.isSorted = True - # v4! + # should isFlat become readonly? + self.isFlat = True # does it have no embedded Streams + + # someday... # self._elementTree = tree.trees.ElementTree(source=self) - def coreInsert(self, offset, element, - *, - ignoreSort=False, setActiveSite=True - ): + def coreInsert( + self, + offset: Union[float, Fraction], + element: Music21Object, + *, + ignoreSort=False, + setActiveSite=True + ): ''' N.B. -- a "core" method, not to be used by general users. Run .insert() instead. @@ -93,7 +105,12 @@ def coreInsert(self, offset, element, if highestSortTuple < thisSortTuple: storeSorted = True - self.setElementOffset(element, float(offset), addElement=True, setActiveSite=setActiveSite) + self.setElementOffset( + element, + float(offset), # why is this not opFrac? + addElement=True, + setActiveSite=setActiveSite + ) element.sites.add(self) # need to explicitly set the activeSite of the element # will be sorted later if necessary @@ -101,7 +118,12 @@ def coreInsert(self, offset, element, # self._elementTree.insert(float(offset), element) return storeSorted - def coreAppend(self, element, setActiveSite=True): + def coreAppend( + self, + element: Music21Object, + *, + setActiveSite=True + ): ''' N.B. -- a "core" method, not to be used by general users. Run .append() instead. @@ -131,12 +153,13 @@ def coreAppend(self, element, setActiveSite=True): # most will set isSorted to False def coreElementsChanged( - self, - *, - updateIsFlat=True, - clearIsSorted=True, - memo=None, - keepIndex=False): + self, + *, + updateIsFlat=True, + clearIsSorted=True, + memo=None, + keepIndex=False + ): ''' NB -- a "core" stream method that is not necessary for most users. @@ -222,7 +245,7 @@ def coreElementsChanged( if keepIndex and indexCache is not None: self._cache['index'] = indexCache - def coreHasElementByMemoryLocation(self, objId): + def coreHasElementByMemoryLocation(self, objId: int) -> bool: ''' NB -- a "core" stream method that is not necessary for most users. use hasElement(obj)