Merge pull request #2355 from diego-plan9/discogs-subtrack-handling

discogs: revise subtrack merging based on subindex

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, assuming 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()

docs/changelog.rst

@@ -29,6 +29,9 @@ Fixes:
   instead of being reset to "other." :bug:`2339`
 * :doc:`/plugins/web`: Fix a crash on Python 3 when serving files from the
   filesystem. :bug:`2353`
+* :doc:`/plugins/discogs`: Improve the handling of releases that contain
+  subtracks. :bug:`2318`
+
 
 For plugin developers: new importer prompt choices (see :ref:`append_prompt_choices`), you can now provide new candidates for the user to consider.
 

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__)