Cleanup for #6177, #6068

Author: henry-oberholtzer

beetsplug/discogs.py

@@ -124,6 +124,17 @@ class AlbumArtistInfo(ArtistInfo):
     albumartists_ids: list[str]
 
 
+class TracklistInfo:
+    def __init__(self):
+        self.index: int = 0
+        self.index_tracks: dict[int, str] = {}
+        self.tracks: list[TrackInfo] = []
+        self.divisions: list[str] = []
+        self.next_divisions: list[str] = []
+        self.mediums: list[str | None] = []
+        self.medium_indices: list[str | None] = []
+
+
 class DiscogsPlugin(MetadataSourcePlugin):
     def __init__(self):
         super().__init__()
@@ -371,7 +382,6 @@ def build_albumartistinfo(self, artists: list[Artist]) -> AlbumArtistInfo:
             "albumartist": info["artist"],
             "albumartist_id": info["artist_id"],
             "albumartists": info["artists"],
-            "albumartists_ids": info["artists_ids"],
             "albumartist_credit": info["artist_credit"],
             "albumartists_credit": info["artists_credit"],
         }
@@ -386,12 +396,6 @@ def build_artistinfo(
         """Iterates through a discogs result and builds
         up the artist fields. Does not contribute to
         artist_sort as Discogs does not define that.
-
-        :param artists: A list of Discogs Artist objects
-
-        :param album_artist: If building an album artist,
-            we need to account for the album_artist anv parameter.
-        :return an ArtistInfo dictionary.
         """
         info: ArtistInfo = {
             "artist": "",
@@ -420,7 +424,7 @@ def build_artistinfo(
         for a in given_artists:
             # Get the artist name
             name = self.strip_disambiguation(a["name"])
-            discogs_id = a["id"]
+            discogs_id = str(a["id"])
             anv = a.get("anv", name)
             role = a.get("role", "").lower()
             # Check if the artist is Various
@@ -621,63 +625,41 @@ def _process_clean_tracklist(
         self,
         clean_tracklist: list[Track],
         albumartistinfo: ArtistInfo,
-    ) -> tuple[
-        list[TrackInfo],
-        dict[int, str],
-        int,
-        list[str],
-        list[str],
-        list[str | None],
-        list[str | None],
-    ]:
+    ) -> TracklistInfo:
         # Distinct works and intra-work divisions, as defined by index tracks.
-        tracks: list[TrackInfo] = []
-        mediums: list[str | None] = []
-        medium_indices: list[str | None] = []
-        index_tracks = {}
-        index = 0
-        divisions: list[str] = []
-        next_divisions: list[str] = []
+        t = TracklistInfo()
         for track in clean_tracklist:
             # Only real tracks have `position`. Otherwise, it's an index track.
             if track["position"]:
-                index += 1
-                if next_divisions:
+                t.index += 1
+                if t.next_divisions:
                     # End of a block of index tracks: update the current
                     # divisions.
-                    divisions += next_divisions
-                    del next_divisions[:]
+                    t.divisions += t.next_divisions
+                    del t.next_divisions[:]
                 track_info, medium, medium_index = self.get_track_info(
-                    track, index, divisions, albumartistinfo
+                    track, t.index, t.divisions, albumartistinfo
                 )
                 track_info.track_alt = track["position"]
-                tracks.append(track_info)
+                t.tracks.append(track_info)
                 if medium:
-                    mediums.append(medium)
+                    t.mediums.append(medium)
                 else:
-                    mediums.append(None)
+                    t.mediums.append(None)
                 if medium_index:
-                    medium_indices.append(medium_index)
+                    t.medium_indices.append(medium_index)
                 else:
-                    medium_indices.append(None)
+                    t.medium_indices.append(None)
             else:
-                next_divisions.append(track["title"])
+                t.next_divisions.append(track["title"])
                 # We expect new levels of division at the beginning of the
                 # tracklist (and possibly elsewhere).
                 try:
-                    divisions.pop()
+                    t.divisions.pop()
                 except IndexError:
                     pass
-                index_tracks[index + 1] = track["title"]
-        return (
-            tracks,
-            index_tracks,
-            index,
-            divisions,
-            next_divisions,
-            mediums,
-            medium_indices,
-        )
+                t.index_tracks[t.index + 1] = track["title"]
+        return t
 
     def get_tracks(
         self,
@@ -694,18 +676,9 @@ def get_tracks(
             self._log.debug("{}", traceback.format_exc())
             self._log.error("uncaught exception in coalesce_tracks: {}", exc)
             clean_tracklist = tracklist
-        processed = self._process_clean_tracklist(
+        t: TracklistInfo = self._process_clean_tracklist(
             clean_tracklist, albumartistinfo
         )
-        (
-            tracks,
-            index_tracks,
-            index,
-            divisions,
-            next_divisions,
-            mediums,
-            medium_indices,
-        ) = processed
         # Fix up medium and medium_index for each track. Discogs position is
         # unreliable, but tracks are in order.
         medium = None
@@ -714,22 +687,24 @@ def get_tracks(
 
         # If a medium has two sides (ie. vinyl or cassette), each pair of
         # consecutive sides should belong to the same medium.
-        if all([medium is not None for medium in mediums]):
-            m = sorted({medium.lower() if medium else "" for medium in mediums})
+        if all([medium is not None for medium in t.mediums]):
+            m = sorted(
+                {medium.lower() if medium else "" for medium in t.mediums}
+            )
             # If all track.medium are single consecutive letters, assume it is
             # a 2-sided medium.
             if "".join(m) in ascii_lowercase:
                 sides_per_medium = 2
 
-        for i, track in enumerate(tracks):
+        for i, track in enumerate(t.tracks):
             # Handle special case where a different medium does not indicate a
             # new disc, when there is no medium_index and the ordinal of medium
             # is not sequential. For example, I, II, III, IV, V. Assume these
             # are the track index, not the medium.
             # side_count is the number of mediums or medium sides (in the case
             # of two-sided mediums) that were seen before.
-            medium_str = mediums[i]
-            medium_index = medium_indices[i]
+            medium_str = t.mediums[i]
+            medium_index = t.medium_indices[i]
             medium_is_index = (
                 medium_str
                 and not medium_index
@@ -760,15 +735,15 @@ def get_tracks(
 
         # Get `disctitle` from Discogs index tracks. Assume that an index track
         # before the first track of each medium is a disc title.
-        for track in tracks:
+        for track in t.tracks:
             if track.medium_index == 1:
-                if track.index in index_tracks:
-                    disctitle = index_tracks[track.index]
+                if track.index in t.index_tracks:
+                    disctitle = t.index_tracks[track.index]
                 else:
                     disctitle = None
             track.disctitle = disctitle
 
-        return tracks
+        return t.tracks
 
     def coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]:
         """Pre-process a tracklist, merging subtracks into a single track. The

docs/changelog.rst

@@ -31,7 +31,7 @@ New features:
   no_convert, never_convert_lossy_files, same format, and max_bitrate
 - :doc:`plugins/titlecase`: Add the `titlecase` plugin to allow users to
       resolve differences in metadata source styles.
-- :doc:`plugins/discogs`: Added support for multi value fields. :bug:`6068`
+- :doc:`plugins/discogs`: Added support for multi-value fields. :bug:`6068`
 
 Bug fixes:
 
@@ -59,8 +59,8 @@ Bug fixes:
   "albumartist" instead of a list of unique album artists.
 - Sanitize log messages by removing control characters preventing terminal
   rendering issues.
-- :doc:`plugins/discogs`: Fixed unexpected flex attr from the Discogs plugin.
-  :bug:`6177`
+- :doc:`plugins/discogs`: Fixed an unexpected flexible attribute originating
+  from the Discogs plugin. :bug:`6177`
 
 For plugin developers:
 

test/plugins/test_discogs.py

@@ -410,8 +410,11 @@ def test_strip_disambiguation(self):
         d = DiscogsPlugin().get_album_info(release)
         assert d.artist == "ARTIST NAME & OTHER ARTIST"
         assert d.artists == ["ARTIST NAME", "OTHER ARTIST"]
+        assert d.artists_ids == ["321", "321"]
         assert d.tracks[0].artist == "TEST ARTIST"
         assert d.tracks[0].artists == ["TEST ARTIST"]
+        assert d.tracks[0].artist_id == "11146"
+        assert d.tracks[0].artists_ids == ["11146"]
         assert d.label == "LABEL NAME"
 
     def test_strip_disambiguation_false(self):
@@ -460,7 +463,7 @@ def test_strip_disambiguation_false(self):
 @pytest.mark.parametrize(
     "track_artist_anv,track_artist,track_artists",
     [
-        (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PEFORMER"]),
+        (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PERFORMER"]),
         (True, "VARIATION Feat. VARIATION", ["VARIATION", "VARIATION"]),
     ],
 )
@@ -480,7 +483,7 @@ def test_strip_disambiguation_false(self):
         (
             False,
             "ARTIST Feat. PERFORMER",
-            ["ARTIST", "PEFORMER"],
+            ["ARTIST", "PERFORMER"],
             "ARTIST & SOLOIST",
             ["ARTIST", "SOLOIST"],
         ),
@@ -551,9 +554,14 @@ def test_anv(
     config["discogs"]["anv"]["artist_credit"] = artist_credit_anv
     r = DiscogsPlugin().get_album_info(release)
     assert r.artist == album_artist
+    assert r.albumartists == album_artists
     assert r.artist_credit == album_artist_credit
+    assert r.albumartist_credit == album_artist_credit
+    assert r.albumartists_credit == album_artists_credit
     assert r.tracks[0].artist == track_artist
+    assert r.tracks[0].artists == track_artists
     assert r.tracks[0].artist_credit == track_artist_credit
+    assert r.tracks[0].artists_credit == track_artists_credit
 
 
 @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock())
@@ -590,6 +598,7 @@ def test_anv_album_artist():
     assert r.artists == ["ARTIST"]
     assert r.albumartist == "ARTIST"
     assert r.albumartist_credit == "ARTIST"
+    assert r.albumartist_id == "321"
     assert r.albumartists == ["ARTIST"]
     assert r.albumartists_credit == ["ARTIST"]
     assert r.artist_credit == "ARTIST"