mirror of
https://github.com/beetbox/beets.git
synced 2025-12-30 12:32:33 +01:00
lastgenre: Fix canonicalization of non-valid genres
- Remove "early whitelist check", since it breaks canonicalization of actually unwanted genres (not whitelisted) resolving "up" to parent genres. - Remove the filter_valid_genres method entirely and get back to inline list comprehensions. The caveat is that None genres are not catched that way (see below, should be one of the last functions that finally returns lists only) - Along the way, fix _last_lookup's rearly return to empty list instead of None.
This commit is contained in:
parent
7f7b900f1b
commit
fa8b5d7495
1 changed files with 24 additions and 39 deletions
|
|
@ -184,31 +184,28 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
return [p[1] for p in depth_tag_pairs]
|
||||
|
||||
def _resolve_genres(self, tags: list[str]) -> list[str]:
|
||||
"""Filter, deduplicate, sort, canonicalize provided genres list.
|
||||
"""Canonicalize, sort and filter a list of genres.
|
||||
|
||||
- Returns an empty list if the input tags list is empty.
|
||||
- If canonicalization is enabled, it extends the list by incorporating
|
||||
parent genres from the canonicalization tree. When a whitelist is set,
|
||||
only parent tags that pass a validity check (_is_valid) are included;
|
||||
otherwise, it adds the oldest ancestor.
|
||||
- During canonicalization, it stops adding parent tags if the count of
|
||||
tags reaches the configured limit (count).
|
||||
otherwise, it adds the oldest ancestor. Adding parent tags is stopped
|
||||
when the count of tags reaches the configured limit (count).
|
||||
- The tags list is then deduplicated to ensure only unique genres are
|
||||
retained.
|
||||
- Optionally, if the 'prefer_specific' configuration is enabled, the
|
||||
list is sorted by the specificity (depth in the canonicalization tree)
|
||||
of the genres.
|
||||
- The method then filters the tag list, ensuring that only valid
|
||||
genres (those that pass the _is_valid method) are kept. If a
|
||||
whitelist is set, only genres in the whitelist are considered valid
|
||||
(which may even result in no genres at all being retained).
|
||||
- Finally, the filtered list of genres, limited to
|
||||
the configured count is returned.
|
||||
- If the 'prefer_specific' configuration is enabled, the list is sorted
|
||||
by the specificity (depth in the canonicalization tree) of the genres.
|
||||
- Finally applies whitelist filtering to ensure that only valid
|
||||
genres are kept. (This may result in no genres at all being retained).
|
||||
- Returns the filtered list of genres, limited to the configured count.
|
||||
"""
|
||||
if not tags:
|
||||
return []
|
||||
|
||||
count = self.config["count"].get(int)
|
||||
|
||||
# Canonicalization (if enabled)
|
||||
if self.canonicalize:
|
||||
# Extend the list to consider tags parents in the c14n tree
|
||||
tags_all = []
|
||||
|
|
@ -242,8 +239,8 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
|
||||
# c14n only adds allowed genres but we may have had forbidden genres in
|
||||
# the original tags list
|
||||
valid_tags = self._filter_valid_genres(tags)
|
||||
return valid_tags[: self.config["count"].get(int)]
|
||||
valid_tags = [t for t in tags if self._is_valid(t)]
|
||||
return valid_tags[:count]
|
||||
|
||||
def fetch_genre(self, lastfm_obj):
|
||||
"""Return the genre for a pylast entity or None if no suitable genre
|
||||
|
|
@ -252,12 +249,6 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
min_weight = self.config["min_weight"].get(int)
|
||||
return self._tags_for(lastfm_obj, min_weight)
|
||||
|
||||
def _filter_valid_genres(self, genres: list[str]) -> list[str]:
|
||||
"""Filter list of genres, only keep valid."""
|
||||
if not genres:
|
||||
return []
|
||||
return [x for x in genres if self._is_valid(x)]
|
||||
|
||||
def _is_valid(self, genre: str) -> bool:
|
||||
"""Check if the genre is valid.
|
||||
|
||||
|
|
@ -281,7 +272,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
"""
|
||||
# Shortcut if we're missing metadata.
|
||||
if any(not s for s in args):
|
||||
return None
|
||||
return []
|
||||
|
||||
key = f"{entity}.{'-'.join(str(a) for a in args)}"
|
||||
if key not in self._genre_cache:
|
||||
|
|
@ -294,29 +285,23 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
return genre
|
||||
|
||||
def fetch_album_genre(self, obj):
|
||||
"""Return the album genre for this Item or Album."""
|
||||
return self._filter_valid_genres(
|
||||
self._last_lookup(
|
||||
"album", LASTFM.get_album, obj.albumartist, obj.album
|
||||
)
|
||||
"""Return raw album genres from Last.fm for this Item or Album."""
|
||||
return self._last_lookup(
|
||||
"album", LASTFM.get_album, obj.albumartist, obj.album
|
||||
)
|
||||
|
||||
def fetch_album_artist_genre(self, obj):
|
||||
"""Return the album artist genre for this Item or Album."""
|
||||
return self._filter_valid_genres(
|
||||
self._last_lookup("artist", LASTFM.get_artist, obj.albumartist)
|
||||
)
|
||||
"""Return raw album artist genres from Last.fm for this Item or Album."""
|
||||
return self._last_lookup("artist", LASTFM.get_artist, obj.albumartist)
|
||||
|
||||
def fetch_artist_genre(self, item):
|
||||
"""Returns the track artist genre for this Item."""
|
||||
return self._filter_valid_genres(
|
||||
self._last_lookup("artist", LASTFM.get_artist, item.artist)
|
||||
)
|
||||
"""Returns raw track artist genres from Last.fm for this Item."""
|
||||
return self._last_lookup("artist", LASTFM.get_artist, item.artist)
|
||||
|
||||
def fetch_track_genre(self, obj):
|
||||
"""Returns the track genre for this Item."""
|
||||
return self._filter_valid_genres(
|
||||
self._last_lookup("track", LASTFM.get_track, obj.artist, obj.title)
|
||||
"""Returns raw track genres from Last.fm for this Item."""
|
||||
return self._last_lookup(
|
||||
"track", LASTFM.get_track, obj.artist, obj.title
|
||||
)
|
||||
|
||||
# Main processing: _get_genre() and helpers.
|
||||
|
|
@ -346,7 +331,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
self, old: list[str], new: list[str]
|
||||
) -> list[str]:
|
||||
"""Combine old and new genres and process via _resolve_genres."""
|
||||
self._log.debug(f"valid last.fm tags: {new}")
|
||||
self._log.debug(f"raw last.fm tags: {new}")
|
||||
self._log.debug(f"existing genres taken into account: {old}")
|
||||
combined = old + new
|
||||
return self._resolve_genres(combined)
|
||||
|
|
|
|||
Loading…
Reference in a new issue