From 0a9cb1fd3ded5f768e04883db7486058565d3e79 Mon Sep 17 00:00:00 2001 From: "Diego M. Rodriguez" Date: Thu, 29 Dec 2016 18:03:54 +0100 Subject: [PATCH] discogs: revise subtrack merging based on subindex Modify the coalescing of subtracks by taking into account the subindices of the subtracks for deciding if they represent "logical" tracks (merging them into one single track, as previously) or if they represent physical tracks (treating them as individual tracks). Add unit tests for nested logical/physical subtracks, and disc titles. --- beetsplug/discogs.py | 26 +++++++++++++++++++------- test/test_discogs.py | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 28b863d4b..d69e92913 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -354,15 +354,27 @@ class DiscogsPlugin(BeetsPlugin): """Modify `tracklist` in place, merging a list of `subtracks` into a single track into `tracklist`.""" # Calculate position based on first subtrack, without subindex. - idx, medium_idx, _ = self.get_track_index(subtracks[0]['position']) + idx, medium_idx, sub_idx = \ + self.get_track_index(subtracks[0]['position']) position = '%s%s' % (idx or '', medium_idx or '') - if len(tracklist) > 1 and not tracklist[-1]['position']: - # Assume the previous index track contains the track title, and - # "convert" it to a real track. The only exception is if the - # index track is the only one on the tracklist, as it probably - # is a medium title. - tracklist[-1]['position'] = position + if tracklist and not tracklist[-1]['position']: + # Assume the previous index track contains the track title. + if sub_idx: + # "Convert" the track title to a real track, discarding the + # subtracks assuming they are logical divisions of a + # physical track (12.2.9 Subtracks). + tracklist[-1]['position'] = position + else: + # Promote the subtracks to real tracks, discarding the + # index track, assumeing the subtracks are physical tracks. + index_track = tracklist.pop() + # Fix artists when they are specified on the index track. + if index_track.get('artists'): + for subtrack in subtracks: + if not subtrack.get('artists'): + subtrack['artists'] = index_track['artists'] + tracklist.extend(subtracks) else: # Merge the subtracks, pick a title, and append the new track. track = subtracks[0].copy() diff --git a/test/test_discogs.py b/test/test_discogs.py index 5fe5926b8..95f5e7554 100644 --- a/test/test_discogs.py +++ b/test/test_discogs.py @@ -272,11 +272,14 @@ class DGAlbumInfoTest(_common.TestCase): d = DiscogsPlugin().get_album_info(release) self.assertEqual(d.mediums, 1) + self.assertEqual(d.tracks[0].disctitle, 'MEDIUM TITLE') self.assertEqual(len(d.tracks), 1) self.assertEqual(d.tracks[0].title, 'TRACK GROUP TITLE') - def test_parse_tracklist_subtracks_nested(self): - """Test parsing of subtracks defined inside a index track.""" + def test_parse_tracklist_subtracks_nested_logical(self): + """Test parsing of subtracks defined inside a index track that are + logical subtracks (ie. should be grouped together into a single track). + """ release = self._make_release_from_positions(['1', '', '3']) # Track 2: Index track with track group title, and sub_tracks release.data['tracklist'][1]['title'] = 'TRACK GROUP TITLE' @@ -290,6 +293,40 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(len(d.tracks), 3) self.assertEqual(d.tracks[1].title, 'TRACK GROUP TITLE') + def test_parse_tracklist_subtracks_nested_physical(self): + """Test parsing of subtracks defined inside a index track that are + physical subtracks (ie. should not be grouped together). + """ + release = self._make_release_from_positions(['1', '', '4']) + # Track 2: Index track with track group title, and sub_tracks + release.data['tracklist'][1]['title'] = 'TRACK GROUP TITLE' + release.data['tracklist'][1]['sub_tracks'] = [ + self._make_track('TITLE ONE', '2', '01:01'), + self._make_track('TITLE TWO', '3', '02:02') + ] + + d = DiscogsPlugin().get_album_info(release) + self.assertEqual(d.mediums, 1) + self.assertEqual(len(d.tracks), 4) + self.assertEqual(d.tracks[1].title, 'TITLE ONE') + self.assertEqual(d.tracks[2].title, 'TITLE TWO') + + def test_parse_tracklist_disctitles(self): + """Test parsing of index tracks that act as disc titles.""" + release = self._make_release_from_positions(['', '1-1', '1-2', '', + '2-1']) + # Track 1: Index track with medium title (Cd1) + release.data['tracklist'][0]['title'] = 'MEDIUM TITLE CD1' + # Track 4: Index track with medium title (Cd2) + release.data['tracklist'][3]['title'] = 'MEDIUM TITLE CD2' + + d = DiscogsPlugin().get_album_info(release) + self.assertEqual(d.mediums, 2) + self.assertEqual(d.tracks[0].disctitle, 'MEDIUM TITLE CD1') + self.assertEqual(d.tracks[1].disctitle, 'MEDIUM TITLE CD1') + self.assertEqual(d.tracks[2].disctitle, 'MEDIUM TITLE CD2') + self.assertEqual(len(d.tracks), 3) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)