Merge pull request #1826 from cuthbertLab/stream-not-flat

Remove Stream.flat - use .flatten() instead

Author: mscuthbert

music21/musicxml/xmlToM21.py

@@ -1864,9 +1864,12 @@ def copy_into_partStaff(source: stream.Stream,
             removeClasses = STAFF_SPECIFIC_CLASSES[:]
             if staffIndex != 0:  # spanners only on the first staff.
                 removeClasses.append('Spanner')
-            newPartStaff = self.stream.template(removeClasses=removeClasses,
-                                                fillWithRests=False,
-                                                exemptFromRemove=EXEMPT_FROM_REMOVE)
+            newPartStaff = t.cast(
+                stream.PartStaff,
+                self.stream.template(removeClasses=removeClasses,
+                                     fillWithRests=False,
+                                     exemptFromRemove=EXEMPT_FROM_REMOVE)
+            )
             partStaffId = f'{self.partId}-Staff{staffKey}'
             newPartStaff.id = partStaffId
             # set group for components (recurse?)

music21/note.py

@@ -1935,9 +1935,7 @@ class Rest(GeneralNote):
     gets rests as well.
 
     >>> r = note.Rest()
-    >>> r.isRest
-    True
-    >>> r.isNote
+    >>> isinstance(r, note.Note)
     False
     >>> r.duration.quarterLength = 2.0
     >>> r.duration.type
@@ -1985,6 +1983,13 @@ class Rest(GeneralNote):
 
     >>> r1 == note.Note()
     False
+
+    Currently, there are these convenience features, but they are going away
+    (They were originally added because isinstance was slow. It is now very fast)
+    >>> r.isRest
+    True
+    >>> r.isNote
+    False
     '''
     isRest = True
     name = 'rest'

music21/prebase.py

@@ -117,7 +117,7 @@ def classes(self) -> tuple[str, ...]:
         {10.0} <music21.clef.GClef>
         {30.0} <music21.clef.FrenchViolinClef>
 
-        `Changed 2015 Sep`: returns a tuple, not a list.
+        Changed in v2: returns a tuple, not a list.
         '''
         try:
             return self._classTupleCacheDict[self.__class__]

music21/stream/base.py

@@ -324,9 +324,6 @@ def __init__(self,
         # restrictClass: type[M21ObjType] = base.Music21Object,
         super().__init__(**keywords)
 
-        # TEMPORARY variable for v9 to deprecate the flat property. -- remove in v10
-        self._created_via_deprecated_flat = False
-
         self.streamStatus = streamStatus.StreamStatus(self)
         self._unlinkedDuration = None
 
@@ -451,13 +448,6 @@ def __iter__(self) -> iterator.StreamIterator[M21ObjType]:
         specialized :class:`music21.stream.StreamIterator` class, which
         adds necessary Stream-specific features.
         '''
-        # temporary for v9 -- remove in v10
-        if self._created_via_deprecated_flat:
-            warnings.warn('.flat is deprecated.  Call .flatten() instead',
-                          exceptions21.Music21DeprecationWarning,
-                          stacklevel=3)
-            self._created_via_deprecated_flat = False
-
         return t.cast(iterator.StreamIterator[M21ObjType],
                       iterator.StreamIterator(self))
 
@@ -4699,11 +4689,11 @@ def measure(self,
     def template(self,
                  *,
                  fillWithRests=True,
-                 removeClasses=None,
+                 removeClasses: Iterable[type|str]|set[type|str]|None = None,
                  retainVoices=True,
                  removeAll=False,
                  exemptFromRemove=frozenset(),
-                 ):
+                 ) -> t.Self:
         '''
         Return a new Stream based on this one, but without the notes and other elements
         but keeping instruments, clefs, keys, etc.
@@ -4874,14 +4864,11 @@ def template(self,
 
         * Changed in v7: all arguments are keyword only.
         * New in v9.9: added exemptFromRemove
-        * Note: in v10
+        * Note: in v10: removeClasses cannot be boolean -- use removeAll instead
         '''
         out = self.cloneEmpty(derivationMethod='template')
         if removeClasses is None:
             removeClasses = {'GeneralNote', 'Dynamic', 'Expression'}
-        elif removeClasses is True:
-            removeClasses = set()
-            removeAll = True
         elif common.isIterable(removeClasses):
             removeClasses = set(removeClasses)
 
@@ -4903,7 +4890,7 @@ def optionalAddRest():
             elOffset = self.elementOffset(el, returnSpecial=True)
 
             # retain all streams (exception: Voices if retainVoices  is False
-            if el.isStream and (retainVoices or ('Voice' not in el.classes)):
+            if isinstance(el, Stream) and (retainVoices or ('Voice' not in el.classes)):
                 optionalAddRest()
                 outEl = el.template(fillWithRests=fillWithRests,
                                     removeClasses=removeClasses,
@@ -4919,7 +4906,7 @@ def optionalAddRest():
 
             # okay now determine if we will be skipping or keeping this element
             skip_element = False
-            if removeAll is True:
+            if removeAll:
                 # with this setting we remove everything by default
                 skip_element = True
             elif el.classSet.intersection(removeClasses):
@@ -7770,7 +7757,7 @@ def flatten(self: StreamType, retainContainers=False) -> StreamType:
         A very important method that returns a new Stream
         that has all sub-containers "flattened" within it,
         that is, it returns a new Stream where no elements nest within
-        other elements.
+        other elements.  (Prior to v7 this was the property .flat)
 
         Here is a simple example of the usefulness of .flatten().  We
         will create a Score with two Parts in it, each with two Notes:
@@ -7847,6 +7834,7 @@ def flatten(self: StreamType, retainContainers=False) -> StreamType:
 
         If `retainContainers=True` then a "semiFlat" version of the stream
         is returned where Streams are also included in the output stream.
+        (Prior to v7 this was the property semiFlat)
 
         In general, you will not need to use this because `.recurse()` is
         more efficient and does not lead to problems of the same
@@ -7963,25 +7951,19 @@ def flatten(self: StreamType, retainContainers=False) -> StreamType:
          <music21.note.Note D>,
          <music21.note.Note D>)
 
-        OMIT_FROM_DOCS
-
-        >>> r = stream.Stream()
-        >>> for j in range(5):
-        ...   q = stream.Stream()
-        ...   for i in range(5):
-        ...      p = stream.Stream()
-        ...      p.repeatInsert(base.Music21Object(), [0, 1, 2, 3, 4])
-        ...      q.insert(i * 10, p)
-        ...   r.insert(j * 100, q)
-
-        >>> len(r)
-        5
-
-        >>> len(r.flatten())
-        125
+        .. note::Why did `.flat` become `.flatten()`? Early on music21's philosophy
+            was to use properties for commonly accessed "views" of a stream. This
+            worked well in the pre-IDE/debugger days of early Python 2. Now however
+            most of us program with IDEs and AI assistance that freely introspect the
+            attributes of objects. So if you're working with a Pitch object, your IDE
+            may have already looked up its .name or .accidental without your knowledge.
+            Properties are generally considered the same as attributes (that's what
+            they're designed for).  The problem is that flattening a stream is a
+            time consuming algorithm that alters the stream and all its elements in
+            the process. Therefore streams should only be flattened when the programmer
+            requests them to be. The way to tell a modern IDE that a process may
+            have consequences is to make it a `.method()` not a `.property`.
 
-        >>> r.flatten()[124].offset
-        444.0
         '''
         # environLocal.printDebug(['flatten(): self', self,
         #  'self.activeSite', self.activeSite])
@@ -8046,20 +8028,6 @@ def flatten(self: StreamType, retainContainers=False) -> StreamType:
 
         return sNew
 
-    @property
-    def flat(self):
-        '''
-        Deprecated: use `.flatten()` instead
-
-        A property that returns the same flattened representation as `.flatten()`
-        as of music21 v7.
-
-        See :meth:`~music21.stream.base.Stream.flatten()` for documentation.
-        '''
-        flatStream = self.flatten(retainContainers=False)
-        flatStream._created_via_deprecated_flat = True
-        return flatStream
-
     @overload
     def recurse(self,
                 *,

music21/stream/core.py

@@ -693,7 +693,7 @@ def coreGatherMissingSpanners(
         '''
         sb = self.spannerBundle
         sIter: StreamIterator|RecursiveIterator
-        if recurse is True:
+        if recurse:
             sIter = self.recurse()  # type: ignore
         else:
             sIter = self.iter()  # type: ignore
@@ -708,16 +708,16 @@ def coreGatherMissingSpanners(
                 if constrainingSpannerBundle is not None and sp not in constrainingSpannerBundle:
                     continue
                 if requireAllPresent:
-                    allFound = True
+                    allFound: bool = True
                     for spannedElement in sp.getSpannedElements():
                         if spannedElement not in sIter:
                             allFound = False
                             break
-                    if allFound is False:
+                    if not allFound:
                         continue
                 collectList.append(sp)
 
-        if insert is False:
+        if not insert:
             return collectList
 
         if collectList:  # do not run elementsChanged if nothing here.

music21/stream/iterator.py

@@ -79,9 +79,6 @@ class StreamIterator(prebase.ProtoM21Object, Sequence[M21ObjType]):
     * StreamIterator.streamLength -- length of elements.
 
     * StreamIterator.srcStreamElements -- srcStream._elements
-    * StreamIterator.cleanupOnStop -- should the StreamIterator delete the
-      reference to srcStream and srcStreamElements when stopping? default
-      False -- DEPRECATED: to be removed in v10.
     * StreamIterator.activeInformation -- a dict that contains information
       about where we are in the parse.  Especially useful for recursive
       streams:
@@ -114,6 +111,7 @@ class StreamIterator(prebase.ProtoM21Object, Sequence[M21ObjType]):
       - StreamIterator inherits from typing.Sequence, hence index
       was moved to elementIndex.
     * Changed in v9: cleanupOnStop is deprecated.  Was not working properly before: noone noticed.
+    * Changed in v10: remove cleanupOnStop.
 
     OMIT_FROM_DOCS
 
@@ -152,7 +150,6 @@ def __init__(self,
         self.sectionIndex: int = -1
         self.iterSection: t.Literal['_elements', '_endElements'] = '_elements'
 
-        self.cleanupOnStop: bool = False
         self.restoreActiveSites: bool = restoreActiveSites
 
         self.overrideDerivation: str|None = None
@@ -358,7 +355,6 @@ def __getitem__(self, k: int|slice|str) -> M21ObjType|list[M21ObjType]|None:
         >>> sI.srcStream is s
         True
 
-
         To request an element by id, put a '#' sign in front of the id,
         like in HTML DOM queries:
 
@@ -388,23 +384,6 @@ def __getitem__(self, k: int|slice|str) -> M21ObjType|list[M21ObjType]|None:
         >>> s.iter().notes[0]
         <music21.note.Note F#>
 
-        Demo of cleanupOnStop = True; the sI[0] call counts as another iteration, so
-        after it is called, there is nothing more to iterate over!  Note that cleanupOnStop
-        will be removed in music21 v10.
-
-        >>> sI.cleanupOnStop = True
-        >>> for n in sI:
-        ...    printer = (repr(n), repr(sI[0]))
-        ...    print(printer)
-        ('<music21.note.Note F#>', '<music21.note.Note F#>')
-        >>> sI.srcStream is s  # set to an empty stream
-        False
-        >>> for n in sI:
-        ...    printer = (repr(n), repr(sI[0]))
-        ...    print(printer)
-
-        (nothing is printed)
-
         * Changed in v8: for strings: prepend a '#' sign to get elements by id.
           The old behavior still works until v9.
           This is an attempt to unify __getitem__ behavior in
@@ -652,23 +631,9 @@ def resetCaches(self) -> None:
 
     def cleanup(self) -> None:
         '''
-        stop iteration; and cleanup if need be.
+        stop iteration; and cleanup if need be.  Can be subclassed. does nothing.
         '''
-        if self.cleanupOnStop:
-            self.reset()
-
-            # cleanupOnStop is rarely used, so we put in
-            # a dummy stream so that self.srcStream does not need
-            # to be typed as Stream|None
-
-            # eventually want this to work
-            # SrcStreamClass = t.cast(type[StreamType], self.srcStream.__class__)
-            SrcStreamClass = self.srcStream.__class__
-
-            del self.srcStream
-            del self.srcStreamElements
-            self.srcStream = SrcStreamClass()
-            self.srcStreamElements = ()
+        pass
 
     # ---------------------------------------------------------------
     # getting items