From 84e52e1b4a4a73151f29d3a99da857dfa0c96583 Mon Sep 17 00:00:00 2001 From: Henry Date: Mon, 22 Sep 2025 20:49:53 -0700 Subject: [PATCH 1/3] Test written, beginning fix --- beetsplug/discogs.py | 1 + test/plugins/test_discogs.py | 57 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index f22ea2274..caefbe64a 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -652,6 +652,7 @@ class DiscogsPlugin(MetadataSourcePlugin): ) artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) + featured = map(lambda artist: artist["name"] if artist["role"].find("Featuring") else "", track.get("extraartists", [])) return TrackInfo( title=title, track_id=track_id, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 92301380e..84573d337 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -451,6 +451,63 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" +@pytest.mark.parametrize("track, expected_artist", + [({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [{ + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "ARTIST feat. MUSICIAN" + ), + ({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [{ + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, { + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "ARTIST feat. PERFORMER & MUSICIAN" + ), + ({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artist": "NEW ARTIST", + "extraartists": [{ + "name": "SOLIST", + "role": "Featuring", + }, { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, { + "name": "RANDOM", + "role": "Written-By", + }, { + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "NEW ARTIST feat. SOLIST, PERFORMER & MUSICIAN" + )]) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +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) + assert t.artist == expected_artist @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", From 2bf411e77d9d5feb077c2d86ad48c1bfc3daeade Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 23 Sep 2025 10:11:13 -0700 Subject: [PATCH 2/3] Featured artists extracted and appended, need to see if join needs to be variable --- beetsplug/discogs.py | 20 +++++++++++++++----- test/plugins/test_discogs.py | 13 ++++++++----- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index caefbe64a..43a501a22 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -339,7 +339,7 @@ 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"]) + tracks = self.get_tracks(result.data["tracklist"], artist, artist_id) # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -446,7 +446,7 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks(self, tracklist): + def get_tracks(self, tracklist, album_artist, album_artist_id): """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -471,7 +471,8 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions) + track_info = self.get_track_info(track, index, divisions, + album_artist, album_artist_id) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -638,7 +639,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions): + def get_track_info(self, track, index, divisions, album_artist, album_artist_id): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -650,9 +651,18 @@ class DiscogsPlugin(MetadataSourcePlugin): 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 = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) - featured = map(lambda artist: artist["name"] if artist["role"].find("Featuring") else "", track.get("extraartists", [])) + # Add featured artists + extraartists = track.get("extraartists", []) + featured = [ + artist["name"] for artist in extraartists if artist["role"].find("Featuring") != -1] + if featured: + artist = f"{artist} feat. {', '.join(featured)}" return TrackInfo( title=title, track_id=track_id, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 84573d337..19428b125 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -477,16 +477,19 @@ class DGAlbumInfoTest(BeetsTestCase): "role": "Featuring", }] }, - "ARTIST feat. PERFORMER & MUSICIAN" + "ARTIST feat. PERFORMER, MUSICIAN" ), ({ "type_": "track", "title": "track", "position": "1", "duration": "5:00", - "artist": "NEW ARTIST", + "artists": [{ + "name": "NEW ARTIST", + "tracks": "", + "id": 11146}], "extraartists": [{ - "name": "SOLIST", + "name": "SOLOIST", "role": "Featuring", }, { "name": "PERFORMER", @@ -499,14 +502,14 @@ class DGAlbumInfoTest(BeetsTestCase): "role": "Featuring", }] }, - "NEW ARTIST feat. SOLIST, PERFORMER & MUSICIAN" + "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN" )]) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) 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) + t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) assert t.artist == expected_artist @pytest.mark.parametrize( From 6aba11d4a0fa85163607218853549004187969ae Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 23 Sep 2025 11:05:48 -0700 Subject: [PATCH 3/3] testing, updated changelog --- beetsplug/discogs.py | 18 ++--- docs/changelog.rst | 2 + test/plugins/test_discogs.py | 126 ++++++++++++++++++++--------------- 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 43a501a22..c8ce43a1e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -386,10 +386,6 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in tracks: track.media = media track.medium_total = mediums.count(track.medium) - if not track.artist: # get_track_info often fails to find artist - track.artist = artist - if not track.artist_id: - track.artist_id = artist_id # Discogs does not have track IDs. Invent our own IDs as proposed # in #2336. track.track_id = f"{album_id}-{track.track_alt}" @@ -471,8 +467,9 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions, - album_artist, album_artist_id) + track_info = self.get_track_info( + track, index, divisions, album_artist, album_artist_id + ) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -639,7 +636,9 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions, album_artist, album_artist_id): + def get_track_info( + self, track, index, divisions, album_artist, album_artist_id + ): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -660,7 +659,10 @@ class DiscogsPlugin(MetadataSourcePlugin): # Add featured artists extraartists = track.get("extraartists", []) featured = [ - artist["name"] for artist in extraartists if artist["role"].find("Featuring") != -1] + artist["name"] + for artist in extraartists + if artist["role"].find("Featuring") != -1 + ] if featured: artist = f"{artist} feat. {', '.join(featured)}" return TrackInfo( diff --git a/docs/changelog.rst b/docs/changelog.rst index 6c81a0d08..a5393e032 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,8 @@ Bug fixes: 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. For packagers: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 19428b125..562bda88f 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -451,67 +451,83 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" -@pytest.mark.parametrize("track, expected_artist", - [({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [{ - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "ARTIST feat. MUSICIAN" - ), - ({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [{ - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, { - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "ARTIST feat. PERFORMER, MUSICIAN" - ), - ({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "artists": [{ - "name": "NEW ARTIST", - "tracks": "", - "id": 11146}], - "extraartists": [{ - "name": "SOLOIST", - "role": "Featuring", - }, { - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, { - "name": "RANDOM", - "role": "Written-By", - }, { - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN" - )]) + +@pytest.mark.parametrize( + "track, expected_artist", + [ + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [ + { + "name": "MUSICIAN", + "role": "Featuring", + } + ], + }, + "ARTIST feat. MUSICIAN", + ), + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [ + { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, + { + "name": "MUSICIAN", + "role": "Featuring", + }, + ], + }, + "ARTIST feat. PERFORMER, MUSICIAN", + ), + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artists": [{"name": "NEW ARTIST", "tracks": "", "id": 11146}], + "extraartists": [ + { + "name": "SOLOIST", + "role": "Featuring", + }, + { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, + { + "name": "RANDOM", + "role": "Written-By", + }, + { + "name": "MUSICIAN", + "role": "Featuring", + }, + ], + }, + "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN", + ), + ], +) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) 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, + """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) assert t.artist == expected_artist + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", [