cuthbertLab / music21

music21 is a Toolkit for Computational Musicology
https://www.music21.org/
Other
2.1k stars 398 forks source link

attachIntervalsBetweenStreams() doesn't clear previous results #1091

Closed dotinspiredby closed 2 years ago

dotinspiredby commented 3 years ago

music21 version

6.7.1

Dear music21 Team! Recently I found in the Stream method attachIntervalsBetweenStreams some unusual thing. I made a piece of code, which returns a list of the intervals between each Part from the SATB Score, and it happened so that instead of None (in place of the rest) it returns the interval from the neighbour pair of Parts. In particular - here is the fragment of the Score, where Bass line is rested at the offset 4.0, but the method returns somehow the P8, which is evidently from the Tenor-Soprano interval at offset 4.0. Also Maybe there is something unclear for me how attachIntervalsBetweenStreams works, but how is it going the way that the third voice is involved in the interval analysis?

Sorry in advance for the bulky piece of code:) - in fact the entire class ScoreDocument isn't needed, it's just to show how I get the list of Part objects for the interval analysis. And as for __make_interval_list method - there is just some logic for spotting the voice crossing (by searching the "-" in the string name of the interval and removing it). And also what makes it more complicated is the fact that intervals are identified by offsets of the left-hand-side stream, so to get the whole picture I need to run the method twice (for example: Soprano-Tenor, Tenor-Soprano).

I would be so grateful for any help or recommendation!

Steps to reproduce

from music21 import converter, stream
from itertools import combinations

class ScoreDocument:
    """ Extra class, which takes a link to xml and returns
    the score which each voice separated into its own staff line"""
    def __init__(self, link):
        self.link = link

    @property
    def test_score(self):
        """ score which each voice separated into its own staff line,
            created mostly for .show() method"""
        return self._distribute_voices()

    @property
    def test_score_voices(self):
        """ returns a list of stream.Part objects from the whole Score
            like [<stream.Part Soprano>, <stream.Part Alto>,
                  <stream.Part Tenor>, <stream.Part Bass>]"""
        return self._split_into_parts(self.__output_name())

    def _distribute_voices(self):
        test = converter.parse(self.link)
        return test.voicesToParts()

    def __output_name(self):
        names = []
        instrument_obj = self.test_score.recurse().getElementsByClass(stream.Part)
        for n in instrument_obj:
            try:
                # this is made to prevent some undefined staff lines, rarely used
                int(n.id[:-3])
            except ValueError:
                names.append(n.id)
        return names

    def _split_into_parts(self, ids):
        for part_id in ids:
            yield self.test_score.parts[part_id]

class Grader:

    def __init__(self, link):
        self.doc = ScoreDocument(link)

    def run(self):
        for pair in combinations(self.doc.test_score_voices, 2):
            self.get_intervals_and_check(pair[-1], pair[0])

    @staticmethod
    def __make_interval_list(voice_a, voice_b, direction, vc_abr):
        interval_list = []
        for measure_index in range(len(voice_a.elements)):
            measure_intervals = []
            voice_a.elements[measure_index].flat.attachIntervalsBetweenStreams(
                voice_b.elements[measure_index].flat)
            for n in voice_a.elements[measure_index].notes:
                if n.editorial.harmonicInterval is None:
                    pass
                else:
                    if "-" in n.editorial.harmonicInterval.directedName:
                        measure_intervals.append(
                            (n.editorial.harmonicInterval.directedName.replace("-", ""), n.offset))
                    else:
                        measure_intervals.append((n.editorial.harmonicInterval.directedName, n.offset))
            interval_list.append(measure_intervals)
        return interval_list

    def get_intervals_and_check(self, voice_1, voice_2):
        intrvls_up_down = self.__make_interval_list(voice_1, voice_2, "up",
                                                    [voice_1, voice_2])
        intrvls_down_up = self.__make_interval_list(voice_2, voice_1, "down", [voice_1, voice_2])
        print(voice_1.id, voice_2.id, intrvls_up_down)
        print(voice_2.id, voice_1.id, intrvls_down_up)

if __name__ == "__main__":
    file_link = "C:/.../bug.xml"
    path = Grader(file_link)
    path.run()

Tenor-v0 Soprano-v0 [[('m6', 2.0), ('P8', 4.0)]] Soprano-v0 Tenor-v0 [[('m6', 2.0), ('P8', 4.0), ('P8', 6.0)]] Bass-v0 Soprano-v0 [[('P12', 6.0)]] Soprano-v0 Bass-v0 [[('m10', 2.0), ('P8', 4.0), ('P12', 6.0)]] P8 at 4.0 does not exist Bass-v0 Tenor-v0 [[('P5', 0.0), ('P5', 6.0)]] Tenor-v0 Bass-v0 [[('P5', 0.0), ('P5', 2.0), ('P8', 4.0)]] same situation measure-1

More information

jacobtylerwalls commented 3 years ago

Thanks for writing. By analyzing the same stream.Part twice, you have overwritten the old intervals with the new intervals, if any. Your unwanted result stems from re-analyzing the same part's notes against a different part and only partially overwriting the results due to the rests, giving you left over intervals from the old analysis.

I suggest retrieving the results and clearing the editorial objects before re-analyzing the part, or else making a deep copy.

jacobtylerwalls commented 3 years ago

Reopening to track the possibility of clearing the previous result when re-analyzing.

diff --git a/music21/stream/base.py b/music21/stream/base.py
index 6eefbfc53..9304a7e69 100644
--- a/music21/stream/base.py
+++ b/music21/stream/base.py
@@ -10592,6 +10592,7 @@ class Stream(core.StreamCoreMixin, base.Music21Object):
         None
         '''
         for n in self.notes:
+            n.editorial.pop('harmonicInterval', None)
             # get simultaneous elements form other stream
             simultEls = cmpStream.getElementsByOffset(self.elementOffset(n),
                                                       mustBeginInSpan=False,
mscuthbert commented 3 years ago

attachIntervalsBetweenStreams has always been a bit of a mess -- it was a tool for a specific task that didn't need voices or chords or any other complication -- , but I agree that cleaning editorial first is a good idea.

dotinspiredby commented 3 years ago

@jacobtylerwalls Thank you a lot! Your help really made my day!!

@mscuthbert Yes, I was aware that .attachIntervals...> method was not supposed for voices/chords.

Thank you very much once again and wishing you a great day!