diff --git a/beets/metadata_plugins.py b/beets/metadata_plugins.py index 0372790af..0b8d81c95 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -271,10 +271,9 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): """Returns an artist string (all artists) and an artist_id (the main artist) for a list of artist object dicts. - For each artist, this function moves articles (such as 'a', 'an', - and 'the') to the front and strips trailing disambiguation numbers. It - returns a tuple containing the comma-separated string of all - normalized artists and the ``id`` of the main/first artist. + For each artist, this function moves articles (such as 'a', 'an', and 'the') + to the front. It returns a tuple containing the comma-separated string + of all normalized artists and the ``id`` of the main/first artist. Alternatively a keyword can be used to combine artists together into a single string by passing the join_key argument. diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d8f1d6ba5..0ffbb0e3e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -76,7 +76,7 @@ TRACK_INDEX_RE = re.compile( re.VERBOSE, ) -DISAMBIGUATION_RE = re.compile(r" \(\d+\)$") +DISAMBIGUATION_RE = re.compile(r" \(\d+\)") class ReleaseFormat(TypedDict): @@ -365,23 +365,22 @@ class DiscogsPlugin(MetadataSourcePlugin): label = catalogno = labelid = None if result.data.get("labels"): - label = result.data["labels"][0].get("name") + label = self.strip_disambiguation( + result.data["labels"][0].get("name") + ) catalogno = result.data["labels"][0].get("catno") labelid = result.data["labels"][0].get("id") cover_art_url = self.select_cover_art(result) - # Additional cleanups (various artists name, catalog number, media). + # Additional cleanups + # (various artists name, catalog number, media, disambiguation). if va: artist = config["va_name"].as_str() + else: + artist = self.strip_disambiguation(artist) if catalogno == "none": catalogno = None - - # Remove Discogs specific artist disambiguation 'Artist (2)' or 'Label (3)' - if self.config["strip_disambiguation"]: - artist = DISAMBIGUATION_RE.sub("", artist) - if label is not None: - label = DISAMBIGUATION_RE.sub("", label) # Explicitly set the `media` for the tracks, since it is expected by # `autotag.apply_metadata`, and set `medium_total`. for track in tracks: @@ -631,6 +630,14 @@ class DiscogsPlugin(MetadataSourcePlugin): return tracklist + def strip_disambiguation(self, text) -> str: + """Removes discogs specific disambiguations from a string. + Turns 'Label Name (5)' to 'Label Name' or 'Artist (1) & Another Artist (2)' + to 'Artist & Another Artist'. Does nothing if strip_disambiguation is False.""" + if not self.config["strip_disambiguation"]: + return text + return DISAMBIGUATION_RE.sub("", text) + def get_track_info(self, track, index, divisions): """Returns a TrackInfo object for a discogs track.""" title = track["title"] @@ -643,6 +650,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist( track.get("artists", []), join_key="join" ) + artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) return TrackInfo( title=title, diff --git a/docs/changelog.rst b/docs/changelog.rst index 0814686f8..ddafd975d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,8 +18,12 @@ 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` Added config option `strip_disambiguation` to allow choice of removing discogs numeric disambiguation :bug:`5366` -- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels :bug:`5366` +- :doc:`plugins/discogs` Added config option `strip_disambiguation` to allow + choice of removing discogs numeric disambiguation :bug:`5366` +- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from + artists but not labels. :bug:`5366` +- :doc:`plugins/discogs` Wrote test coverage for removing disambiguation. + :bug:`5366` For packagers: diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 44c0c0e22..3dac558bd 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -109,6 +109,9 @@ Other configurations available under ``discogs:`` are: - **search_limit**: The maximum number of results to return from Discogs. This is useful if you want to limit the number of results returned to speed up searches. Default: ``5`` +- **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`` .. _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 c279ff128..5fe73dcac 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -375,7 +375,7 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.style is None def test_strip_disambiguation_label_artist(self): - """Test removing discogs disambiguation""" + """Test removing discogs disambiguation from artist and label""" data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", @@ -399,21 +399,21 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.label == "LABEL NAME" def test_strip_disambiguation_off_label_artist(self): - """Test not removing discogs disambiguation""" + """Test not removing discogs disambiguation from artist and label""" + config["discogs"]["strip_disambiguation"] = False data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [self._make_track("A", "1", "01:01")], + "tracklist": [self._make_track("a", "1", "01:01")], "artists": [{"name": "ARTIST NAME (2)", "id": 321, "join": ""}], - "title": "TITLE", + "title": "title", "labels": [ { "name": "LABEL NAME (5)", - "catno": "CATALOG NUMBER", + "catno": "catalog number", } ], } - config["discogs"]["strip_disambiguation"] = False release = Bag( data=data, title=data["title"], @@ -423,6 +423,57 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.artist == "ARTIST NAME (2)" assert d.label == "LABEL NAME (5)" + def test_strip_disambiguation_multiple_artists(self): + """Test removing disambiguation if there are multiple artists on the release""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [self._make_track("a", "1", "01:01")], + "artists": [ + {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, + {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.artist == "ARTIST NAME & OTHER ARTIST" + + def test_strip_disambiguation_artist_tracks(self): + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + { + "name": "TEST ARTIST (5)", + "tracks": "", + "id": 11146, + } + ], + } + ], + "artists": [{"name": "OTHER ARTIST (5)", "id": 321, "join": ""}], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.tracks[0].artist == "TEST ARTIST" + assert d.artist == "OTHER ARTIST" + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype",