mirror of
https://github.com/beetbox/beets.git
synced 2026-02-09 00:41:57 +01:00
fix(lastgenre): Canonicalize keep_existing fallback
Fixes a bug where existing tags were set to None, if they weren't whitelisted, but an whitelisted canonicalized parent existed up the tree. In all other cases, the original genres are canonicalized and considered for the final genre, except in the keep_existing logic branch. This PR fixes the issue and results in the expected behavior for this combination of options. For the bug to trigger several conditions had to be met: - Canonicalization is enabled and a whitelist is specified. - `force` and `keep_existing` are set. Meaning, that Lastfm is queried for a genre, but the existing genres are still left around when none are found online. - A release with a non-whitelisted genre exists, but that genre has a whitelisted genre parent up the tree. - That very release has no genre on lastfm. This is rather convoluted, but stay with me :D What would happen is the following: - `keep_genres` is set to the existing genres, as `force` and `keep_existing` is set. - Genres for `track`/`album`/`artist` aren't found for this release, as they don't exist in lastfm. - Then the `keep_existing` logic is entered. - The old logic only checks if the existing genres have an **exact** match for the whitelist. In contrast to all other code branches, we don't do the `_try_resolve_stage` in case there's no direct match, resulting in no match. - We continue to the fallback logic, which returns the fallback (`None` in my case) This patch results in one last try to resolve the existing genres when `keep_existing` is set, which includes canonicalization (if enabled).
This commit is contained in:
parent
aaf046e6bd
commit
4d7b9cb14b
3 changed files with 35 additions and 0 deletions
|
|
@ -482,6 +482,13 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
if obj.genre and self.config["keep_existing"]:
|
||||
if not self.whitelist or self._is_valid(obj.genre.lower()):
|
||||
return obj.genre, "original fallback"
|
||||
else:
|
||||
# If the original genre doesn't match a whitelisted genre, check
|
||||
# if we can canonicalize it to find a matching, whitelisted genre!
|
||||
if result := _try_resolve_stage(
|
||||
"original fallback", keep_genres, []
|
||||
):
|
||||
return result
|
||||
|
||||
# Return fallback string.
|
||||
if fallback := self.config["fallback"].get():
|
||||
|
|
|
|||
|
|
@ -57,6 +57,9 @@ New features:
|
|||
|
||||
Bug fixes:
|
||||
|
||||
- :doc:`/plugins/lastgenre`: Canonicalize genres when ``force`` and
|
||||
``keep_existing`` are ``on``, yet no genre info on lastfm could be found.
|
||||
:bug:`6303`
|
||||
- Handle potential OSError when unlinking temporary files in ArtResizer.
|
||||
:bug:`5615`
|
||||
- :doc:`/plugins/spotify`: Updated Spotify API credentials. :bug:`6270`
|
||||
|
|
|
|||
|
|
@ -541,6 +541,31 @@ class LastGenrePluginTest(PluginTestCase):
|
|||
"keep + album, whitelist",
|
||||
),
|
||||
),
|
||||
# 16 - canonicalization transforms non-whitelisted original genres to canonical
|
||||
# forms and deduplication works, **even** when no new genres are found online.
|
||||
#
|
||||
# "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the
|
||||
# tree to "Disco" and "Electronic".
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "album",
|
||||
"whitelist": True,
|
||||
"canonical": True,
|
||||
"prefer_specific": False,
|
||||
"count": 10,
|
||||
},
|
||||
"Cosmic Disco",
|
||||
{
|
||||
"album": [],
|
||||
"artist": [],
|
||||
},
|
||||
(
|
||||
"Disco, Electronic",
|
||||
"keep + original fallback, whitelist",
|
||||
),
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_get_genre(config_values, item_genre, mock_genres, expected_result):
|
||||
|
|
|
|||
Loading…
Reference in a new issue