From 9242db04a556a587ac6097b00a62498331a2bc8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 May 2025 12:51:05 +0100 Subject: [PATCH 1/8] discogs: add configurable search_limit --- beetsplug/discogs.py | 14 +++++++------- docs/changelog.rst | 2 ++ docs/plugins/discogs.rst | 15 ++++++++++++--- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 1852f300f..187a30e4e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -73,6 +73,7 @@ class DiscogsPlugin(BeetsPlugin): "separator": ", ", "index_tracks": False, "append_style_genre": False, + "search_limit": 5, } ) self.config["apikey"].redact = True @@ -257,8 +258,8 @@ class DiscogsPlugin(BeetsPlugin): ) if track_result: candidates.append(track_result) - # first 10 results, don't overwhelm with options - return candidates[:10] + + return candidates def album_for_id(self, album_id): """Fetches an album by its Discogs ID and returns an AlbumInfo object @@ -303,8 +304,9 @@ class DiscogsPlugin(BeetsPlugin): query = re.sub(r"(?i)\b(CD|disc|vinyl)\s*\d+", "", query) try: - releases = self.discogs_client.search(query, type="release").page(1) - + results = self.discogs_client.search(query, type="release") + results.per_page = self.config["search_limit"].as_number() + releases = results.page(1) except CONNECTION_ERRORS: self._log.debug( "Communication error while searching for {0!r}", @@ -312,9 +314,7 @@ class DiscogsPlugin(BeetsPlugin): exc_info=True, ) return [] - return [ - album for album in map(self.get_album_info, releases[:5]) if album - ] + return map(self.get_album_info, releases) def get_master_year(self, master_id): """Fetches a master release given its Discogs ID and returns its year diff --git a/docs/changelog.rst b/docs/changelog.rst index 3370f5396..ebb9880a9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,8 @@ New features: :bug:`4605` * :doc:`plugins/web`: Show notifications when a track plays. This uses the Media Session API to customize media notifications. +* :doc:`plugins/discogs`: Add configurable ``search_limit`` option to + limit the number of results returned by the Discogs metadata search queries. Bug fixes: diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index ac67f2d0a..c8df12a41 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -101,11 +101,20 @@ This option is useful when importing classical music. Other configurations available under ``discogs:`` are: -- **append_style_genre**: Appends the Discogs style (if found) to the genre tag. This can be useful if you want more granular genres to categorize your music. - For example, a release in Discogs might have a genre of "Electronic" and a style of "Techno": enabling this setting would set the genre to be "Electronic, Techno" (assuming default separator of ``", "``) instead of just "Electronic". +- **append_style_genre**: Appends the Discogs style (if found) to the genre + tag. This can be useful if you want more granular genres to categorize your + music. For example, a release in Discogs might have a genre of "Electronic" + and a style of "Techno": enabling this setting would set the genre to be + "Electronic, Techno" (assuming default separator of ``", "``) instead of just + "Electronic". Default: ``False`` -- **separator**: How to join multiple genre and style values from Discogs into a string. +- **separator**: How to join multiple genre and style values from Discogs into + a string. Default: ``", "`` +- **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`` Troubleshooting From 09862aeaea523a6de688543500f1ddfb664fa3ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 May 2025 13:00:06 +0100 Subject: [PATCH 2/8] discogs: Add types to public methods --- beetsplug/discogs.py | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 187a30e4e..cd6e0cfd1 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -26,6 +26,7 @@ import socket import time import traceback from string import ascii_lowercase +from typing import TYPE_CHECKING import confuse from discogs_client import Client, Master, Release @@ -40,6 +41,11 @@ from beets.autotag.hooks import AlbumInfo, TrackInfo, string_dist from beets.plugins import BeetsPlugin, MetadataSourcePlugin, get_distance from beets.util.id_extractors import extract_release_id +if TYPE_CHECKING: + from collections.abc import Iterable + + from beets.library import Item + USER_AGENT = f"beets/{beets.__version__} +https://beets.io/" API_KEY = "rAzVUQYRaoFjeBjyWuWZ" API_SECRET = "plxtUTqoCzwxZpqdPysCwGuBSmZNdZVy" @@ -157,16 +163,9 @@ class DiscogsPlugin(BeetsPlugin): data_source="Discogs", info=track_info, config=self.config ) - def candidates(self, items, artist, album, va_likely): - """Returns a list of AlbumInfo objects for discogs search results - matching an album and artist (if not various). - """ - if not album and not artist: - self._log.debug( - "Skipping Discogs query. Files missing album and artist tags." - ) - return [] - + def candidates( + self, items: list[Item], artist: str, album: str, va_likely: bool + ) -> Iterable[AlbumInfo]: if va_likely: query = album else: @@ -220,24 +219,9 @@ class DiscogsPlugin(BeetsPlugin): return None - def item_candidates(self, item, artist, title): - """Returns a list of TrackInfo objects for Search API results - matching ``title`` and ``artist``. - :param item: Singleton item to be matched. - :type item: beets.library.Item - :param artist: The artist of the track to be matched. - :type artist: str - :param title: The title of the track to be matched. - :type title: str - :return: Candidate TrackInfo objects. - :rtype: list[beets.autotag.hooks.TrackInfo] - """ - if not artist and not title: - self._log.debug( - "Skipping Discogs query. File missing artist and title tags." - ) - return [] - + def item_candidates( + self, item: Item, artist: str, title: str + ) -> Iterable[TrackInfo]: query = f"{artist} {title}" try: albums = self.get_albums(query) @@ -261,7 +245,7 @@ class DiscogsPlugin(BeetsPlugin): return candidates - def album_for_id(self, album_id): + def album_for_id(self, album_id: str) -> AlbumInfo | None: """Fetches an album by its Discogs ID and returns an AlbumInfo object or None if the album is not found. """ @@ -292,7 +276,7 @@ class DiscogsPlugin(BeetsPlugin): return None return self.get_album_info(result) - def get_albums(self, query): + def get_albums(self, query: str) -> Iterable[AlbumInfo]: """Returns a list of AlbumInfo objects for a discogs search query.""" # Strip non-word characters from query. Things like "!" and "-" can # cause a query to return no results, even if they match the artist or From 12149b3e6d525075f0521da178bd29fd180882ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 May 2025 13:44:54 +0100 Subject: [PATCH 3/8] discogs: remove duplicate error handling --- beetsplug/discogs.py | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index cd6e0cfd1..93791ad5e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -166,22 +166,7 @@ class DiscogsPlugin(BeetsPlugin): def candidates( self, items: list[Item], artist: str, album: str, va_likely: bool ) -> Iterable[AlbumInfo]: - if va_likely: - query = album - else: - query = f"{artist} {album}" - try: - return self.get_albums(query) - except DiscogsAPIError as e: - self._log.debug("API Error: {0} (query: {1})", e, query) - if e.status_code == 401: - self.reset_auth() - return self.candidates(items, artist, album, va_likely) - else: - return [] - except CONNECTION_ERRORS: - self._log.debug("Connection error in album search", exc_info=True) - return [] + return self.get_albums(f"{artist} {album}" if va_likely else album) def get_track_from_album_by_title( self, album_info, title, dist_threshold=0.3 @@ -222,18 +207,8 @@ class DiscogsPlugin(BeetsPlugin): def item_candidates( self, item: Item, artist: str, title: str ) -> Iterable[TrackInfo]: - query = f"{artist} {title}" - try: - albums = self.get_albums(query) - except DiscogsAPIError as e: - self._log.debug("API Error: {0} (query: {1})", e, query) - if e.status_code == 401: - self.reset_auth() - return self.item_candidates(item, artist, title) - else: - return [] - except CONNECTION_ERRORS: - self._log.debug("Connection error in track search", exc_info=True) + albums = self.candidates([item], artist, title, False) + candidates = [] for album_cur in albums: self._log.debug("searching within album {0}", album_cur.album) From 8e5858254b056fd555c8afce12128f0a7c40d7e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 May 2025 14:27:41 +0100 Subject: [PATCH 4/8] discogs: cache master release lookups --- beetsplug/discogs.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 93791ad5e..ad1310712 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -25,6 +25,7 @@ import re import socket import time import traceback +from functools import cache from string import ascii_lowercase from typing import TYPE_CHECKING @@ -275,16 +276,16 @@ class DiscogsPlugin(BeetsPlugin): return [] return map(self.get_album_info, releases) - def get_master_year(self, master_id): + @cache + def get_master_year(self, master_id: str) -> int | None: """Fetches a master release given its Discogs ID and returns its year or None if the master release is not found. """ - self._log.debug("Searching for master release {0}", master_id) + self._log.debug("Getting master release {0}", master_id) result = Master(self.discogs_client, {"id": master_id}) try: - year = result.fetch("year") - return year + return result.fetch("year") except DiscogsAPIError as e: if e.status_code != 404: self._log.debug( From 9cc7ecaceb39456e0f739be87c4eef096f9634c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 May 2025 14:53:05 +0100 Subject: [PATCH 5/8] discogs: cache TRACK_INDEX_RE --- beetsplug/discogs.py | 42 ++++++++++++++++++++---------------- test/plugins/test_discogs.py | 40 ++++++++++++++++------------------ 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index ad1310712..8fdd515bd 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -61,6 +61,22 @@ CONNECTION_ERRORS = ( ) +TRACK_INDEX_RE = re.compile( + r""" + (.*?) # medium: everything before medium_index. + (\d*?) # medium_index: a number at the end of + # `position`, except if followed by a subtrack index. + # subtrack_index: can only be matched if medium + # or medium_index have been matched, and can be + ( + (?<=\w)\.[\w]+ # a dot followed by a string (A.1, 2.A) + | (?<=\d)[A-Z]+ # a string that follows a number (1A, B2a) + )? + """, + re.VERBOSE, +) + + class ReleaseFormat(TypedDict): name: str qty: int @@ -655,33 +671,21 @@ class DiscogsPlugin(BeetsPlugin): medium_index=medium_index, ) - def get_track_index(self, position): + @staticmethod + def get_track_index( + position: str, + ) -> tuple[str | None, str | None, str | None]: """Returns the medium, medium index and subtrack index for a discogs track position.""" # Match the standard Discogs positions (12.2.9), which can have several # forms (1, 1-1, A1, A1.1, A1a, ...). - match = re.match( - r"^(.*?)" # medium: everything before medium_index. - r"(\d*?)" # medium_index: a number at the end of - # `position`, except if followed by a subtrack - # index. - # subtrack_index: can only be matched if medium - # or medium_index have been matched, and can be - r"((?<=\w)\.[\w]+" # - a dot followed by a string (A.1, 2.A) - r"|(?<=\d)[A-Z]+" # - a string that follows a number (1A, B2a) - r")?" - r"$", - position.upper(), - ) - - if match: + medium = index = subindex = None + if match := TRACK_INDEX_RE.fullmatch(position.upper()): medium, index, subindex = match.groups() if subindex and subindex.startswith("."): subindex = subindex[1:] - else: - self._log.debug("Invalid position: {0}", position) - medium = index = subindex = None + return medium or None, index or None, subindex or None def get_track_length(self, duration): diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index eb9a625b1..c31ac7511 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -171,27 +171,6 @@ class DGAlbumInfoTest(BeetsTestCase): assert t[3].index == 4 assert t[3].medium_total == 1 - def test_parse_position(self): - """Test the conversion of discogs `position` to medium, medium_index - and subtrack_index.""" - # List of tuples (discogs_position, (medium, medium_index, subindex) - positions = [ - ("1", (None, "1", None)), - ("A12", ("A", "12", None)), - ("12-34", ("12-", "34", None)), - ("CD1-1", ("CD1-", "1", None)), - ("1.12", (None, "1", "12")), - ("12.a", (None, "12", "A")), - ("12.34", (None, "12", "34")), - ("1ab", (None, "1", "AB")), - # Non-standard - ("IV", ("IV", None, None)), - ] - - d = DiscogsPlugin() - for position, expected in positions: - assert d.get_track_index(position) == expected - def test_parse_tracklist_without_sides(self): """Test standard Discogs position 12.2.9#1: "without sides".""" release = self._make_release_from_positions(["1", "2", "3"]) @@ -417,3 +396,22 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): result = DiscogsPlugin.get_media_and_albumtype(formats) assert result == (expected_media, expected_albumtype) + + +@pytest.mark.parametrize( + "position, medium, index, subindex", + [ + ("1", None, "1", None), + ("A12", "A", "12", None), + ("12-34", "12-", "34", None), + ("CD1-1", "CD1-", "1", None), + ("1.12", None, "1", "12"), + ("12.a", None, "12", "A"), + ("12.34", None, "12", "34"), + ("1ab", None, "1", "AB"), + # Non-standard + ("IV", "IV", None, None), + ], +) +def test_get_track_index(position, medium, index, subindex): + assert DiscogsPlugin.get_track_index(position) == (medium, index, subindex) From d9b67acff5478fc96bce61e80c7ff97f2f6d6545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 May 2025 14:55:25 +0100 Subject: [PATCH 6/8] discogs: simplify getting track from album --- beetsplug/discogs.py | 59 ++++++++++++-------------------------------- 1 file changed, 16 insertions(+), 43 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 8fdd515bd..34d8f21d1 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -43,7 +43,7 @@ from beets.plugins import BeetsPlugin, MetadataSourcePlugin, get_distance from beets.util.id_extractors import extract_release_id if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Callable, Iterable from beets.library import Item @@ -185,57 +185,30 @@ class DiscogsPlugin(BeetsPlugin): ) -> Iterable[AlbumInfo]: return self.get_albums(f"{artist} {album}" if va_likely else album) - def get_track_from_album_by_title( - self, album_info, title, dist_threshold=0.3 - ): - def compare_func(track_info): - track_title = getattr(track_info, "title", None) - dist = string_dist(track_title, title) - return track_title and dist < dist_threshold - - return self.get_track_from_album(album_info, compare_func) - - def get_track_from_album(self, album_info, compare_func): - """Return the first track of the release where `compare_func` returns - true. - - :return: TrackInfo object. - :rtype: beets.autotag.hooks.TrackInfo - """ - if not album_info: + def get_track_from_album( + self, album_info: AlbumInfo, compare: Callable[[TrackInfo], float] + ) -> TrackInfo | None: + """Return the best matching track of the release.""" + scores_and_tracks = [(compare(t), t) for t in album_info.tracks] + score, track_info = min(scores_and_tracks, key=lambda x: x[0]) + if score > 0.3: return None - for track_info in album_info.tracks: - # check for matching position - if not compare_func(track_info): - continue - - # attach artist info if not provided - if not track_info["artist"]: - track_info["artist"] = album_info.artist - track_info["artist_id"] = album_info.artist_id - # attach album info - track_info["album"] = album_info.album - - return track_info - - return None + track_info["artist"] = album_info.artist + track_info["artist_id"] = album_info.artist_id + track_info["album"] = album_info.album + return track_info def item_candidates( self, item: Item, artist: str, title: str ) -> Iterable[TrackInfo]: albums = self.candidates([item], artist, title, False) - candidates = [] - for album_cur in albums: - self._log.debug("searching within album {0}", album_cur.album) - track_result = self.get_track_from_album_by_title( - album_cur, item["title"] - ) - if track_result: - candidates.append(track_result) + def compare_func(track_info: TrackInfo) -> float: + return string_dist(track_info.title, title) - return candidates + tracks = (self.get_track_from_album(a, compare_func) for a in albums) + return list(filter(None, tracks)) def album_for_id(self, album_id: str) -> AlbumInfo | None: """Fetches an album by its Discogs ID and returns an AlbumInfo object From d3ef627494ed6678b3189a633c3b82f8bf30011e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 18 May 2025 04:17:19 +0100 Subject: [PATCH 7/8] Expect plugins to return Iterables instead of Iterators --- beets/plugins.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index 25452560a..63e5d3bde 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -46,7 +46,7 @@ else: if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterable from confuse import ConfigView @@ -70,7 +70,7 @@ if TYPE_CHECKING: P = ParamSpec("P") Ret = TypeVar("Ret", bound=Any) Listener = Callable[..., None] - IterF = Callable[P, Iterator[Ret]] + IterF = Callable[P, Iterable[Ret]] PLUGIN_NAMESPACE = "beetsplug" @@ -240,7 +240,7 @@ class BeetsPlugin: def candidates( self, items: list[Item], artist: str, album: str, va_likely: bool - ) -> Iterator[AlbumInfo]: + ) -> Iterable[AlbumInfo]: """Return :py:class:`AlbumInfo` candidates that match the given album. :param items: List of items in the album @@ -252,7 +252,7 @@ class BeetsPlugin: def item_candidates( self, item: Item, artist: str, title: str - ) -> Iterator[TrackInfo]: + ) -> Iterable[TrackInfo]: """Return :py:class:`TrackInfo` candidates that match the given track. :param item: Track item @@ -487,7 +487,7 @@ def notify_info_yielded(event: str) -> Callable[[IterF[P, Ret]], IterF[P, Ret]]: def decorator(func: IterF[P, Ret]) -> IterF[P, Ret]: @wraps(func) - def wrapper(*args: P.args, **kwargs: P.kwargs) -> Iterator[Ret]: + def wrapper(*args: P.args, **kwargs: P.kwargs) -> Iterable[Ret]: for v in func(*args, **kwargs): send(event, info=v) yield v @@ -498,14 +498,14 @@ def notify_info_yielded(event: str) -> Callable[[IterF[P, Ret]], IterF[P, Ret]]: @notify_info_yielded("albuminfo_received") -def candidates(*args, **kwargs) -> Iterator[AlbumInfo]: +def candidates(*args, **kwargs) -> Iterable[AlbumInfo]: """Return matching album candidates from all plugins.""" for plugin in find_plugins(): yield from plugin.candidates(*args, **kwargs) @notify_info_yielded("trackinfo_received") -def item_candidates(*args, **kwargs) -> Iterator[TrackInfo]: +def item_candidates(*args, **kwargs) -> Iterable[TrackInfo]: """Return matching track candidates from all plugins.""" for plugin in find_plugins(): yield from plugin.item_candidates(*args, **kwargs) @@ -865,7 +865,7 @@ class MetadataSourcePlugin(Generic[R], BeetsPlugin, metaclass=abc.ABCMeta): def candidates( self, items: list[Item], artist: str, album: str, va_likely: bool - ) -> Iterator[AlbumInfo]: + ) -> Iterable[AlbumInfo]: query_filters = {"album": album} if not va_likely: query_filters["artist"] = artist @@ -875,7 +875,7 @@ class MetadataSourcePlugin(Generic[R], BeetsPlugin, metaclass=abc.ABCMeta): def item_candidates( self, item: Item, artist: str, title: str - ) -> Iterator[TrackInfo]: + ) -> Iterable[TrackInfo]: for result in self._search_api( "track", {"artist": artist}, keywords=title ): From e151b4b49bb85cdf8074201b56fadfbc6079cab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 19 May 2025 08:55:58 +0100 Subject: [PATCH 8/8] Implement track_for_id to allow fetching singletons by discogs id --- beetsplug/discogs.py | 8 ++++++++ docs/changelog.rst | 3 +++ 2 files changed, 11 insertions(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 34d8f21d1..696f1d1ac 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -241,6 +241,14 @@ class DiscogsPlugin(BeetsPlugin): return None return self.get_album_info(result) + def track_for_id(self, track_id: str) -> TrackInfo | None: + if album := self.album_for_id(track_id): + for track in album.tracks: + if track.track_id == track_id: + return track + + return None + def get_albums(self, query: str) -> Iterable[AlbumInfo]: """Returns a list of AlbumInfo objects for a discogs search query.""" # Strip non-word characters from query. Things like "!" and "-" can diff --git a/docs/changelog.rst b/docs/changelog.rst index ebb9880a9..825e287f6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,6 +19,9 @@ New features: Media Session API to customize media notifications. * :doc:`plugins/discogs`: Add configurable ``search_limit`` option to limit the number of results returned by the Discogs metadata search queries. +* :doc:`plugins/discogs`: Implement ``track_for_id`` method to allow retrieving + singletons by their Discogs ID. + :bug:`4661` Bug fixes: