Slightly simplify lastgenre client

beetsplug/lastgenre/__init__.py

@@ -364,18 +364,14 @@ def _try_resolve_stage(
         # Run through stages: track, album, artist,
         # album artist, or most popular track genre.
         if isinstance(obj, library.Item) and "track" in self.sources:
-            if new_genres := self.client.fetch_track_genre(
-                obj.artist, obj.title
-            ):
+            if new_genres := self.client.fetch("track", obj):
                 if result := _try_resolve_stage(
                     "track", keep_genres, new_genres
                 ):
                     return result
 
         if "album" in self.sources:
-            if new_genres := self.client.fetch_album_genre(
-                obj.albumartist, obj.album
-            ):
+            if new_genres := self.client.fetch("album", obj):
                 if result := _try_resolve_stage(
                     "album", keep_genres, new_genres
                 ):
@@ -384,10 +380,10 @@ def _try_resolve_stage(
         if "artist" in self.sources:
             new_genres = []
             if isinstance(obj, library.Item):
-                new_genres = self.client.fetch_artist_genre(obj.artist)
+                new_genres = self.client.fetch("artist", obj)
                 stage_label = "artist"
             elif obj.albumartist != config["va_name"].as_str():
-                new_genres = self.client.fetch_artist_genre(obj.albumartist)
+                new_genres = self.client.fetch("album_artist", obj)
                 stage_label = "album artist"
                 if not new_genres:
                     self._log.extra_debug(
@@ -400,9 +396,7 @@ def _try_resolve_stage(
                             'Fetching artist genre for "{}"',
                             albumartist,
                         )
-                        new_genres += self.client.fetch_artist_genre(
-                            albumartist
-                        )
+                        new_genres += self.client.fetch("album_artist", obj)
                     if new_genres:
                         stage_label = "multi-valued album artist"
             else:
@@ -412,11 +406,9 @@ def _try_resolve_stage(
                 for item in obj.items():
                     item_genre = None
                     if "track" in self.sources:
-                        item_genre = self.client.fetch_track_genre(
-                            item.artist, item.title
-                        )
+                        item_genre = self.client.fetch("track", item)
                     if not item_genre:
-                        item_genre = self.client.fetch_artist_genre(item.artist)
+                        item_genre = self.client.fetch("artist", item)
                     if item_genre:
                         item_genres += item_genre
                 if item_genres:

beetsplug/lastgenre/client.py

@@ -19,7 +19,7 @@
 from __future__ import annotations
 
 import traceback
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, ClassVar
 
 import pylast
 
@@ -28,6 +28,7 @@
 if TYPE_CHECKING:
     from collections.abc import Callable
 
+    from beets.library import LibModel
     from beets.logging import BeetsLogger
 
     GenreCache = dict[str, list[str]]
@@ -48,6 +49,18 @@
 class LastFmClient:
     """Client for fetching genres from Last.fm."""
 
+    FETCH_METHODS: ClassVar[
+        dict[
+            str,
+            tuple[Callable[..., Any], Callable[[LibModel], tuple[str, ...]]],
+        ]
+    ] = {
+        "track": (LASTFM.get_track, lambda obj: (obj.artist, obj.title)),
+        "album": (LASTFM.get_album, lambda obj: (obj.albumartist, obj.album)),
+        "artist": (LASTFM.get_artist, lambda obj: (obj.artist,)),
+        "album_artist": (LASTFM.get_artist, lambda obj: (obj.albumartist,)),
+    }
+
     def __init__(self, log: BeetsLogger, min_weight: int):
         """Initialize the client.
 
@@ -57,36 +70,17 @@ def __init__(self, log: BeetsLogger, min_weight: int):
         self._min_weight = min_weight
         self._genre_cache: GenreCache = {}
 
-    def fetch_genre(
-        self, lastfm_obj: pylast.Album | pylast.Artist | pylast.Track
+    def fetch_genres(
+        self, obj: pylast.Album | pylast.Artist | pylast.Track
     ) -> list[str]:
-        """Return genres for a pylast entity. Returns an empty list if
-        no suitable genres are found.
-        """
-        return self._tags_for(lastfm_obj, self._min_weight)
-
-    def _tags_for(
-        self,
-        obj: pylast.Album | pylast.Artist | pylast.Track,
-        min_weight: int | None = None,
-    ) -> list[str]:
-        """Core genre identification routine.
+        """Return genres for a pylast entity.
 
         Given a pylast entity (album or track), return a list of
         tag names for that entity. Return an empty list if the entity is
         not found or another error occurs.
-
-        If `min_weight` is specified, tags are filtered by weight.
         """
-        # Work around an inconsistency in pylast where
-        # Album.get_top_tags() does not return TopItem instances.
-        # https://github.com/pylast/pylast/issues/86
-        obj_to_query: Any = obj
-        if isinstance(obj, pylast.Album):
-            obj_to_query = super(pylast.Album, obj)
-
         try:
-            res: Any = obj_to_query.get_top_tags()
+            res = obj.get_top_tags()
         except PYLAST_EXCEPTIONS as exc:
             self._log.debug("last.fm error: {}", exc)
             return []
@@ -97,13 +91,11 @@ def _tags_for(
             return []
 
         # Filter by weight (optionally).
-        if min_weight:
+        if min_weight := self._min_weight:
             res = [el for el in res if (int(el.weight or 0)) >= min_weight]
 
         # Get strings from tags.
-        tags: list[str] = [el.item.get_name().lower() for el in res]
-
-        return tags
+        return [el.item.get_name().lower() for el in res]
 
     def _last_lookup(
         self, entity: str, method: Callable[..., Any], *args: str
@@ -123,24 +115,15 @@ def _last_lookup(
         key = f"{entity}.{'-'.join(str(a) for a in args)}"
         if key not in self._genre_cache:
             args_replaced = [a.replace("\u2010", "-") for a in args]
-            self._genre_cache[key] = self.fetch_genre(method(*args_replaced))
-
-        genre = self._genre_cache[key]
-        self._log.extra_debug("last.fm (unfiltered) {} tags: {}", entity, genre)
-        return genre
+            self._genre_cache[key] = self.fetch_genres(method(*args_replaced))
 
-    def fetch_album_genre(self, albumartist: str, albumtitle: str) -> list[str]:
-        """Return genres from Last.fm for the album by albumartist."""
-        return self._last_lookup(
-            "album", LASTFM.get_album, albumartist, albumtitle
+        genres = self._genre_cache[key]
+        self._log.extra_debug(
+            "last.fm (unfiltered) {} tags: {}", entity, genres
         )
+        return genres
 
-    def fetch_artist_genre(self, artist: str) -> list[str]:
-        """Return genres from Last.fm for the artist."""
-        return self._last_lookup("artist", LASTFM.get_artist, artist)
-
-    def fetch_track_genre(self, trackartist: str, tracktitle: str) -> list[str]:
-        """Return genres from Last.fm for the track by artist."""
-        return self._last_lookup(
-            "track", LASTFM.get_track, trackartist, tracktitle
-        )
+    def fetch(self, kind: str, obj: LibModel) -> list[str]:
+        """Fetch fetcher for Last.fm genres."""
+        method, arg_fn = self.FETCH_METHODS[kind]
+        return self._last_lookup(kind, method, *arg_fn(obj))

test/plugins/test_lastgenre.py

@@ -167,7 +167,7 @@ def test_no_duplicate(self):
         self._setup_config(count=99)
         assert self.plugin._resolve_genres(["blues", "blues"]) == ["blues"]
 
-    def test_tags_for(self):
+    def test_fetch_genre(self):
         class MockPylastElem:
             def __init__(self, name):
                 self.name = name
@@ -186,9 +186,11 @@ def get_top_tags(self):
                 return [tag1, tag2]
 
         plugin = lastgenre.LastGenrePlugin()
-        res = plugin.client._tags_for(MockPylastObj())
+        res = plugin.client.fetch_genres(MockPylastObj())
         assert res == ["pop", "rap"]
-        res = plugin.client._tags_for(MockPylastObj(), min_weight=50)
+
+        plugin.client._min_weight = 50
+        res = plugin.client.fetch_genres(MockPylastObj())
         assert res == ["pop"]
 
     def test_sort_by_depth(self):
@@ -604,27 +606,18 @@ def config(config):
         ),
     ],
 )
+@pytest.mark.usefixtures("config")
 def test_get_genre(
-    config, config_values, item_genre, mock_genres, expected_result
+    monkeypatch, config_values, item_genre, mock_genres, expected_result
 ):
     """Test _get_genre with various configurations."""
-
-    def mock_fetch_track_genre(self, trackartist, tracktitle):
-        return mock_genres["track"]
-
-    def mock_fetch_album_genre(self, albumartist, albumtitle):
-        return mock_genres["album"]
-
-    def mock_fetch_artist_genre(self, artist):
-        return mock_genres["artist"]
-
     # Mock the last.fm fetchers. When whitelist enabled, we can assume only
     # whitelisted genres get returned, the plugin's _resolve_genre method
     # ensures it.
-    lastgenre.client.LastFmClient.fetch_track_genre = mock_fetch_track_genre
-    lastgenre.client.LastFmClient.fetch_album_genre = mock_fetch_album_genre
-    lastgenre.client.LastFmClient.fetch_artist_genre = mock_fetch_artist_genre
-
+    monkeypatch.setattr(
+        "beetsplug.lastgenre.client.LastFmClient.fetch",
+        lambda _, kind, __: mock_genres[kind],
+    )
     # Initialize plugin instance and item
     plugin = lastgenre.LastGenrePlugin()
     # Configure