From 9c8172be12fa13973f0f3ca8cd5f538ba0b1f38f Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Fri, 26 Sep 2025 01:55:28 +0200 Subject: [PATCH 01/16] Write initial ANV test --- test/plugins/test_discogs.py | 97 ++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 3652c0a54..a93c0da0b 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -520,6 +520,103 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): assert result == (expected_media, expected_albumtype) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +class DGTestAnv(BeetsTestCase): + @pytest.mark.parametrize( + "config_input, expected_output", + [ + ({ + "track_artist": False, + "album_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTIST NAME & SOLOIST" + "album_artist_credit": "ARTIST NAME & SOLOIST" + "track_arist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + }), + ({ + "album_artist": False, + "track_artist": True, + "artist_credit": False + }, + { + "album_artist": "ARTIST NAME & SOLOIST" + "album_artist_credit": "ARTIST NAME & SOLOIST" + "track_artist": "ARTY Feat. FORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + }), + ({ + "album_artist": True, + "track_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTY & SOLO" + "album_artist_credit": "ARTIST NAME & SOLOIST" + "track_arist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + }), + ({ + "album_artist": True, + "track_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTY & SOLO" + "album_artist_credit": "ARTIST & SOLOIST" + "track_arist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + }) + ({ + "album_artist": False, + "track_artist": False, + "artist_credit": True + }, + { + "album_artist": "ARTIST & SOLOIST" + "album_artist_credit": "ARTY & SOLO" + "track_arist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTY Feat. FORMER" + }) + ]) + def test_use_anv(self, config_input, expected_output): + config["discogs"]["album_artist_anv"] = config_input["album_artist"] + config["discogs"]["track_artist_anv"] = config_input["track_artist"] + config["discogs"]["artist_credit_anv"] = config_input["artist_credit"] + release = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + {"name": "ARTIST", "tracks": "", "anv": "ARTY", "id": 11146} + ], + "extraartists": [ + {"name": "PERFORMER", "role": "Featuring", "tracks": "", "anv": "FORMER", "id": 787} + ], + } + ], + "artists": [ + {"name": "ARTIST NAME", "anv": "ARTY", "id": 321, "join": "&"}, + {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, + ], + "title": "title", + } + d = DiscogsPlugin().get_album_info(release) + assert d.artist == expected_output["album_artist"] + assert d.artist_credit == expected_output["album_artist_credit"] + assert d.tracks[0].artist == expected_output["track_artist"] + assert d.tracks[0].artist_credit == expected_output["track_artist_credit"] + config["discogs"]["album_artist_anv"] = False + config["discogs"]["track_artist_anv"] = False + config["discogs"]["artist_credit_anv"] = False + @pytest.mark.parametrize( "position, medium, index, subindex", From 533aa6379bb3c5aa6e92da6d3d51f34c2f4979f4 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 25 Sep 2025 22:38:02 -0700 Subject: [PATCH 02/16] Test working, need to implement anv now --- beetsplug/discogs.py | 2 + test/plugins/test_discogs.py | 198 ++++++++++++++++++----------------- 2 files changed, 104 insertions(+), 96 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 3dc962464..ee432f11f 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -402,6 +402,7 @@ class DiscogsPlugin(MetadataSourcePlugin): album=album, album_id=album_id, artist=artist, + artist_credit=artist, artist_id=artist_id, tracks=tracks, albumtype=albumtype, @@ -668,6 +669,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return TrackInfo( title=title, track_id=track_id, + artist_credit=artist, artist=artist, artist_id=artist_id, length=length, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index a93c0da0b..a7d1c8407 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -521,102 +521,108 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): assert result == (expected_media, expected_albumtype) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) -class DGTestAnv(BeetsTestCase): - @pytest.mark.parametrize( - "config_input, expected_output", - [ - ({ - "track_artist": False, - "album_artist": False, - "artist_credit": False - }, - { - "album_artist": "ARTIST NAME & SOLOIST" - "album_artist_credit": "ARTIST NAME & SOLOIST" - "track_arist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - }), - ({ - "album_artist": False, - "track_artist": True, - "artist_credit": False - }, - { - "album_artist": "ARTIST NAME & SOLOIST" - "album_artist_credit": "ARTIST NAME & SOLOIST" - "track_artist": "ARTY Feat. FORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - }), - ({ - "album_artist": True, - "track_artist": False, - "artist_credit": False - }, - { - "album_artist": "ARTY & SOLO" - "album_artist_credit": "ARTIST NAME & SOLOIST" - "track_arist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - }), - ({ - "album_artist": True, - "track_artist": False, - "artist_credit": False - }, - { - "album_artist": "ARTY & SOLO" - "album_artist_credit": "ARTIST & SOLOIST" - "track_arist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - }) - ({ - "album_artist": False, - "track_artist": False, - "artist_credit": True - }, - { - "album_artist": "ARTIST & SOLOIST" - "album_artist_credit": "ARTY & SOLO" - "track_arist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTY Feat. FORMER" - }) - ]) - def test_use_anv(self, config_input, expected_output): - config["discogs"]["album_artist_anv"] = config_input["album_artist"] - config["discogs"]["track_artist_anv"] = config_input["track_artist"] - config["discogs"]["artist_credit_anv"] = config_input["artist_credit"] - release = { - "id": 123, - "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [ - { - "title": "track", - "position": "A", - "type_": "track", - "duration": "5:44", - "artists": [ - {"name": "ARTIST", "tracks": "", "anv": "ARTY", "id": 11146} - ], - "extraartists": [ - {"name": "PERFORMER", "role": "Featuring", "tracks": "", "anv": "FORMER", "id": 787} - ], - } - ], - "artists": [ - {"name": "ARTIST NAME", "anv": "ARTY", "id": 321, "join": "&"}, - {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, - ], - "title": "title", - } - d = DiscogsPlugin().get_album_info(release) - assert d.artist == expected_output["album_artist"] - assert d.artist_credit == expected_output["album_artist_credit"] - assert d.tracks[0].artist == expected_output["track_artist"] - assert d.tracks[0].artist_credit == expected_output["track_artist_credit"] - config["discogs"]["album_artist_anv"] = False - config["discogs"]["track_artist_anv"] = False - config["discogs"]["artist_credit_anv"] = False - +@pytest.mark.parametrize( + "config_input, expected_output", + [ + ({ + "track_artist": False, + "album_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTIST NAME & SOLOIST", + "album_artist_credit": "ARTIST NAME & SOLOIST", + "track_artist": "ARTIST feat. PERFORMER", + "track_artist_credit": "ARTIST feat. PERFORMER" + }), + ({ + "album_artist": False, + "track_artist": True, + "artist_credit": False + }, + { + "album_artist": "ARTIST NAME & SOLOIST", + "album_artist_credit": "ARTIST NAME & SOLOIST", + "track_artist": "ARTY feat. FORMER", + "track_artist_credit": "ARTIST feat. PERFORMER" + }), + ({ + "album_artist": True, + "track_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTY & SOLO", + "album_artist_credit": "ARTIST NAME & SOLOIST", + "track_artist": "ARTIST feat. PERFORMER", + "track_artist_credit": "ARTIST feat. PERFORMER" + }), + ({ + "album_artist": True, + "track_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTY & SOLO", + "album_artist_credit": "ARTIST NAME & SOLOIST", + "track_artist": "ARTIST feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + }), + ({ + "album_artist": False, + "track_artist": False, + "artist_credit": True + }, + { + "album_artist": "ARTIST NAME & SOLOIST", + "album_artist_credit": "ARTY & SOLO", + "track_artist": "ARTIST feat. PERFORMER", + "track_artist_credit": "ARTY Feat. FORMER" + }) + +]) +def test_use_anv(config_input, expected_output): + d = DiscogsPlugin() + d.config["album_artist_anv"] = config_input["album_artist"] + d.config["track_artist_anv"] = config_input["track_artist"] + d.config["artist_credit_anv"] = config_input["artist_credit"] + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [{ + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [{ + "name": "ARTIST", + "tracks": "", + "anv": "ARTY", + "id": 11146 + }], + "extraartists": [{ + "name": "PERFORMER", + "role": "Featuring", + "anv": "FORMER", + "id": 787 + }], + }], + "artists": [ + {"name": "ARTIST NAME", "anv": "ARTY", "id": 321, "join": "&"}, + {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + r = d.get_album_info(release) + assert r.artist == expected_output["album_artist"] + assert r.artist_credit == expected_output["album_artist_credit"] + assert r.tracks[0].artist == expected_output["track_artist"] + assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] @pytest.mark.parametrize( "position, medium, index, subindex", From 5a43d6add40abd00f63d3a4e2f9db71880696e32 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 26 Sep 2025 16:01:33 -0700 Subject: [PATCH 03/16] Testing and implemented anv method, also added Featuring customizable string --- beetsplug/discogs.py | 93 ++++++++++++------ test/plugins/test_discogs.py | 177 ++++++++++++++--------------------- 2 files changed, 135 insertions(+), 135 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index ee432f11f..727d8fbb7 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -97,8 +97,12 @@ class DiscogsPlugin(MetadataSourcePlugin): "user_token": "", "separator": ", ", "index_tracks": False, + "featured_label": "Feat.", "append_style_genre": False, "strip_disambiguation": True, + "album_artist_anv": False, + "track_artist_anv": False, + "artist_credit_anv": True } ) self.config["apikey"].redact = True @@ -302,6 +306,19 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype + def get_artist(self, artists, use_anv=False) -> tuple[str, str | None]: + """ Iterates through a discogs result, fetching data + if the artist anv is to be used, maps that to the name. + Calls the parent class get_artist method.""" + artist_data = [] + for artist in artists: + if use_anv and (anv := artist.get("anv", "")): + artist["name"] = anv + artist_data.append(artist) + artist, artist_id = super().get_artist( + artist_data, join_key="join") + return self.strip_disambiguation(artist), artist_id + def get_album_info(self, result): """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet @@ -330,8 +347,12 @@ class DiscogsPlugin(MetadataSourcePlugin): self._log.warning("Release does not contain the required fields") return None - artist, artist_id = self.get_artist( - [a.data for a in result.artists], join_key="join" + artist_data = [a.data for a in result.artists] + album_artist, album_artist_id = self.get_artist(artist_data, + self.config["album_artist_anv"] + ) + artist_credit, _ = self.get_artist(artist_data, + self.config["artist_credit_anv"] ) album = re.sub(r" +", " ", result.title) album_id = result.data["id"] @@ -339,7 +360,8 @@ class DiscogsPlugin(MetadataSourcePlugin): # convenient `.tracklist` property, which will strip out useful artist # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. - tracks = self.get_tracks(result.data["tracklist"], artist, artist_id) + tracks = self.get_tracks(result.data["tracklist"], (album_artist, album_artist_id, + artist_credit)) # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -376,9 +398,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # Additional cleanups # (various artists name, catalog number, media, disambiguation). if va: - artist = config["va_name"].as_str() - else: - artist = self.strip_disambiguation(artist) + album_artist, artist_credit = config["va_name"].as_str() if catalogno == "none": catalogno = None # Explicitly set the `media` for the tracks, since it is expected by @@ -401,9 +421,9 @@ class DiscogsPlugin(MetadataSourcePlugin): return AlbumInfo( album=album, album_id=album_id, - artist=artist, - artist_credit=artist, - artist_id=artist_id, + artist=album_artist, + artist_credit=artist_credit, + artist_id=album_artist_id, tracks=tracks, albumtype=albumtype, va=va, @@ -421,7 +441,7 @@ class DiscogsPlugin(MetadataSourcePlugin): data_url=data_url, discogs_albumid=discogs_albumid, discogs_labelid=labelid, - discogs_artistid=artist_id, + discogs_artistid=album_artist_id, cover_art_url=cover_art_url, ) @@ -443,7 +463,7 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks(self, tracklist, album_artist, album_artist_id): + def get_tracks(self, tracklist, album_artist_data): """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -469,7 +489,7 @@ class DiscogsPlugin(MetadataSourcePlugin): divisions += next_divisions del next_divisions[:] track_info = self.get_track_info( - track, index, divisions, album_artist, album_artist_id + track, index, divisions, album_artist_data ) track_info.track_alt = track["position"] tracks.append(track_info) @@ -638,9 +658,11 @@ class DiscogsPlugin(MetadataSourcePlugin): return DISAMBIGUATION_RE.sub("", text) def get_track_info( - self, track, index, divisions, album_artist, album_artist_id + self, track, index, divisions, album_artist_data ): """Returns a TrackInfo object for a discogs track.""" + album_artist, album_artist_id, artist_credit = album_artist_data + title = track["title"] if self.config["index_tracks"]: prefix = ", ".join(divisions) @@ -648,28 +670,39 @@ class DiscogsPlugin(MetadataSourcePlugin): title = f"{prefix}: {title}" track_id = None medium, medium_index, _ = self.get_track_index(track["position"]) - artist, artist_id = self.get_artist( - track.get("artists", []), join_key="join" - ) - # If no artist and artist is returned, set to match album artist - if not artist: - artist = album_artist - artist_id = album_artist_id + + artist = album_artist + artist_credit = album_artist + artist_id = album_artist_id + + # If artists are found on the track, we will use those instead + if (artists := track.get("artists", [])): + artist, artist_id = self.get_artist(artists, + self.config["track_artist_anv"] + ) + artist_credit, _ = self.get_artist(artists, + self.config["artist_credit_anv"] + ) length = self.get_track_length(track["duration"]) + # Add featured artists - extraartists = track.get("extraartists", []) - featured = [ - artist["name"] - for artist in extraartists - if "Featuring" in artist["role"] - ] - if featured: - artist = f"{artist} feat. {', '.join(featured)}" - artist = self.strip_disambiguation(artist) + if (extraartists := track.get("extraartists", [])): + featured_list = [ + artist for artist + in extraartists + if "Featuring" + in artist["role"]] + featured, _ = self.get_artist(featured_list, + self.config["track_artist_anv"]) + featured_credit, _ = self.get_artist(featured_list, + self.config["artist_credit_anv"]) + if featured: + artist = f"{artist} {self.config['featured_label']} {featured}" + artist_credit = f"{artist_credit} {self.config['featured_label']} {featured_credit}" return TrackInfo( title=title, track_id=track_id, - artist_credit=artist, + artist_credit=artist_credit, artist=artist, artist_id=artist_id, length=length, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index a7d1c8407..97c177736 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -452,6 +452,72 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.label == "LABEL NAME (5)" config["discogs"]["strip_disambiguation"] = True + def test_use_anv(self): + test_cases = [ + ({ + "track_artist": False, + "album_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTIST NAME & SOLOIST", + "album_artist_credit": "ARTIST NAME & SOLOIST", + "track_artist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + }), + ({ + "track_artist": True, + "album_artist": False, + "artist_credit": False + }, + { + "album_artist": "ARTIST NAME & SOLOIST", + "album_artist_credit": "ARTIST NAME & SOLOIST", + "track_artist": "ARTY Feat. FORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER" + })] + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [{ + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [{ + "name": "ARTIST", + "tracks": "", + "anv": "ARTY", + "id": 11146 + }], + "extraartists": [{ + "name": "PERFORMER", + "role": "Featuring", + "anv": "FORMER", + "id": 787 + }], + }], + "artists": [ + {"name": "ARTIST NAME", "anv": "ARTISTIC", "id": 321, "join": "&"}, + {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + for test_case in test_cases: + config_input, expected_output = test_case + r = DiscogsPlugin().get_album_info(release) + config["album_artist_anv"] = config_input["album_artist"] + config["track_artist_anv"] = config_input["track_artist"] + config["artist_credit_anv"] = config_input["artist_credit"] + assert r.artist == expected_output["album_artist"] + assert r.artist_credit == expected_output["album_artist_credit"] + assert r.tracks[0].artist == expected_output["track_artist"] + assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] @pytest.mark.parametrize( "track, expected_artist", @@ -469,23 +535,27 @@ class DGAlbumInfoTest(BeetsTestCase): "extraartists": [ { "name": "SOLOIST", + "id": 3, "role": "Featuring", }, { "name": "PERFORMER (1)", + "id": 5, "role": "Other Role, Featuring", }, { "name": "RANDOM", + "id": 8, "role": "Written-By", }, { "name": "MUSICIAN", + "id": 10, "role": "Featuring [Uncredited]", }, ], }, - "NEW ARTIST, VOCALIST feat. SOLOIST, PERFORMER, MUSICIAN", + "NEW ARTIST, VOCALIST Feat. SOLOIST, PERFORMER, MUSICIAN", ), ], ) @@ -494,7 +564,7 @@ def test_parse_featured_artists(track, expected_artist): """Tests the plugins ability to parse a featured artist. Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" - t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) + t = DiscogsPlugin().get_track_info(track, 1, 1, ("ARTIST", 2, "ARTIST CREDIT")) assert t.artist == expected_artist @@ -520,109 +590,6 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): assert result == (expected_media, expected_albumtype) -@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) -@pytest.mark.parametrize( - "config_input, expected_output", - [ - ({ - "track_artist": False, - "album_artist": False, - "artist_credit": False - }, - { - "album_artist": "ARTIST NAME & SOLOIST", - "album_artist_credit": "ARTIST NAME & SOLOIST", - "track_artist": "ARTIST feat. PERFORMER", - "track_artist_credit": "ARTIST feat. PERFORMER" - }), - ({ - "album_artist": False, - "track_artist": True, - "artist_credit": False - }, - { - "album_artist": "ARTIST NAME & SOLOIST", - "album_artist_credit": "ARTIST NAME & SOLOIST", - "track_artist": "ARTY feat. FORMER", - "track_artist_credit": "ARTIST feat. PERFORMER" - }), - ({ - "album_artist": True, - "track_artist": False, - "artist_credit": False - }, - { - "album_artist": "ARTY & SOLO", - "album_artist_credit": "ARTIST NAME & SOLOIST", - "track_artist": "ARTIST feat. PERFORMER", - "track_artist_credit": "ARTIST feat. PERFORMER" - }), - ({ - "album_artist": True, - "track_artist": False, - "artist_credit": False - }, - { - "album_artist": "ARTY & SOLO", - "album_artist_credit": "ARTIST NAME & SOLOIST", - "track_artist": "ARTIST feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - }), - ({ - "album_artist": False, - "track_artist": False, - "artist_credit": True - }, - { - "album_artist": "ARTIST NAME & SOLOIST", - "album_artist_credit": "ARTY & SOLO", - "track_artist": "ARTIST feat. PERFORMER", - "track_artist_credit": "ARTY Feat. FORMER" - }) - -]) -def test_use_anv(config_input, expected_output): - d = DiscogsPlugin() - d.config["album_artist_anv"] = config_input["album_artist"] - d.config["track_artist_anv"] = config_input["track_artist"] - d.config["artist_credit_anv"] = config_input["artist_credit"] - data = { - "id": 123, - "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [{ - "title": "track", - "position": "A", - "type_": "track", - "duration": "5:44", - "artists": [{ - "name": "ARTIST", - "tracks": "", - "anv": "ARTY", - "id": 11146 - }], - "extraartists": [{ - "name": "PERFORMER", - "role": "Featuring", - "anv": "FORMER", - "id": 787 - }], - }], - "artists": [ - {"name": "ARTIST NAME", "anv": "ARTY", "id": 321, "join": "&"}, - {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, - ], - "title": "title", - } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) - r = d.get_album_info(release) - assert r.artist == expected_output["album_artist"] - assert r.artist_credit == expected_output["album_artist_credit"] - assert r.tracks[0].artist == expected_output["track_artist"] - assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] @pytest.mark.parametrize( "position, medium, index, subindex", From 0ec668939534230d51475aeafe5be0db5efc8d01 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 26 Sep 2025 17:56:25 -0700 Subject: [PATCH 04/16] test updates, one case still failing --- beetsplug/discogs.py | 12 +++++------- test/plugins/test_discogs.py | 19 ++++++++++--------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 727d8fbb7..7082c4730 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -349,19 +349,17 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_data = [a.data for a in result.artists] album_artist, album_artist_id = self.get_artist(artist_data, - self.config["album_artist_anv"] - ) + self.config["album_artist_anv"]) artist_credit, _ = self.get_artist(artist_data, - self.config["artist_credit_anv"] - ) + self.config["artist_credit_anv"]) album = re.sub(r" +", " ", result.title) album_id = result.data["id"] # Use `.data` to access the tracklist directly instead of the # convenient `.tracklist` property, which will strip out useful artist # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. - tracks = self.get_tracks(result.data["tracklist"], (album_artist, album_artist_id, - artist_credit)) + tracks = self.get_tracks(result.data["tracklist"], + (album_artist, album_artist_id, artist_credit)) # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -672,7 +670,7 @@ class DiscogsPlugin(MetadataSourcePlugin): medium, medium_index, _ = self.get_track_index(track["position"]) artist = album_artist - artist_credit = album_artist + artist_credit = artist_credit artist_id = album_artist_id # If artists are found on the track, we will use those instead diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 97c177736..921007fb0 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -453,11 +453,12 @@ class DGAlbumInfoTest(BeetsTestCase): config["discogs"]["strip_disambiguation"] = True def test_use_anv(self): + """ Test using artist name variations. """ test_cases = [ ({ - "track_artist": False, - "album_artist": False, - "artist_credit": False + "track_artist_anv": False, + "album_artist_anv": False, + "artist_credit_anv": False }, { "album_artist": "ARTIST NAME & SOLOIST", @@ -466,9 +467,9 @@ class DGAlbumInfoTest(BeetsTestCase): "track_artist_credit": "ARTIST Feat. PERFORMER" }), ({ - "track_artist": True, - "album_artist": False, - "artist_credit": False + "track_artist_anv": True, + "album_artist_anv": False, + "artist_credit_anv": False }, { "album_artist": "ARTIST NAME & SOLOIST", @@ -510,10 +511,10 @@ class DGAlbumInfoTest(BeetsTestCase): ) for test_case in test_cases: config_input, expected_output = test_case + config["discogs"]["album_artist_anv"] = config_input["album_artist_anv"] + config["discogs"]["track_artist_anv"] = config_input["track_artist_anv"] + config["discogs"]["artist_credit_anv"] = config_input["artist_credit_anv"] r = DiscogsPlugin().get_album_info(release) - config["album_artist_anv"] = config_input["album_artist"] - config["track_artist_anv"] = config_input["track_artist"] - config["artist_credit_anv"] = config_input["artist_credit"] assert r.artist == expected_output["album_artist"] assert r.artist_credit == expected_output["album_artist_credit"] assert r.tracks[0].artist == expected_output["track_artist"] From 1e677d57c16abafb28180b963d44e16ea0e67c82 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 26 Sep 2025 21:37:18 -0700 Subject: [PATCH 05/16] Updates to documentation --- beetsplug/discogs.py | 28 ++--- docs/changelog.rst | 9 ++ test/plugins/test_discogs.py | 193 +++++++++++++++++++++++------------ 3 files changed, 145 insertions(+), 85 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 7082c4730..b3da89058 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -312,9 +312,10 @@ class DiscogsPlugin(MetadataSourcePlugin): Calls the parent class get_artist method.""" artist_data = [] for artist in artists: - if use_anv and (anv := artist.get("anv", "")): - artist["name"] = anv - artist_data.append(artist) + a = artist.copy() + if use_anv and (anv := a.get("anv", "")): + a["name"] = anv + artist_data.append(a) artist, artist_id = super().get_artist( artist_data, join_key="join") return self.strip_disambiguation(artist), artist_id @@ -659,7 +660,8 @@ class DiscogsPlugin(MetadataSourcePlugin): self, track, index, divisions, album_artist_data ): """Returns a TrackInfo object for a discogs track.""" - album_artist, album_artist_id, artist_credit = album_artist_data + + artist, artist_id, artist_credit = album_artist_data title = track["title"] if self.config["index_tracks"]: @@ -669,18 +671,10 @@ class DiscogsPlugin(MetadataSourcePlugin): track_id = None medium, medium_index, _ = self.get_track_index(track["position"]) - artist = album_artist - artist_credit = artist_credit - artist_id = album_artist_id - # If artists are found on the track, we will use those instead if (artists := track.get("artists", [])): - artist, artist_id = self.get_artist(artists, - self.config["track_artist_anv"] - ) - artist_credit, _ = self.get_artist(artists, - self.config["artist_credit_anv"] - ) + artist, artist_id = self.get_artist(artists, self.config["track_artist_anv"]) + artist_credit, _ = self.get_artist(artists, self.config["artist_credit_anv"]) length = self.get_track_length(track["duration"]) # Add featured artists @@ -690,10 +684,8 @@ class DiscogsPlugin(MetadataSourcePlugin): in extraartists if "Featuring" in artist["role"]] - featured, _ = self.get_artist(featured_list, - self.config["track_artist_anv"]) - featured_credit, _ = self.get_artist(featured_list, - self.config["artist_credit_anv"]) + featured, _ = self.get_artist(featured_list, self.config["track_artist_anv"]) + featured_credit, _ = self.get_artist(featured_list, self.config["artist_credit_anv"]) if featured: artist = f"{artist} {self.config['featured_label']} {featured}" artist_credit = f"{artist_credit} {self.config['featured_label']} {featured_credit}" diff --git a/docs/changelog.rst b/docs/changelog.rst index 1b6dae5e2..aafbc5030 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,15 @@ New features: - :doc:`plugins/discogs`: New config option `strip_disambiguation` to toggle stripping discogs numeric disambiguation on artist and label fields. - :doc:`plugins/discogs` Added support for featured artists. +- :doc:`plugins/discogs` New configuration option `featured_label` to change the + default string used to join featured artists. The default string is `Feat.` +- :doc:`plugins/discogs` Added support for `artist_credit` in Discogs tags +- :doc:`plugins/discogs` Added support for Discogs artist name variations. + Three new boolean configuration options specify where the variations are written, + if at all. `album_artist_anv` writes variations to the album artist tag. + `track_artist_anv` writes to a tracks artist field. `artist_credit_anv` writes + to the `artist_credit` field on both albums and tracks. + Bug fixes: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 921007fb0..34a95d9e4 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -452,73 +452,132 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.label == "LABEL NAME (5)" config["discogs"]["strip_disambiguation"] = True - def test_use_anv(self): - """ Test using artist name variations. """ - test_cases = [ - ({ - "track_artist_anv": False, - "album_artist_anv": False, - "artist_credit_anv": False - }, - { - "album_artist": "ARTIST NAME & SOLOIST", - "album_artist_credit": "ARTIST NAME & SOLOIST", - "track_artist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - }), - ({ - "track_artist_anv": True, - "album_artist_anv": False, - "artist_credit_anv": False - }, - { - "album_artist": "ARTIST NAME & SOLOIST", - "album_artist_credit": "ARTIST NAME & SOLOIST", - "track_artist": "ARTY Feat. FORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER" - })] - data = { - "id": 123, - "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [{ - "title": "track", - "position": "A", - "type_": "track", - "duration": "5:44", - "artists": [{ - "name": "ARTIST", - "tracks": "", - "anv": "ARTY", - "id": 11146 - }], - "extraartists": [{ - "name": "PERFORMER", - "role": "Featuring", - "anv": "FORMER", - "id": 787 - }], - }], - "artists": [ - {"name": "ARTIST NAME", "anv": "ARTISTIC", "id": 321, "join": "&"}, - {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, - ], - "title": "title", - } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) - for test_case in test_cases: - config_input, expected_output = test_case - config["discogs"]["album_artist_anv"] = config_input["album_artist_anv"] - config["discogs"]["track_artist_anv"] = config_input["track_artist_anv"] - config["discogs"]["artist_credit_anv"] = config_input["artist_credit_anv"] - r = DiscogsPlugin().get_album_info(release) - assert r.artist == expected_output["album_artist"] - assert r.artist_credit == expected_output["album_artist_credit"] - assert r.tracks[0].artist == expected_output["track_artist"] - assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] + + +@pytest.mark.parametrize( + "config_input,expected_output", + [ + ({ + "track_artist_anv": False, + "album_artist_anv": False, + "artist_credit_anv": False + }, + { + "track_artist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER", + "album_artist": "ARTIST & SOLOIST", + "album_artist_credit": "ARTIST & SOLOIST", + }), + ({ + "track_artist_anv": True, + "album_artist_anv": False, + "artist_credit_anv": False + }, + { + "track_artist": "VARIATION Feat. VARIATION", + "track_artist_credit": "ARTIST Feat. PERFORMER", + "album_artist": "ARTIST & SOLOIST", + "album_artist_credit": "ARTIST & SOLOIST", + }), + ({ + "track_artist_anv": True, + "album_artist_anv": True, + "artist_credit_anv": False + }, + { + "track_artist": "VARIATION Feat. VARIATION", + "track_artist_credit": "ARTIST Feat. PERFORMER", + "album_artist": "VARIATION & VARIATION", + "album_artist_credit": "ARTIST & SOLOIST", + }), + ({ + "track_artist_anv": True, + "album_artist_anv": True, + "artist_credit_anv": True + }, + { + "track_artist": "VARIATION Feat. VARIATION", + "track_artist_credit": "VARIATION Feat. VARIATION", + "album_artist": "VARIATION & VARIATION", + "album_artist_credit": "VARIATION & VARIATION", + }) +]) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_anv(config_input, expected_output): + """ Test using artist name variations. """ + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [{ + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [{ + "name": "ARTIST", + "tracks": "", + "anv": "VARIATION", + "id": 11146 + }], + "extraartists": [{ + "name": "PERFORMER", + "role": "Featuring", + "anv": "VARIATION", + "id": 787 + }], + }], + "artists": [ + {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321, "join": "&"}, + {"name": "SOLOIST", "anv": "VARIATION", "id": 445, "join": ""}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + config["discogs"]["album_artist_anv"] = config_input["album_artist_anv"] + config["discogs"]["track_artist_anv"] = config_input["track_artist_anv"] + config["discogs"]["artist_credit_anv"] = config_input["artist_credit_anv"] + r = DiscogsPlugin().get_album_info(release) + assert r.artist == expected_output["album_artist"] + assert r.artist_credit == expected_output["album_artist_credit"] + assert r.tracks[0].artist == expected_output["track_artist"] + assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] + +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_anv_album_artist(): + """ Test using artist name variations when the album artist + is the same as the track artist, but only the track artist + should use the artist name variation.""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [{ + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + }], + "artists": [ + {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + config["discogs"]["album_artist_anv"] = False + config["discogs"]["track_artist_anv"] = True + config["discogs"]["artist_credit_anv"] = False + r = DiscogsPlugin().get_album_info(release) + assert r.artist == "ARTIST" + assert r.artist_credit == "ARTIST" + assert r.tracks[0].artist == "VARIATION" + assert r.tracks[0].artist_credit == "ARTIST" @pytest.mark.parametrize( "track, expected_artist", From dd57c0da2d8fb5a0ecfb1be0b8b767e1eb33ec3e Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 27 Sep 2025 10:42:29 -0700 Subject: [PATCH 06/16] improve flexibility of use of anv on artist tracks --- beetsplug/discogs.py | 23 +++++++++++++++++------ test/plugins/test_discogs.py | 2 +- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index b3da89058..0eaef2227 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -349,10 +349,10 @@ class DiscogsPlugin(MetadataSourcePlugin): return None artist_data = [a.data for a in result.artists] - album_artist, album_artist_id = self.get_artist(artist_data, - self.config["album_artist_anv"]) - artist_credit, _ = self.get_artist(artist_data, - self.config["artist_credit_anv"]) + album_artist, album_artist_id = self.get_artist(artist_data) + album_artist_anv, _ = self.get_artist(artist_data, use_anv=True) + artist_credit = album_artist_anv + album = re.sub(r" +", " ", result.title) album_id = result.data["id"] # Use `.data` to access the tracklist directly instead of the @@ -360,7 +360,13 @@ class DiscogsPlugin(MetadataSourcePlugin): # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. tracks = self.get_tracks(result.data["tracklist"], - (album_artist, album_artist_id, artist_credit)) + (album_artist, album_artist_anv, album_artist_id)) + + # Assign ANV to the proper fields for tagging + if not self.config["artist_credit_anv"]: + artist_credit = album_artist + if self.config["album_artist_anv"]: + album_artist = album_artist_anv # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -661,7 +667,12 @@ class DiscogsPlugin(MetadataSourcePlugin): ): """Returns a TrackInfo object for a discogs track.""" - artist, artist_id, artist_credit = album_artist_data + artist, artist_anv, artist_id = album_artist_data + artist_credit = artist_anv + if not self.config["artist_credit_anv"]: + artist_credit = artist + if self.config["track_artist_anv"]: + artist = artist_anv title = track["title"] if self.config["index_tracks"]: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 34a95d9e4..6dab169ca 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -624,7 +624,7 @@ def test_parse_featured_artists(track, expected_artist): """Tests the plugins ability to parse a featured artist. Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" - t = DiscogsPlugin().get_track_info(track, 1, 1, ("ARTIST", 2, "ARTIST CREDIT")) + t = DiscogsPlugin().get_track_info(track, 1, 1, ("ARTIST", "ARTIST CREDIT", 2)) assert t.artist == expected_artist From b1903417f40e93a79bc1f88f3398882536e1cb63 Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 27 Sep 2025 14:29:25 -0700 Subject: [PATCH 07/16] Add artist credit support, artist name variation support, more flexible featured credit. --- beetsplug/discogs.py | 53 +++++++------ docs/changelog.rst | 24 +++--- docs/plugins/discogs.rst | 8 ++ test/plugins/test_discogs.py | 142 +++++++++++++++++------------------ 4 files changed, 117 insertions(+), 110 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0eaef2227..0d796e7b7 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -97,12 +97,12 @@ class DiscogsPlugin(MetadataSourcePlugin): "user_token": "", "separator": ", ", "index_tracks": False, - "featured_label": "Feat.", + "featured_string": "Feat.", "append_style_genre": False, "strip_disambiguation": True, "album_artist_anv": False, "track_artist_anv": False, - "artist_credit_anv": True + "artist_credit_anv": True, } ) self.config["apikey"].redact = True @@ -307,7 +307,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype def get_artist(self, artists, use_anv=False) -> tuple[str, str | None]: - """ Iterates through a discogs result, fetching data + """Iterates through a discogs result, fetching data if the artist anv is to be used, maps that to the name. Calls the parent class get_artist method.""" artist_data = [] @@ -316,8 +316,7 @@ class DiscogsPlugin(MetadataSourcePlugin): if use_anv and (anv := a.get("anv", "")): a["name"] = anv artist_data.append(a) - artist, artist_id = super().get_artist( - artist_data, join_key="join") + artist, artist_id = super().get_artist(artist_data, join_key="join") return self.strip_disambiguation(artist), artist_id def get_album_info(self, result): @@ -359,8 +358,10 @@ class DiscogsPlugin(MetadataSourcePlugin): # convenient `.tracklist` property, which will strip out useful artist # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. - tracks = self.get_tracks(result.data["tracklist"], - (album_artist, album_artist_anv, album_artist_id)) + tracks = self.get_tracks( + result.data["tracklist"], + (album_artist, album_artist_anv, album_artist_id), + ) # Assign ANV to the proper fields for tagging if not self.config["artist_credit_anv"]: @@ -662,9 +663,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info( - self, track, index, divisions, album_artist_data - ): + def get_track_info(self, track, index, divisions, album_artist_data): """Returns a TrackInfo object for a discogs track.""" artist, artist_anv, artist_id = album_artist_data @@ -683,23 +682,33 @@ class DiscogsPlugin(MetadataSourcePlugin): medium, medium_index, _ = self.get_track_index(track["position"]) # If artists are found on the track, we will use those instead - if (artists := track.get("artists", [])): - artist, artist_id = self.get_artist(artists, self.config["track_artist_anv"]) - artist_credit, _ = self.get_artist(artists, self.config["artist_credit_anv"]) + if artists := track.get("artists", []): + artist, artist_id = self.get_artist( + artists, self.config["track_artist_anv"] + ) + artist_credit, _ = self.get_artist( + artists, self.config["artist_credit_anv"] + ) length = self.get_track_length(track["duration"]) # Add featured artists - if (extraartists := track.get("extraartists", [])): + if extraartists := track.get("extraartists", []): featured_list = [ - artist for artist - in extraartists - if "Featuring" - in artist["role"]] - featured, _ = self.get_artist(featured_list, self.config["track_artist_anv"]) - featured_credit, _ = self.get_artist(featured_list, self.config["artist_credit_anv"]) + artist + for artist in extraartists + if "Featuring" in artist["role"] + ] + featured, _ = self.get_artist( + featured_list, self.config["track_artist_anv"] + ) + featured_credit, _ = self.get_artist( + featured_list, self.config["artist_credit_anv"] + ) if featured: - artist = f"{artist} {self.config['featured_label']} {featured}" - artist_credit = f"{artist_credit} {self.config['featured_label']} {featured_credit}" + artist += f" {self.config['featured_string']} {featured}" + artist_credit += ( + f" {self.config['featured_string']} {featured_credit}" + ) return TrackInfo( title=title, track_id=track_id, diff --git a/docs/changelog.rst b/docs/changelog.rst index ffb916a8d..9476c7ad8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,16 +15,14 @@ New features: converted files. - :doc:`plugins/discogs`: New config option `strip_disambiguation` to toggle stripping discogs numeric disambiguation on artist and label fields. -- :doc:`plugins/discogs` Added support for featured artists. -- :doc:`plugins/discogs` New configuration option `featured_label` to change the - default string used to join featured artists. The default string is `Feat.` -- :doc:`plugins/discogs` Added support for `artist_credit` in Discogs tags -- :doc:`plugins/discogs` Added support for Discogs artist name variations. - Three new boolean configuration options specify where the variations are written, - if at all. `album_artist_anv` writes variations to the album artist tag. - `track_artist_anv` writes to a tracks artist field. `artist_credit_anv` writes - to the `artist_credit` field on both albums and tracks. - +- :doc:`plugins/discogs` Added support for featured artists. :bug:`6038` +- :doc:`plugins/discogs` New configuration option `featured_string` to change + the default string used to join featured artists. The default string is + `Feat.`. +- :doc:`plugins/discogs` Support for `artist_credit` in Discogs tags. + :bug:`3354` +- :doc:`plugins/discogs` Support for name variations and config options to + specify where the variations are written. :bug:`3354` Bug fixes: @@ -35,12 +33,10 @@ Bug fixes: - :doc:`plugins/spotify` Removed old and undocumented config options `artist_field`, `album_field` and `track` that were causing issues with track matching. :bug:`5189` -- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from - artists but not labels. :bug:`5366` -- :doc:`plugins/discogs` Fixed issue with ignoring featured artists in the - extraartists field. - :doc:`plugins/spotify` Fixed an issue where candidate lookup would not find matches due to query escaping (single vs double quotes). +- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from + artists but not labels. :bug:`5366` For packagers: diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 3dac558bd..0446685d7 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -112,6 +112,14 @@ Other configurations available under ``discogs:`` are: - **strip_disambiguation**: Discogs uses strings like ``"(4)"`` to mark distinct artists and labels with the same name. If you'd like to use the discogs disambiguation in your tags, you can disable it. Default: ``True`` +- **featured_string**: Configure the string used for noting featured artists. + Useful if you prefer ``Featuring`` or ``ft.``. Default: ``Feat.`` +- **artist_credit_anv**: Whether the Artist Name Variation (ANV) should be + applied to the artist credit tag. Default: ``True`` +- **track_artist_anv**: Same as ``artist_credit_anv`` for a track's artist + field. Default: ``False`` +- **album_artist_anv**: As previous, but for the album artist. Default: + ``False`` .. _discogs guidelines: https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 6dab169ca..439cd3585 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -453,85 +453,73 @@ class DGAlbumInfoTest(BeetsTestCase): config["discogs"]["strip_disambiguation"] = True - @pytest.mark.parametrize( - "config_input,expected_output", - [ - ({ - "track_artist_anv": False, - "album_artist_anv": False, - "artist_credit_anv": False - }, - { - "track_artist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER", - "album_artist": "ARTIST & SOLOIST", - "album_artist_credit": "ARTIST & SOLOIST", - }), - ({ - "track_artist_anv": True, - "album_artist_anv": False, - "artist_credit_anv": False - }, - { - "track_artist": "VARIATION Feat. VARIATION", - "track_artist_credit": "ARTIST Feat. PERFORMER", - "album_artist": "ARTIST & SOLOIST", - "album_artist_credit": "ARTIST & SOLOIST", - }), - ({ - "track_artist_anv": True, - "album_artist_anv": True, - "artist_credit_anv": False - }, - { - "track_artist": "VARIATION Feat. VARIATION", - "track_artist_credit": "ARTIST Feat. PERFORMER", - "album_artist": "VARIATION & VARIATION", - "album_artist_credit": "ARTIST & SOLOIST", - }), - ({ - "track_artist_anv": True, - "album_artist_anv": True, - "artist_credit_anv": True - }, - { - "track_artist": "VARIATION Feat. VARIATION", - "track_artist_credit": "VARIATION Feat. VARIATION", - "album_artist": "VARIATION & VARIATION", - "album_artist_credit": "VARIATION & VARIATION", - }) -]) + "config_input,expected_output", + [ + ( + { + "track_artist_anv": False, + "album_artist_anv": False, + "artist_credit_anv": False, + }, + { + "track_artist": "ARTIST Feat. PERFORMER", + "track_artist_credit": "ARTIST Feat. PERFORMER", + "album_artist": "ARTIST & SOLOIST", + "album_artist_credit": "ARTIST & SOLOIST", + }, + ), + ( + { + "track_artist_anv": True, + "album_artist_anv": True, + "artist_credit_anv": True, + }, + { + "track_artist": "VARIATION Feat. VARIATION", + "track_artist_credit": "VARIATION Feat. VARIATION", + "album_artist": "VARIATION & VARIATION", + "album_artist_credit": "VARIATION & VARIATION", + }, + ), + ], +) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_anv(config_input, expected_output): - """ Test using artist name variations. """ + """Test using artist name variations.""" data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [{ + "tracklist": [ + { "title": "track", "position": "A", "type_": "track", "duration": "5:44", - "artists": [{ - "name": "ARTIST", - "tracks": "", - "anv": "VARIATION", - "id": 11146 - }], - "extraartists": [{ - "name": "PERFORMER", - "role": "Featuring", - "anv": "VARIATION", - "id": 787 - }], - }], + "artists": [ + { + "name": "ARTIST", + "tracks": "", + "anv": "VARIATION", + "id": 11146, + } + ], + "extraartists": [ + { + "name": "PERFORMER", + "role": "Featuring", + "anv": "VARIATION", + "id": 787, + } + ], + } + ], "artists": [ {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321, "join": "&"}, {"name": "SOLOIST", "anv": "VARIATION", "id": 445, "join": ""}, ], "title": "title", - } + } release = Bag( data=data, title=data["title"], @@ -546,25 +534,28 @@ def test_anv(config_input, expected_output): assert r.tracks[0].artist == expected_output["track_artist"] assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] + @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_anv_album_artist(): - """ Test using artist name variations when the album artist - is the same as the track artist, but only the track artist + """Test using artist name variations when the album artist + is the same as the track artist, but only the track artist should use the artist name variation.""" data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [{ + "tracklist": [ + { "title": "track", "position": "A", "type_": "track", "duration": "5:44", - }], + } + ], "artists": [ {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321}, ], "title": "title", - } + } release = Bag( data=data, title=data["title"], @@ -579,6 +570,7 @@ def test_anv_album_artist(): assert r.tracks[0].artist == "VARIATION" assert r.tracks[0].artist_credit == "ARTIST" + @pytest.mark.parametrize( "track, expected_artist", [ @@ -595,22 +587,22 @@ def test_anv_album_artist(): "extraartists": [ { "name": "SOLOIST", - "id": 3, + "id": 3, "role": "Featuring", }, { "name": "PERFORMER (1)", - "id": 5, + "id": 5, "role": "Other Role, Featuring", }, { "name": "RANDOM", - "id": 8, + "id": 8, "role": "Written-By", }, { "name": "MUSICIAN", - "id": 10, + "id": 10, "role": "Featuring [Uncredited]", }, ], @@ -624,7 +616,9 @@ def test_parse_featured_artists(track, expected_artist): """Tests the plugins ability to parse a featured artist. Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" - t = DiscogsPlugin().get_track_info(track, 1, 1, ("ARTIST", "ARTIST CREDIT", 2)) + t = DiscogsPlugin().get_track_info( + track, 1, 1, ("ARTIST", "ARTIST CREDIT", 2) + ) assert t.artist == expected_artist From 9efe728f47a439cfdb92c35707ed8771c3298a50 Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 27 Sep 2025 14:49:56 -0700 Subject: [PATCH 08/16] type checking, tuple unpacking fix in various artists --- beetsplug/discogs.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0d796e7b7..ec8638589 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -306,7 +306,9 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype - def get_artist(self, artists, use_anv=False) -> tuple[str, str | None]: + def get_artist( + self, artists: Iterable[dict[str | int, str]], use_anv: bool = False + ) -> tuple[str, str | None]: """Iterates through a discogs result, fetching data if the artist anv is to be used, maps that to the name. Calls the parent class get_artist method.""" @@ -404,7 +406,9 @@ class DiscogsPlugin(MetadataSourcePlugin): # Additional cleanups # (various artists name, catalog number, media, disambiguation). if va: - album_artist, artist_credit = config["va_name"].as_str() + va_name = config["va_name"].as_str() + album_artist = va_name + artist_credit = va_name if catalogno == "none": catalogno = None # Explicitly set the `media` for the tracks, since it is expected by From abc8c2d5d8e2ef11081fadd768b89aae5b9a4718 Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 27 Sep 2025 15:05:14 -0700 Subject: [PATCH 09/16] resolve overriding method type error --- beetsplug/discogs.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index ec8638589..3c42a5621 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -306,19 +306,19 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype - def get_artist( + def get_artist_with_anv( self, artists: Iterable[dict[str | int, str]], use_anv: bool = False ) -> tuple[str, str | None]: """Iterates through a discogs result, fetching data if the artist anv is to be used, maps that to the name. Calls the parent class get_artist method.""" - artist_data = [] - for artist in artists: - a = artist.copy() + artist_list = [] + for artist_data in artists: + a = artist_data.copy() if use_anv and (anv := a.get("anv", "")): a["name"] = anv - artist_data.append(a) - artist, artist_id = super().get_artist(artist_data, join_key="join") + artist_list.append(a) + artist, artist_id = self.get_artist(artist_list, join_key="join") return self.strip_disambiguation(artist), artist_id def get_album_info(self, result): @@ -350,8 +350,10 @@ class DiscogsPlugin(MetadataSourcePlugin): return None artist_data = [a.data for a in result.artists] - album_artist, album_artist_id = self.get_artist(artist_data) - album_artist_anv, _ = self.get_artist(artist_data, use_anv=True) + album_artist, album_artist_id = self.get_artist_with_anv(artist_data) + album_artist_anv, _ = self.get_artist_with_anv( + artist_data, use_anv=True + ) artist_credit = album_artist_anv album = re.sub(r" +", " ", result.title) @@ -687,10 +689,10 @@ class DiscogsPlugin(MetadataSourcePlugin): # If artists are found on the track, we will use those instead if artists := track.get("artists", []): - artist, artist_id = self.get_artist( + artist, artist_id = self.get_artist_with_anv( artists, self.config["track_artist_anv"] ) - artist_credit, _ = self.get_artist( + artist_credit, _ = self.get_artist_with_anv( artists, self.config["artist_credit_anv"] ) length = self.get_track_length(track["duration"]) @@ -702,10 +704,10 @@ class DiscogsPlugin(MetadataSourcePlugin): for artist in extraartists if "Featuring" in artist["role"] ] - featured, _ = self.get_artist( + featured, _ = self.get_artist_with_anv( featured_list, self.config["track_artist_anv"] ) - featured_credit, _ = self.get_artist( + featured_credit, _ = self.get_artist_with_anv( featured_list, self.config["artist_credit_anv"] ) if featured: From c44c535b22339d6cec210b0dd8bea01da90be21d Mon Sep 17 00:00:00 2001 From: Henry Date: Sun, 28 Sep 2025 10:49:56 -0700 Subject: [PATCH 10/16] Fully parametrize testing --- test/plugins/test_discogs.py | 62 ++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 439cd3585..40dd30e53 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -454,38 +454,30 @@ class DGAlbumInfoTest(BeetsTestCase): @pytest.mark.parametrize( - "config_input,expected_output", + "track_artist_anv,track_artist", + [(False, "ARTIST Feat. PERFORMER"), (True, "VARIATION Feat. VARIATION")], +) +@pytest.mark.parametrize( + "album_artist_anv,album_artist", + [(False, "ARTIST & SOLOIST"), (True, "VARIATION & VARIATION")], +) +@pytest.mark.parametrize( + "artist_credit_anv,track_artist_credit,album_artist_credit", [ - ( - { - "track_artist_anv": False, - "album_artist_anv": False, - "artist_credit_anv": False, - }, - { - "track_artist": "ARTIST Feat. PERFORMER", - "track_artist_credit": "ARTIST Feat. PERFORMER", - "album_artist": "ARTIST & SOLOIST", - "album_artist_credit": "ARTIST & SOLOIST", - }, - ), - ( - { - "track_artist_anv": True, - "album_artist_anv": True, - "artist_credit_anv": True, - }, - { - "track_artist": "VARIATION Feat. VARIATION", - "track_artist_credit": "VARIATION Feat. VARIATION", - "album_artist": "VARIATION & VARIATION", - "album_artist_credit": "VARIATION & VARIATION", - }, - ), + (False, "ARTIST Feat. PERFORMER", "ARTIST & SOLOIST"), + (True, "VARIATION Feat. VARIATION", "VARIATION & VARIATION"), ], ) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) -def test_anv(config_input, expected_output): +def test_anv( + track_artist_anv, + track_artist, + album_artist_anv, + album_artist, + artist_credit_anv, + track_artist_credit, + album_artist_credit, +): """Test using artist name variations.""" data = { "id": 123, @@ -525,14 +517,14 @@ def test_anv(config_input, expected_output): title=data["title"], artists=[Bag(data=d) for d in data["artists"]], ) - config["discogs"]["album_artist_anv"] = config_input["album_artist_anv"] - config["discogs"]["track_artist_anv"] = config_input["track_artist_anv"] - config["discogs"]["artist_credit_anv"] = config_input["artist_credit_anv"] + config["discogs"]["album_artist_anv"] = album_artist_anv + config["discogs"]["track_artist_anv"] = track_artist_anv + config["discogs"]["artist_credit_anv"] = artist_credit_anv r = DiscogsPlugin().get_album_info(release) - assert r.artist == expected_output["album_artist"] - assert r.artist_credit == expected_output["album_artist_credit"] - assert r.tracks[0].artist == expected_output["track_artist"] - assert r.tracks[0].artist_credit == expected_output["track_artist_credit"] + assert r.artist == album_artist + assert r.artist_credit == album_artist_credit + assert r.tracks[0].artist == track_artist + assert r.tracks[0].artist_credit == track_artist_credit @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) From fcebe8123a455b172c8b4fed41d39c2d92ad33d8 Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 30 Sep 2025 20:01:35 -0700 Subject: [PATCH 11/16] Expand documentation --- docs/plugins/discogs.rst | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 0446685d7..43b60148f 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -96,6 +96,11 @@ whereas with ``index_tracks`` disabled you'd get: This option is useful when importing classical music. +### Handling Artist Name Variations (ANVs) + +An ANV is an alternate way that an artist may be credited on a release. If the +band name changes or is misspelled on different releases. The artist name ac + Other configurations available under ``discogs:`` are: - **append_style_genre**: Appends the Discogs style (if found) to the genre tag. @@ -114,12 +119,20 @@ Other configurations available under ``discogs:`` are: disambiguation in your tags, you can disable it. Default: ``True`` - **featured_string**: Configure the string used for noting featured artists. Useful if you prefer ``Featuring`` or ``ft.``. Default: ``Feat.`` -- **artist_credit_anv**: Whether the Artist Name Variation (ANV) should be - applied to the artist credit tag. Default: ``True`` -- **track_artist_anv**: Same as ``artist_credit_anv`` for a track's artist - field. Default: ``False`` -- **album_artist_anv**: As previous, but for the album artist. Default: - ``False`` +- **artist_credit_anv**, **track_artist_anv**, **album_artist_anv**: These + configuration option are dedicated to handling Arist Name Variations (ANVs). + Sometimes a release credits artists differently compared to the majority of + their work. For example, "Basement Jaxx" may be credited as "Tha Jaxx" or "The + Basement Jaxx". By default, the Discogs plugin stores ANVs in the + ``artist_credit`` field. You can select any combination of these three to + control where beets writes and stores the variation credit. + + - **artist_credit_anv**: Write ANV to the ``artist_credit`` field. + Default: ``True`` + - **track_artist_anv**: Write ANV to the ``artist`` field. Default: + ``False`` + - **album_artist_anv**: Write ANV to the ``album_artist`` field. Default: + ``False`` .. _discogs guidelines: https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings From f5acdec2b104b84b0b02000a2bacfc0962a4965f Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 3 Oct 2025 14:44:22 -0700 Subject: [PATCH 12/16] Update configuration format. --- beetsplug/discogs.py | 24 +++++++++++++----------- docs/plugins/discogs.rst | 31 +++++++++++++------------------ test/plugins/test_discogs.py | 12 ++++++------ 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 3c42a5621..ef479f686 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -100,9 +100,11 @@ class DiscogsPlugin(MetadataSourcePlugin): "featured_string": "Feat.", "append_style_genre": False, "strip_disambiguation": True, - "album_artist_anv": False, - "track_artist_anv": False, - "artist_credit_anv": True, + "anv": { + "artist_credit": True, + "artist": False, + "album_artist": False, + }, } ) self.config["apikey"].redact = True @@ -368,9 +370,9 @@ class DiscogsPlugin(MetadataSourcePlugin): ) # Assign ANV to the proper fields for tagging - if not self.config["artist_credit_anv"]: + if not self.config["anv"]["artist_credit"]: artist_credit = album_artist - if self.config["album_artist_anv"]: + if self.config["anv"]["album_artist"]: album_artist = album_artist_anv # Extract information for the optional AlbumInfo fields, if possible. @@ -674,9 +676,9 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_anv, artist_id = album_artist_data artist_credit = artist_anv - if not self.config["artist_credit_anv"]: + if not self.config["anv"]["artist_credit"]: artist_credit = artist - if self.config["track_artist_anv"]: + if self.config["anv"]["artist"]: artist = artist_anv title = track["title"] @@ -690,10 +692,10 @@ class DiscogsPlugin(MetadataSourcePlugin): # If artists are found on the track, we will use those instead if artists := track.get("artists", []): artist, artist_id = self.get_artist_with_anv( - artists, self.config["track_artist_anv"] + artists, self.config["anv"]["artist"] ) artist_credit, _ = self.get_artist_with_anv( - artists, self.config["artist_credit_anv"] + artists, self.config["anv"]["artist_credit"] ) length = self.get_track_length(track["duration"]) @@ -705,10 +707,10 @@ class DiscogsPlugin(MetadataSourcePlugin): if "Featuring" in artist["role"] ] featured, _ = self.get_artist_with_anv( - featured_list, self.config["track_artist_anv"] + featured_list, self.config["anv"]["artist"] ) featured_credit, _ = self.get_artist_with_anv( - featured_list, self.config["artist_credit_anv"] + featured_list, self.config["anv"]["artist_credit"] ) if featured: artist += f" {self.config['featured_string']} {featured}" diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 43b60148f..0d55630c4 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -96,11 +96,6 @@ whereas with ``index_tracks`` disabled you'd get: This option is useful when importing classical music. -### Handling Artist Name Variations (ANVs) - -An ANV is an alternate way that an artist may be credited on a release. If the -band name changes or is misspelled on different releases. The artist name ac - Other configurations available under ``discogs:`` are: - **append_style_genre**: Appends the Discogs style (if found) to the genre tag. @@ -119,20 +114,20 @@ Other configurations available under ``discogs:`` are: disambiguation in your tags, you can disable it. Default: ``True`` - **featured_string**: Configure the string used for noting featured artists. Useful if you prefer ``Featuring`` or ``ft.``. Default: ``Feat.`` -- **artist_credit_anv**, **track_artist_anv**, **album_artist_anv**: These - configuration option are dedicated to handling Arist Name Variations (ANVs). - Sometimes a release credits artists differently compared to the majority of - their work. For example, "Basement Jaxx" may be credited as "Tha Jaxx" or "The - Basement Jaxx". By default, the Discogs plugin stores ANVs in the - ``artist_credit`` field. You can select any combination of these three to - control where beets writes and stores the variation credit. +- **anv**: These configuration option are dedicated to handling Artist Name + Variations (ANVs). Sometimes a release credits artists differently compared to + the majority of their work. For example, "Basement Jaxx" may be credited as + "Tha Jaxx" or "The Basement Jaxx".You can select any combination of these + config options to control where beets writes and stores the variation credit. + The default, shown below, writes variations to the artist_credit field. - - **artist_credit_anv**: Write ANV to the ``artist_credit`` field. - Default: ``True`` - - **track_artist_anv**: Write ANV to the ``artist`` field. Default: - ``False`` - - **album_artist_anv**: Write ANV to the ``album_artist`` field. Default: - ``False`` +.. code-block:: yaml + + discogs: + anv: + artist_credit: True + artist: False + album_artist: False .. _discogs guidelines: https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 40dd30e53..eb65bc588 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -517,9 +517,9 @@ def test_anv( title=data["title"], artists=[Bag(data=d) for d in data["artists"]], ) - config["discogs"]["album_artist_anv"] = album_artist_anv - config["discogs"]["track_artist_anv"] = track_artist_anv - config["discogs"]["artist_credit_anv"] = artist_credit_anv + config["discogs"]["anv"]["album_artist"] = album_artist_anv + config["discogs"]["anv"]["artist"] = track_artist_anv + config["discogs"]["anv"]["artist_credit"] = artist_credit_anv r = DiscogsPlugin().get_album_info(release) assert r.artist == album_artist assert r.artist_credit == album_artist_credit @@ -553,9 +553,9 @@ def test_anv_album_artist(): title=data["title"], artists=[Bag(data=d) for d in data["artists"]], ) - config["discogs"]["album_artist_anv"] = False - config["discogs"]["track_artist_anv"] = True - config["discogs"]["artist_credit_anv"] = False + config["discogs"]["anv"]["album_artist"] = False + config["discogs"]["anv"]["artist"] = True + config["discogs"]["anv"]["artist_credit"] = False r = DiscogsPlugin().get_album_info(release) assert r.artist == "ARTIST" assert r.artist_credit == "ARTIST" From b9a840a2a3aa182e76db6c2268ca4dc96fd2f2e8 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 3 Oct 2025 15:01:34 -0700 Subject: [PATCH 13/16] Update all functions with types --- beetsplug/discogs.py | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index ef479f686..d06cecd38 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -30,7 +30,7 @@ from string import ascii_lowercase from typing import TYPE_CHECKING, Sequence import confuse -from discogs_client import Client, Master, Release +from discogs_client import Client, Master, Release, Track from discogs_client.exceptions import DiscogsAPIError from requests.exceptions import ConnectionError from typing_extensions import TypedDict @@ -43,7 +43,7 @@ from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin if TYPE_CHECKING: - from collections.abc import Callable, Iterable + from collections.abc import Callable, Iterable, Tuple from beets.library import Item @@ -104,7 +104,7 @@ class DiscogsPlugin(MetadataSourcePlugin): "artist_credit": True, "artist": False, "album_artist": False, - }, + }, } ) self.config["apikey"].redact = True @@ -147,7 +147,7 @@ class DiscogsPlugin(MetadataSourcePlugin): """Get the path to the JSON file for storing the OAuth token.""" return self.config["tokenfile"].get(confuse.Filename(in_app_dir=True)) - def authenticate(self, c_key, c_secret): + def authenticate(self, c_key: str, c_secret: str) -> tuple[str, str]: # Get the link for the OAuth page. auth_client = Client(USER_AGENT, c_key, c_secret) try: @@ -323,7 +323,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist(artist_list, join_key="join") return self.strip_disambiguation(artist), artist_id - def get_album_info(self, result): + def get_album_info(self, result: Release) -> AlbumInfo: """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet # present if the result is from a `discogs_client.search()`. @@ -459,7 +459,7 @@ class DiscogsPlugin(MetadataSourcePlugin): cover_art_url=cover_art_url, ) - def select_cover_art(self, result): + def select_cover_art(self, result: Release) -> str | None: """Returns the best candidate image, if any, from a Discogs `Release` object.""" if result.data.get("images") and len(result.data.get("images")) > 0: # The first image in this list appears to be the one displayed first @@ -469,7 +469,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return None - def format(self, classification): + def format(self, classification: Iterable[str]) -> str | None: if classification: return ( self.config["separator"].as_str().join(sorted(classification)) @@ -477,7 +477,11 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks(self, tracklist, album_artist_data): + def get_tracks( + self, + tracklist: Iterable[Track], + album_artist_data: Tuple[str, str, int], + ) -> Iterable[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -579,13 +583,17 @@ class DiscogsPlugin(MetadataSourcePlugin): return tracks - def coalesce_tracks(self, raw_tracklist): + def coalesce_tracks( + self, raw_tracklist: Iterable[Track] + ) -> Iterable[Track]: """Pre-process a tracklist, merging subtracks into a single track. The title for the merged track is the one from the previous index track, if present; otherwise it is a combination of the subtracks titles. """ - def add_merged_subtracks(tracklist, subtracks): + def add_merged_subtracks( + tracklist: Iterable[Track], subtracks: Iterable[Track] + ) -> Iterable[Track]: """Modify `tracklist` in place, merging a list of `subtracks` into a single track into `tracklist`.""" # Calculate position based on first subtrack, without subindex. @@ -671,7 +679,13 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions, album_artist_data): + def get_track_info( + self, + track: Track, + index: int, + divisions: int, + album_artist_data: Tuple[str, str, int], + ) -> TrackInfo: """Returns a TrackInfo object for a discogs track.""" artist, artist_anv, artist_id = album_artist_data @@ -746,7 +760,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return medium or None, index or None, subindex or None - def get_track_length(self, duration): + def get_track_length(self, duration: str) -> int: """Returns the track length in seconds for a discogs duration.""" try: length = time.strptime(duration, "%M:%S") From a909dddd1655ef93a55784ec1834497aaace76b4 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 3 Oct 2025 18:52:37 -0700 Subject: [PATCH 14/16] adding typechecks, need to fix the medium discrepancy --- beetsplug/discogs.py | 56 +++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d06cecd38..3618b2f1d 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -27,13 +27,13 @@ import time import traceback from functools import cache from string import ascii_lowercase -from typing import TYPE_CHECKING, Sequence +from typing import TYPE_CHECKING, Sequence, cast import confuse -from discogs_client import Client, Master, Release, Track +from discogs_client import Client, Master, Release from discogs_client.exceptions import DiscogsAPIError from requests.exceptions import ConnectionError -from typing_extensions import TypedDict +from typing_extensions import TypedDict, NotRequired import beets import beets.ui @@ -43,7 +43,7 @@ from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin if TYPE_CHECKING: - from collections.abc import Callable, Iterable, Tuple + from collections.abc import Callable, Iterable from beets.library import Item @@ -84,6 +84,25 @@ class ReleaseFormat(TypedDict): qty: int descriptions: list[str] | None +class Artist(TypedDict): + name: str + anv: str + join: str + role: str + tracks: str + id: str + resource_url: str + +class Track(TypedDict): + position: str + type_: str + title: str + duration: str + artists: list[Artist] + extraartists: NotRequired[list[Artist]] + +class TrackWithSubtrack(Track): + sub_tracks: NotRequired[list[Track]] class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): @@ -309,7 +328,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype def get_artist_with_anv( - self, artists: Iterable[dict[str | int, str]], use_anv: bool = False + self, artists: list[Artist], use_anv: bool = False ) -> tuple[str, str | None]: """Iterates through a discogs result, fetching data if the artist anv is to be used, maps that to the name. @@ -320,10 +339,10 @@ class DiscogsPlugin(MetadataSourcePlugin): if use_anv and (anv := a.get("anv", "")): a["name"] = anv artist_list.append(a) - artist, artist_id = self.get_artist(artist_list, join_key="join") + artist, artist_id = self.get_artist(cast(Iterable[dict[str | int, str]], artist_list), join_key="join") return self.strip_disambiguation(artist), artist_id - def get_album_info(self, result: Release) -> AlbumInfo: + def get_album_info(self, result: Release) -> AlbumInfo | None: """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet # present if the result is from a `discogs_client.search()`. @@ -479,9 +498,9 @@ class DiscogsPlugin(MetadataSourcePlugin): def get_tracks( self, - tracklist: Iterable[Track], - album_artist_data: Tuple[str, str, int], - ) -> Iterable[TrackInfo]: + tracklist: list[Track], + album_artist_data: tuple[str, str, str | None], + ) -> list[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -496,7 +515,8 @@ class DiscogsPlugin(MetadataSourcePlugin): index_tracks = {} index = 0 # Distinct works and intra-work divisions, as defined by index tracks. - divisions, next_divisions = [], [] + divisions: list[str] = [] + next_divisions: list[str] = [] for track in clean_tracklist: # Only real tracks have `position`. Otherwise, it's an index track. if track["position"]: @@ -584,16 +604,14 @@ class DiscogsPlugin(MetadataSourcePlugin): return tracks def coalesce_tracks( - self, raw_tracklist: Iterable[Track] - ) -> Iterable[Track]: + self, raw_tracklist + ): """Pre-process a tracklist, merging subtracks into a single track. The title for the merged track is the one from the previous index track, if present; otherwise it is a combination of the subtracks titles. """ - def add_merged_subtracks( - tracklist: Iterable[Track], subtracks: Iterable[Track] - ) -> Iterable[Track]: + def add_merged_subtracks(tracklist, subtracks): """Modify `tracklist` in place, merging a list of `subtracks` into a single track into `tracklist`.""" # Calculate position based on first subtrack, without subindex. @@ -683,8 +701,8 @@ class DiscogsPlugin(MetadataSourcePlugin): self, track: Track, index: int, - divisions: int, - album_artist_data: Tuple[str, str, int], + divisions: list[str], + album_artist_data: tuple[str, str, str | None], ) -> TrackInfo: """Returns a TrackInfo object for a discogs track.""" @@ -760,7 +778,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return medium or None, index or None, subindex or None - def get_track_length(self, duration: str) -> int: + def get_track_length(self, duration: str) -> int | None: """Returns the track length in seconds for a discogs duration.""" try: length = time.strptime(duration, "%M:%S") From 2a80bdef2e358f7b34a973639a7c5211809a37c8 Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 4 Oct 2025 11:03:17 -0700 Subject: [PATCH 15/16] Added type hints to all functions --- beetsplug/discogs.py | 112 ++++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 39 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 3618b2f1d..1ef07be68 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -33,7 +33,7 @@ import confuse from discogs_client import Client, Master, Release from discogs_client.exceptions import DiscogsAPIError from requests.exceptions import ConnectionError -from typing_extensions import TypedDict, NotRequired +from typing_extensions import NotRequired, TypedDict import beets import beets.ui @@ -84,6 +84,7 @@ class ReleaseFormat(TypedDict): qty: int descriptions: list[str] | None + class Artist(TypedDict): name: str anv: str @@ -93,6 +94,7 @@ class Artist(TypedDict): id: str resource_url: str + class Track(TypedDict): position: str type_: str @@ -101,8 +103,23 @@ class Track(TypedDict): artists: list[Artist] extraartists: NotRequired[list[Artist]] -class TrackWithSubtrack(Track): - sub_tracks: NotRequired[list[Track]] + +class TrackWithSubtracks(Track): + sub_tracks: list[TrackWithSubtracks] + + +class IntermediateTrackInfo(TrackInfo): + """Allows work with string mediums from + get_track_info""" + + def __init__( + self, + medium_str: str | None, + **kwargs, + ) -> None: + self.medium_str = medium_str + super().__init__(**kwargs) + class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): @@ -131,7 +148,7 @@ class DiscogsPlugin(MetadataSourcePlugin): self.config["user_token"].redact = True self.setup() - def setup(self, session=None): + def setup(self, session=None) -> None: """Create the `discogs_client` field. Authenticate if necessary.""" c_key = self.config["apikey"].as_str() c_secret = self.config["apisecret"].as_str() @@ -157,12 +174,12 @@ class DiscogsPlugin(MetadataSourcePlugin): self.discogs_client = Client(USER_AGENT, c_key, c_secret, token, secret) - def reset_auth(self): + def reset_auth(self) -> None: """Delete token file & redo the auth steps.""" os.remove(self._tokenfile()) self.setup() - def _tokenfile(self): + def _tokenfile(self) -> str: """Get the path to the JSON file for storing the OAuth token.""" return self.config["tokenfile"].get(confuse.Filename(in_app_dir=True)) @@ -339,7 +356,9 @@ class DiscogsPlugin(MetadataSourcePlugin): if use_anv and (anv := a.get("anv", "")): a["name"] = anv artist_list.append(a) - artist, artist_id = self.get_artist(cast(Iterable[dict[str | int, str]], artist_list), join_key="join") + artist, artist_id = self.get_artist( + cast(list[dict[str | int, str]], artist_list), join_key="join" + ) return self.strip_disambiguation(artist), artist_id def get_album_info(self, result: Release) -> AlbumInfo | None: @@ -496,25 +515,15 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks( + def _process_clean_tracklist( self, - tracklist: list[Track], + clean_tracklist: list[Track], album_artist_data: tuple[str, str, str | None], - ) -> list[TrackInfo]: - """Returns a list of TrackInfo objects for a discogs tracklist.""" - try: - clean_tracklist = self.coalesce_tracks(tracklist) - except Exception as exc: - # FIXME: this is an extra precaution for making sure there are no - # side effects after #2222. It should be removed after further - # testing. - self._log.debug("{}", traceback.format_exc()) - self._log.error("uncaught exception in coalesce_tracks: {}", exc) - clean_tracklist = tracklist - tracks = [] + ) -> tuple[list[TrackInfo], dict[int, str], int, list[str], list[str]]: + # Distinct works and intra-work divisions, as defined by index tracks. + tracks: list[TrackInfo] = [] index_tracks = {} index = 0 - # Distinct works and intra-work divisions, as defined by index tracks. divisions: list[str] = [] next_divisions: list[str] = [] for track in clean_tracklist: @@ -540,7 +549,29 @@ class DiscogsPlugin(MetadataSourcePlugin): except IndexError: pass index_tracks[index + 1] = track["title"] + return tracks, index_tracks, index, divisions, next_divisions + def get_tracks( + self, + tracklist: list[Track], + album_artist_data: tuple[str, str, str | None], + ) -> list[TrackInfo]: + """Returns a list of TrackInfo objects for a discogs tracklist.""" + try: + clean_tracklist: list[Track] = self.coalesce_tracks( + cast(list[TrackWithSubtracks], tracklist) + ) + except Exception as exc: + # FIXME: this is an extra precaution for making sure there are no + # side effects after #2222. It should be removed after further + # testing. + self._log.debug("{}", traceback.format_exc()) + self._log.error("uncaught exception in coalesce_tracks: {}", exc) + clean_tracklist = tracklist + processed = self._process_clean_tracklist( + clean_tracklist, album_artist_data + ) + tracks, index_tracks, index, divisions, next_divisions = processed # Fix up medium and medium_index for each track. Discogs position is # unreliable, but tracks are in order. medium = None @@ -549,8 +580,8 @@ class DiscogsPlugin(MetadataSourcePlugin): # If a medium has two sides (ie. vinyl or cassette), each pair of # consecutive sides should belong to the same medium. - if all([track.medium is not None for track in tracks]): - m = sorted({track.medium.lower() for track in tracks}) + if all([track.medium_str is not None for track in tracks]): + m = sorted({track.medium_str.lower() for track in tracks}) # If all track.medium are single consecutive letters, assume it is # a 2-sided medium. if "".join(m) in ascii_lowercase: @@ -564,17 +595,17 @@ class DiscogsPlugin(MetadataSourcePlugin): # side_count is the number of mediums or medium sides (in the case # of two-sided mediums) that were seen before. medium_is_index = ( - track.medium + track.medium_str and not track.medium_index and ( - len(track.medium) != 1 + len(track.medium_str) != 1 or # Not within standard incremental medium values (A, B, C, ...). - ord(track.medium) - 64 != side_count + 1 + ord(track.medium_str) - 64 != side_count + 1 ) ) - if not medium_is_index and medium != track.medium: + if not medium_is_index and medium != track.medium_str: side_count += 1 if sides_per_medium == 2: if side_count % sides_per_medium: @@ -585,7 +616,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # Medium changed. Reset index_count. medium_count += 1 index_count = 0 - medium = track.medium + medium = track.medium_str index_count += 1 medium_count = 1 if medium_count == 0 else medium_count @@ -601,17 +632,20 @@ class DiscogsPlugin(MetadataSourcePlugin): disctitle = None track.disctitle = disctitle - return tracks + return cast(list[TrackInfo], tracks) def coalesce_tracks( - self, raw_tracklist - ): + self, raw_tracklist: list[TrackWithSubtracks] + ) -> list[Track]: """Pre-process a tracklist, merging subtracks into a single track. The title for the merged track is the one from the previous index track, if present; otherwise it is a combination of the subtracks titles. """ - def add_merged_subtracks(tracklist, subtracks): + def add_merged_subtracks( + tracklist: list[TrackWithSubtracks], + subtracks: list[TrackWithSubtracks], + ) -> None: """Modify `tracklist` in place, merging a list of `subtracks` into a single track into `tracklist`.""" # Calculate position based on first subtrack, without subindex. @@ -651,8 +685,8 @@ class DiscogsPlugin(MetadataSourcePlugin): tracklist.append(track) # Pre-process the tracklist, trying to identify subtracks. - subtracks = [] - tracklist = [] + subtracks: list[TrackWithSubtracks] = [] + tracklist: list[TrackWithSubtracks] = [] prev_subindex = "" for track in raw_tracklist: # Regular subtrack (track with subindex). @@ -687,7 +721,7 @@ class DiscogsPlugin(MetadataSourcePlugin): if subtracks: add_merged_subtracks(tracklist, subtracks) - return tracklist + return cast(list[Track], tracklist) def strip_disambiguation(self, text: str) -> str: """Removes discogs specific disambiguations from a string. @@ -703,7 +737,7 @@ class DiscogsPlugin(MetadataSourcePlugin): index: int, divisions: list[str], album_artist_data: tuple[str, str, str | None], - ) -> TrackInfo: + ) -> IntermediateTrackInfo: """Returns a TrackInfo object for a discogs track.""" artist, artist_anv, artist_id = album_artist_data @@ -749,7 +783,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_credit += ( f" {self.config['featured_string']} {featured_credit}" ) - return TrackInfo( + return IntermediateTrackInfo( title=title, track_id=track_id, artist_credit=artist_credit, @@ -757,7 +791,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_id=artist_id, length=length, index=index, - medium=medium, + medium_str=medium, medium_index=medium_index, ) From ed73903deb89903d8808966db74f575227c29ccd Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 4 Oct 2025 11:59:15 -0700 Subject: [PATCH 16/16] type corrections --- beetsplug/discogs.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 1ef07be68..878448556 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -350,15 +350,17 @@ class DiscogsPlugin(MetadataSourcePlugin): """Iterates through a discogs result, fetching data if the artist anv is to be used, maps that to the name. Calls the parent class get_artist method.""" - artist_list = [] + artist_list: list[dict[str | int, str]] = [] for artist_data in artists: - a = artist_data.copy() - if use_anv and (anv := a.get("anv", "")): + a: dict[str | int, str] = { + "name": artist_data["name"], + "id": artist_data["id"], + "join": artist_data.get("join", ""), + } + if use_anv and (anv := artist_data.get("anv", "")): a["name"] = anv artist_list.append(a) - artist, artist_id = self.get_artist( - cast(list[dict[str | int, str]], artist_list), join_key="join" - ) + artist, artist_id = self.get_artist(artist_list, join_key="join") return self.strip_disambiguation(artist), artist_id def get_album_info(self, result: Release) -> AlbumInfo | None: