mirror of
https://github.com/beetbox/beets.git
synced 2025-12-16 05:34:47 +01:00
Lastgenre: New config option keep_existing (#4982)
## New config option `keep_existing` (#4982) - Fix the behavior of the`force` option. Previously disabling the option had "incomplete" behaviour: - If content was found, a whitelist check was issued and if valid the plugin exited early and logged ("keep"). - This whitelist check was not aware of multiple genres (separated typically by a string like `, `), thus it failed erased all existing genres and overwrote with new ones. _**This didn't feel like a typical behaviour of a `force` option, which this PR tries to improve as follows...**_ - String-separated multi-genres are now compiled into a list and depending on the `whitelist` option are kept and enriched with freshly fetched last.fm genres. - If force is off, pre-populated tags are not touched. - A lot of refactoring was done, some absolutely required, some as a preparation for future work on the plugin. - The main processing function `_get_genre` was massively overhauled and got a new `pytest.mark.parametrize` test which includes much more test cases.
This commit is contained in:
commit
9682f24c07
3 changed files with 501 additions and 177 deletions
|
|
@ -25,12 +25,14 @@ https://gist.github.com/1241307
|
|||
import codecs
|
||||
import os
|
||||
import traceback
|
||||
from typing import Union
|
||||
|
||||
import pylast
|
||||
import yaml
|
||||
|
||||
from beets import config, library, plugins, ui
|
||||
from beets.util import normpath, plurality
|
||||
from beets.library import Album, Item
|
||||
from beets.util import normpath, plurality, unique_list
|
||||
|
||||
LASTFM = pylast.LastFMNetwork(api_key=plugins.LASTFM_KEY)
|
||||
|
||||
|
|
@ -45,12 +47,6 @@ REPLACE = {
|
|||
}
|
||||
|
||||
|
||||
def deduplicate(seq):
|
||||
"""Remove duplicates from sequence while preserving order."""
|
||||
seen = set()
|
||||
return [x for x in seq if x not in seen and not seen.add(x)]
|
||||
|
||||
|
||||
# Canonicalization tree processing.
|
||||
|
||||
|
||||
|
|
@ -102,14 +98,14 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
"fallback": None,
|
||||
"canonical": False,
|
||||
"source": "album",
|
||||
"force": True,
|
||||
"force": False,
|
||||
"keep_existing": False,
|
||||
"auto": True,
|
||||
"separator": ", ",
|
||||
"prefer_specific": False,
|
||||
"title_case": True,
|
||||
}
|
||||
)
|
||||
|
||||
self.setup()
|
||||
|
||||
def setup(self):
|
||||
|
|
@ -153,17 +149,32 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
flatten_tree(genres_tree, [], self.c14n_branches)
|
||||
|
||||
@property
|
||||
def sources(self):
|
||||
def sources(self) -> tuple[str, ...]:
|
||||
"""A tuple of allowed genre sources. May contain 'track',
|
||||
'album', or 'artist.'
|
||||
"""
|
||||
source = self.config["source"].as_choice(("track", "album", "artist"))
|
||||
if source == "track":
|
||||
return "track", "album", "artist"
|
||||
elif source == "album":
|
||||
if source == "album":
|
||||
return "album", "artist"
|
||||
elif source == "artist":
|
||||
if source == "artist":
|
||||
return ("artist",)
|
||||
return tuple()
|
||||
|
||||
# More canonicalization and general helpers.
|
||||
|
||||
def _to_delimited_genre_string(self, tags: list[str]) -> str:
|
||||
"""Reduce tags list to configured count, format and return as delimited
|
||||
string."""
|
||||
separator = self.config["separator"].as_str()
|
||||
max_count = self.config["count"].get(int)
|
||||
|
||||
genres = tags[:max_count]
|
||||
if self.config["title_case"]:
|
||||
genres = [g.title() for g in genres]
|
||||
|
||||
return separator.join(genres)
|
||||
|
||||
def _get_depth(self, tag):
|
||||
"""Find the depth of a tag in the genres tree."""
|
||||
|
|
@ -183,12 +194,10 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
depth_tag_pairs.sort(reverse=True)
|
||||
return [p[1] for p in depth_tag_pairs]
|
||||
|
||||
def _resolve_genres(self, tags):
|
||||
"""Given a list of strings, return a genre by joining them into a
|
||||
single string and (optionally) canonicalizing each.
|
||||
"""
|
||||
def _resolve_genres(self, tags: list[str]) -> list[str]:
|
||||
"""Filter, deduplicate, sort and canonicalize the given genres."""
|
||||
if not tags:
|
||||
return None
|
||||
return []
|
||||
|
||||
count = self.config["count"].get(int)
|
||||
if self.canonicalize:
|
||||
|
|
@ -201,7 +210,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
parents = [
|
||||
x
|
||||
for x in find_parents(tag, self.c14n_branches)
|
||||
if self._is_allowed(x)
|
||||
if self._is_valid(x)
|
||||
]
|
||||
else:
|
||||
parents = [find_parents(tag, self.c14n_branches)[-1]]
|
||||
|
|
@ -216,7 +225,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
break
|
||||
tags = tags_all
|
||||
|
||||
tags = deduplicate(tags)
|
||||
tags = unique_list(tags)
|
||||
|
||||
# Sort the tags by specificity.
|
||||
if self.config["prefer_specific"]:
|
||||
|
|
@ -224,63 +233,46 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
|
||||
# c14n only adds allowed genres but we may have had forbidden genres in
|
||||
# the original tags list
|
||||
tags = [self._format_tag(x) for x in tags if self._is_allowed(x)]
|
||||
|
||||
return (
|
||||
self.config["separator"]
|
||||
.as_str()
|
||||
.join(tags[: self.config["count"].get(int)])
|
||||
)
|
||||
|
||||
def _format_tag(self, tag):
|
||||
if self.config["title_case"]:
|
||||
return tag.title()
|
||||
return tag
|
||||
return [x for x in tags if self._is_valid(x)]
|
||||
|
||||
def fetch_genre(self, lastfm_obj):
|
||||
"""Return the genre for a pylast entity or None if no suitable genre
|
||||
can be found. Ex. 'Electronic, House, Dance'
|
||||
"""
|
||||
min_weight = self.config["min_weight"].get(int)
|
||||
return self._resolve_genres(self._tags_for(lastfm_obj, min_weight))
|
||||
return self._tags_for(lastfm_obj, min_weight)
|
||||
|
||||
def _is_allowed(self, genre):
|
||||
"""Determine whether the genre is present in the whitelist,
|
||||
returning a boolean.
|
||||
def _is_valid(self, genre: str) -> bool:
|
||||
"""Check if the genre is valid.
|
||||
|
||||
Depending on the whitelist property, valid means a genre is in the
|
||||
whitelist or any genre is allowed.
|
||||
"""
|
||||
if genre is None:
|
||||
return False
|
||||
if not self.whitelist or genre in self.whitelist:
|
||||
if genre and (not self.whitelist or genre.lower() in self.whitelist):
|
||||
return True
|
||||
return False
|
||||
|
||||
# Cached entity lookups.
|
||||
# Cached last.fm entity lookups.
|
||||
|
||||
def _last_lookup(self, entity, method, *args):
|
||||
"""Get a genre based on the named entity using the callable `method`
|
||||
whose arguments are given in the sequence `args`. The genre lookup
|
||||
is cached based on the entity name and the arguments. Before the
|
||||
lookup, each argument is has some Unicode characters replaced with
|
||||
rough ASCII equivalents in order to return better results from the
|
||||
Last.fm database.
|
||||
is cached based on the entity name and the arguments.
|
||||
|
||||
Before the lookup, each argument has the "-" Unicode character replaced
|
||||
with its rough ASCII equivalents in order to return better results from
|
||||
the Last.fm database.
|
||||
"""
|
||||
# Shortcut if we're missing metadata.
|
||||
if any(not s for s in args):
|
||||
return None
|
||||
|
||||
key = "{}.{}".format(entity, "-".join(str(a) for a in args))
|
||||
if key in self._genre_cache:
|
||||
return self._genre_cache[key]
|
||||
else:
|
||||
args_replaced = []
|
||||
for arg in args:
|
||||
for k, v in REPLACE.items():
|
||||
arg = arg.replace(k, v)
|
||||
args_replaced.append(arg)
|
||||
key = f"{entity}.{'-'.join(str(a) for a in args)}"
|
||||
if key not in self._genre_cache:
|
||||
args = [a.replace("\u2010", "-") for a in args]
|
||||
self._genre_cache[key] = self.fetch_genre(method(*args))
|
||||
|
||||
genre = self.fetch_genre(method(*args_replaced))
|
||||
self._genre_cache[key] = genre
|
||||
return genre
|
||||
return self._genre_cache[key]
|
||||
|
||||
def fetch_album_genre(self, obj):
|
||||
"""Return the album genre for this Item or Album."""
|
||||
|
|
@ -302,42 +294,86 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
"track", LASTFM.get_track, obj.artist, obj.title
|
||||
)
|
||||
|
||||
def _get_genre(self, obj):
|
||||
"""Get the genre string for an Album or Item object based on
|
||||
self.sources. Return a `(genre, source)` pair. The
|
||||
prioritization order is:
|
||||
# Main processing: _get_genre() and helpers.
|
||||
|
||||
def _get_existing_genres(self, obj: Union[Album, Item]) -> list[str]:
|
||||
"""Return a list of genres for this Item or Album. Empty string genres
|
||||
are removed."""
|
||||
separator = self.config["separator"].get()
|
||||
if isinstance(obj, library.Item):
|
||||
item_genre = obj.get("genre", with_album=False).split(separator)
|
||||
else:
|
||||
item_genre = obj.get("genre").split(separator)
|
||||
|
||||
# Filter out empty strings
|
||||
return [g for g in item_genre if g]
|
||||
|
||||
def _combine_genres(
|
||||
self, old: list[str], new: list[str]
|
||||
) -> Union[str, None]:
|
||||
"""Combine old and new genres."""
|
||||
self._log.debug(f"fetched last.fm tags: {new}")
|
||||
combined = old + new
|
||||
resolved = self._resolve_genres(combined)
|
||||
return self._to_delimited_genre_string(resolved) or None
|
||||
|
||||
def _get_genre(
|
||||
self, obj: Union[Album, Item]
|
||||
) -> tuple[Union[str, None], ...]:
|
||||
"""Get the final genre string for an Album or Item object.
|
||||
|
||||
`self.sources` specifies allowed genre sources. Starting with the first
|
||||
source in this tuple, the following stages run through until a genre is
|
||||
found or no options are left:
|
||||
- track (for Items only)
|
||||
- album
|
||||
- artist
|
||||
- original
|
||||
- fallback
|
||||
- artist, albumartist or "most popular track genre" (for VA-albums)
|
||||
- original fallback
|
||||
- configured fallback
|
||||
- None
|
||||
|
||||
A `(genre, label)` pair is returned, where `label` is a string used for
|
||||
logging. For example, "keep + artist, whitelist" indicates that existing
|
||||
genres were combined with new last.fm genres and whitelist filtering was
|
||||
applied, while "artist, any" means only new last.fm genres are included
|
||||
and the whitelist feature was disabled.
|
||||
"""
|
||||
keep_genres = []
|
||||
label = ""
|
||||
genres = self._get_existing_genres(obj)
|
||||
|
||||
# Shortcut to existing genre if not forcing.
|
||||
if not self.config["force"] and self._is_allowed(obj.genre):
|
||||
return obj.genre, "keep"
|
||||
|
||||
# Track genre (for Items only).
|
||||
if isinstance(obj, library.Item):
|
||||
if "track" in self.sources:
|
||||
result = self.fetch_track_genre(obj)
|
||||
if result:
|
||||
return result, "track"
|
||||
|
||||
# Album genre.
|
||||
if "album" in self.sources:
|
||||
result = self.fetch_album_genre(obj)
|
||||
if result:
|
||||
return result, "album"
|
||||
|
||||
# Artist (or album artist) genre.
|
||||
if "artist" in self.sources:
|
||||
result = None
|
||||
if genres and not self.config["force"]:
|
||||
# Without force pre-populated tags are returned as-is.
|
||||
if isinstance(obj, library.Item):
|
||||
result = self.fetch_artist_genre(obj)
|
||||
return obj.get("genre", with_album=False), "keep any, no-force"
|
||||
return obj.get("genre"), "keep any, no-force"
|
||||
|
||||
if self.config["force"]:
|
||||
# Force doesn't keep any unless keep_existing is set.
|
||||
# Whitelist validation is handled in _resolve_genres.
|
||||
if self.config["keep_existing"]:
|
||||
keep_genres = [g.lower() for g in genres]
|
||||
|
||||
# Run through stages: track, album, artist,
|
||||
# album artist, or most popular track genre.
|
||||
if (
|
||||
isinstance(obj, library.Item)
|
||||
and "track" in self.sources
|
||||
and (new_genres := self.fetch_track_genre(obj))
|
||||
):
|
||||
label = "track"
|
||||
elif "album" in self.sources and (
|
||||
new_genres := self.fetch_album_genre(obj)
|
||||
):
|
||||
label = "album"
|
||||
elif "artist" in self.sources:
|
||||
new_genres = None
|
||||
if isinstance(obj, library.Item):
|
||||
new_genres = self.fetch_artist_genre(obj)
|
||||
label = "artist"
|
||||
elif obj.albumartist != config["va_name"].as_str():
|
||||
result = self.fetch_album_artist_genre(obj)
|
||||
new_genres = self.fetch_album_artist_genre(obj)
|
||||
label = "album artist"
|
||||
else:
|
||||
# For "Various Artists", pick the most popular track genre.
|
||||
item_genres = []
|
||||
|
|
@ -348,26 +384,39 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
if not item_genre:
|
||||
item_genre = self.fetch_artist_genre(item)
|
||||
if item_genre:
|
||||
item_genres.append(item_genre)
|
||||
item_genres += item_genre
|
||||
if item_genres:
|
||||
result, _ = plurality(item_genres)
|
||||
most_popular, rank = plurality(item_genres)
|
||||
new_genres = [most_popular]
|
||||
label = "most popular track"
|
||||
self._log.debug(
|
||||
'Most popular track genre "{}" ({}) for VA album.',
|
||||
most_popular,
|
||||
rank,
|
||||
)
|
||||
|
||||
if result:
|
||||
return result, "artist"
|
||||
# Return with a combined or freshly fetched genre list.
|
||||
if new_genres:
|
||||
suffix = "whitelist" if self.whitelist else "any"
|
||||
label += f", {suffix}"
|
||||
|
||||
# Filter the existing genre.
|
||||
if keep_genres:
|
||||
label = f"keep + {label}"
|
||||
return self._combine_genres(keep_genres, new_genres), label
|
||||
|
||||
# Nothing found, leave original.
|
||||
if obj.genre:
|
||||
result = self._resolve_genres([obj.genre])
|
||||
if result:
|
||||
return result, "original"
|
||||
return obj.genre, "original fallback"
|
||||
|
||||
# Fallback string.
|
||||
fallback = self.config["fallback"].get()
|
||||
if fallback:
|
||||
# No original, return fallback string.
|
||||
if fallback := self.config["fallback"].get():
|
||||
return fallback, "fallback"
|
||||
|
||||
# No fallback configured.
|
||||
return None, None
|
||||
|
||||
# Beets plugin hooks and CLI.
|
||||
|
||||
def commands(self):
|
||||
lastgenre_cmd = ui.Subcommand("lastgenre", help="fetch genres")
|
||||
lastgenre_cmd.parser.add_option(
|
||||
|
|
@ -375,7 +424,28 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
"--force",
|
||||
dest="force",
|
||||
action="store_true",
|
||||
help="re-download genre when already present",
|
||||
help="modify existing genres",
|
||||
)
|
||||
lastgenre_cmd.parser.add_option(
|
||||
"-F",
|
||||
"--no-force",
|
||||
dest="force",
|
||||
action="store_false",
|
||||
help="don't modify existing genres",
|
||||
)
|
||||
lastgenre_cmd.parser.add_option(
|
||||
"-k",
|
||||
"--keep-existing",
|
||||
dest="keep_existing",
|
||||
action="store_true",
|
||||
help="combine with existing genres when modifying",
|
||||
)
|
||||
lastgenre_cmd.parser.add_option(
|
||||
"-K",
|
||||
"--no-keep-existing",
|
||||
dest="keep_existing",
|
||||
action="store_false",
|
||||
help="don't combine with existing genres when modifying",
|
||||
)
|
||||
lastgenre_cmd.parser.add_option(
|
||||
"-s",
|
||||
|
|
@ -396,7 +466,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
"--albums",
|
||||
action="store_true",
|
||||
dest="album",
|
||||
help="match albums instead of items",
|
||||
help="match albums instead of items (default)",
|
||||
)
|
||||
lastgenre_cmd.parser.set_defaults(album=True)
|
||||
|
||||
|
|
|
|||
|
|
@ -25,7 +25,7 @@ tags can be considered genres. This way, tags like "my favorite music" or "seen
|
|||
live" won't be considered genres. The plugin ships with a fairly extensive
|
||||
`internal whitelist`_, but you can set your own in the config file using the
|
||||
``whitelist`` configuration value or forgo a whitelist altogether by setting
|
||||
the option to `false`.
|
||||
the option to ``no``.
|
||||
|
||||
The genre list file should contain one genre per line. Blank lines are ignored.
|
||||
For the curious, the default genre list is generated by a `script that scrapes
|
||||
|
|
@ -111,6 +111,53 @@ Last.fm returns both of those tags, lastgenre is going to use the most
|
|||
popular, which is often the most generic (in this case ``folk``). By setting
|
||||
``prefer_specific`` to true, lastgenre would use ``americana`` instead.
|
||||
|
||||
Handling pre-populated tags
|
||||
^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
The ``force``, ``keep_existing`` and ``whitelist`` options control how
|
||||
pre-existing genres are handled.
|
||||
|
||||
As you would assume, setting ``force: no`` **won't touch pre-existing genre
|
||||
tags** and will only **fetch new genres for empty tags**. When ``force`` is
|
||||
``yes`` the setting of the ``whitelist`` option (as documented in `Usage`_)
|
||||
applies to any existing or newly fetched genres.
|
||||
|
||||
The follwing configurations are possible:
|
||||
|
||||
**Setup 1** (default)
|
||||
|
||||
Add new last.fm genres when **empty**. Any present tags stay **untouched**.
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
force: no
|
||||
keep_existing: no
|
||||
|
||||
**Setup 2**
|
||||
|
||||
**Overwrite all**. Only fresh last.fm genres remain.
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
force: yes
|
||||
keep_existing: no
|
||||
|
||||
**Setup 3**
|
||||
|
||||
**Combine** genres in present tags with new ones (be aware of that with an
|
||||
enabled ``whitelist`` setting, of course some genres might get cleaned up. To
|
||||
make sure any existing genres remain, set ``whitelist: no``).
|
||||
|
||||
.. code-block:: yaml
|
||||
|
||||
force: yes
|
||||
keep_existing: yes
|
||||
|
||||
If ``force`` is disabled the ``keep_existing`` option is simply ignored (since ``force:
|
||||
no`` means `not touching` existing tags anyway).
|
||||
|
||||
|
||||
|
||||
Configuration
|
||||
-------------
|
||||
|
||||
|
|
@ -128,14 +175,22 @@ configuration file. The available options are:
|
|||
- **fallback**: A string if to use a fallback genre when no genre is found.
|
||||
You can use the empty string ``''`` to reset the genre.
|
||||
Default: None.
|
||||
- **force**: By default, beets will always fetch new genres, even if the files
|
||||
already have one. To instead leave genres in place in when they pass the
|
||||
whitelist, set the ``force`` option to ``no``.
|
||||
Default: ``yes``.
|
||||
- **force**: By default, lastgenre will fetch new genres for empty tags only.
|
||||
Enable the ``keep_existing`` option to combine existing and new genres. (see
|
||||
`Handling pre-populated tags`_).
|
||||
Default: ``no``.
|
||||
- **keep_existing**: This option alters the ``force`` behaviour.
|
||||
If both ``force`` and ``keep_existing`` are enabled, existing genres are
|
||||
combined with new ones. Depending on the ``whitelist`` setting, existing and
|
||||
new genres are filtered accordingly. To ensure only fresh last.fm genres,
|
||||
disable this option. (see `Handling pre-populated tags`_)
|
||||
Default: ``no``.
|
||||
- **min_weight**: Minimum popularity factor below which genres are discarded.
|
||||
Default: 10.
|
||||
- **prefer_specific**: Sort genres by the most to least specific, rather than
|
||||
most to least popular. Default: ``no``.
|
||||
most to least popular. Note that this option requires a ``canonical`` tree,
|
||||
and if not configured it will automatically enable and use the built-in tree.
|
||||
Default: ``no``.
|
||||
- **source**: Which entity to look up in Last.fm. Can be
|
||||
either ``artist``, ``album`` or ``track``.
|
||||
Default: ``album``.
|
||||
|
|
|
|||
|
|
@ -16,6 +16,8 @@
|
|||
|
||||
from unittest.mock import Mock
|
||||
|
||||
import pytest
|
||||
|
||||
from beets import config
|
||||
from beets.test import _common
|
||||
from beets.test.helper import BeetsTestCase
|
||||
|
|
@ -44,46 +46,49 @@ class LastGenrePluginTest(BeetsTestCase):
|
|||
def test_default(self):
|
||||
"""Fetch genres with whitelist and c14n deactivated"""
|
||||
self._setup_config()
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == "Delta Blues"
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == ["delta blues"]
|
||||
|
||||
def test_c14n_only(self):
|
||||
"""Default c14n tree funnels up to most common genre except for *wrong*
|
||||
genres that stay unchanged.
|
||||
"""
|
||||
self._setup_config(canonical=True, count=99)
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == "Blues"
|
||||
assert self.plugin._resolve_genres(["iota blues"]) == "Iota Blues"
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == ["blues"]
|
||||
assert self.plugin._resolve_genres(["iota blues"]) == ["iota blues"]
|
||||
|
||||
def test_whitelist_only(self):
|
||||
"""Default whitelist rejects *wrong* (non existing) genres."""
|
||||
self._setup_config(whitelist=True)
|
||||
assert self.plugin._resolve_genres(["iota blues"]) == ""
|
||||
assert self.plugin._resolve_genres(["iota blues"]) == []
|
||||
|
||||
def test_whitelist_c14n(self):
|
||||
"""Default whitelist and c14n both activated result in all parents
|
||||
genres being selected (from specific to common).
|
||||
"""
|
||||
self._setup_config(canonical=True, whitelist=True, count=99)
|
||||
assert (
|
||||
self.plugin._resolve_genres(["delta blues"]) == "Delta Blues, Blues"
|
||||
)
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == [
|
||||
"delta blues",
|
||||
"blues",
|
||||
]
|
||||
|
||||
def test_whitelist_custom(self):
|
||||
"""Keep only genres that are in the whitelist."""
|
||||
self._setup_config(whitelist={"blues", "rock", "jazz"}, count=2)
|
||||
assert self.plugin._resolve_genres(["pop", "blues"]) == "Blues"
|
||||
assert self.plugin._resolve_genres(["pop", "blues"]) == ["blues"]
|
||||
|
||||
self._setup_config(canonical="", whitelist={"rock"})
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == ""
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == []
|
||||
|
||||
def test_count(self):
|
||||
"""Keep the n first genres, as we expect them to be sorted from more to
|
||||
less popular.
|
||||
def test_to_delimited_string(self):
|
||||
"""Keep the n first genres, format them and return a
|
||||
separator-delimited string.
|
||||
"""
|
||||
self._setup_config(whitelist={"blues", "rock", "jazz"}, count=2)
|
||||
self._setup_config(count=2)
|
||||
assert (
|
||||
self.plugin._resolve_genres(["jazz", "pop", "rock", "blues"])
|
||||
== "Jazz, Rock"
|
||||
self.plugin._to_delimited_genre_string(
|
||||
["jazz", "pop", "rock", "blues"]
|
||||
)
|
||||
== "Jazz, Pop"
|
||||
)
|
||||
|
||||
def test_count_c14n(self):
|
||||
|
|
@ -93,31 +98,28 @@ class LastGenrePluginTest(BeetsTestCase):
|
|||
)
|
||||
# thanks to c14n, 'blues' superseeds 'country blues' and takes the
|
||||
# second slot
|
||||
assert (
|
||||
self.plugin._resolve_genres(
|
||||
["jazz", "pop", "country blues", "rock"]
|
||||
)
|
||||
== "Jazz, Blues"
|
||||
)
|
||||
assert self.plugin._resolve_genres(
|
||||
["jazz", "pop", "country blues", "rock"]
|
||||
) == ["jazz", "blues"]
|
||||
|
||||
def test_c14n_whitelist(self):
|
||||
"""Genres first pass through c14n and are then filtered"""
|
||||
self._setup_config(canonical=True, whitelist={"rock"})
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == ""
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == []
|
||||
|
||||
def test_empty_string_enables_canonical(self):
|
||||
"""For backwards compatibility, setting the `canonical` option
|
||||
to the empty string enables it using the default tree.
|
||||
"""
|
||||
self._setup_config(canonical="", count=99)
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == "Blues"
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == ["blues"]
|
||||
|
||||
def test_empty_string_enables_whitelist(self):
|
||||
"""Again for backwards compatibility, setting the `whitelist`
|
||||
option to the empty string enables the default set of genres.
|
||||
"""
|
||||
self._setup_config(whitelist="")
|
||||
assert self.plugin._resolve_genres(["iota blues"]) == ""
|
||||
assert self.plugin._resolve_genres(["iota blues"]) == []
|
||||
|
||||
def test_prefer_specific_loads_tree(self):
|
||||
"""When prefer_specific is enabled but canonical is not the
|
||||
|
|
@ -129,15 +131,15 @@ class LastGenrePluginTest(BeetsTestCase):
|
|||
def test_prefer_specific_without_canonical(self):
|
||||
"""Prefer_specific works without canonical."""
|
||||
self._setup_config(prefer_specific=True, canonical=False, count=4)
|
||||
assert (
|
||||
self.plugin._resolve_genres(["math rock", "post-rock"])
|
||||
== "Post-Rock, Math Rock"
|
||||
)
|
||||
assert self.plugin._resolve_genres(["math rock", "post-rock"]) == [
|
||||
"post-rock",
|
||||
"math rock",
|
||||
]
|
||||
|
||||
def test_no_duplicate(self):
|
||||
"""Remove duplicated genres."""
|
||||
self._setup_config(count=99)
|
||||
assert self.plugin._resolve_genres(["blues", "blues"]) == "Blues"
|
||||
assert self.plugin._resolve_genres(["blues", "blues"]) == ["blues"]
|
||||
|
||||
def test_tags_for(self):
|
||||
class MockPylastElem:
|
||||
|
|
@ -163,51 +165,6 @@ class LastGenrePluginTest(BeetsTestCase):
|
|||
res = plugin._tags_for(MockPylastObj(), min_weight=50)
|
||||
assert res == ["pop"]
|
||||
|
||||
def test_get_genre(self):
|
||||
mock_genres = {"track": "1", "album": "2", "artist": "3"}
|
||||
|
||||
def mock_fetch_track_genre(self, obj=None):
|
||||
return mock_genres["track"]
|
||||
|
||||
def mock_fetch_album_genre(self, obj):
|
||||
return mock_genres["album"]
|
||||
|
||||
def mock_fetch_artist_genre(self, obj):
|
||||
return mock_genres["artist"]
|
||||
|
||||
lastgenre.LastGenrePlugin.fetch_track_genre = mock_fetch_track_genre
|
||||
lastgenre.LastGenrePlugin.fetch_album_genre = mock_fetch_album_genre
|
||||
lastgenre.LastGenrePlugin.fetch_artist_genre = mock_fetch_artist_genre
|
||||
|
||||
self._setup_config(whitelist=False)
|
||||
item = _common.item()
|
||||
item.genre = mock_genres["track"]
|
||||
|
||||
config["lastgenre"] = {"force": False}
|
||||
res = self.plugin._get_genre(item)
|
||||
assert res == (item.genre, "keep")
|
||||
|
||||
config["lastgenre"] = {"force": True, "source": "track"}
|
||||
res = self.plugin._get_genre(item)
|
||||
assert res == (mock_genres["track"], "track")
|
||||
|
||||
config["lastgenre"] = {"source": "album"}
|
||||
res = self.plugin._get_genre(item)
|
||||
assert res == (mock_genres["album"], "album")
|
||||
|
||||
config["lastgenre"] = {"source": "artist"}
|
||||
res = self.plugin._get_genre(item)
|
||||
assert res == (mock_genres["artist"], "artist")
|
||||
|
||||
mock_genres["artist"] = None
|
||||
res = self.plugin._get_genre(item)
|
||||
assert res == (item.genre, "original")
|
||||
|
||||
config["lastgenre"] = {"fallback": "rap"}
|
||||
item.genre = None
|
||||
res = self.plugin._get_genre(item)
|
||||
assert res == (config["lastgenre"]["fallback"].get(), "fallback")
|
||||
|
||||
def test_sort_by_depth(self):
|
||||
self._setup_config(canonical=True)
|
||||
# Normal case.
|
||||
|
|
@ -218,3 +175,245 @@ class LastGenrePluginTest(BeetsTestCase):
|
|||
tags = ("electronic", "ambient", "chillout")
|
||||
res = self.plugin._sort_by_depth(tags)
|
||||
assert res == ["ambient", "electronic"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"config_values, item_genre, mock_genres, expected_result",
|
||||
[
|
||||
# 0 - force and keep whitelisted
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "album", # means album or artist genre
|
||||
"whitelist": True,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
"count": 10,
|
||||
},
|
||||
"Blues",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Blues, Jazz", "keep + album, whitelist"),
|
||||
),
|
||||
# 1 - force and keep whitelisted, unknown original
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "album",
|
||||
"whitelist": True,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"original unknown, Blues",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Blues, Jazz", "keep + album, whitelist"),
|
||||
),
|
||||
# 2 - force and keep whitelisted on empty tag
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "album",
|
||||
"whitelist": True,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Jazz", "album, whitelist"),
|
||||
),
|
||||
# 3 force and keep, artist configured
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "artist", # means artist genre, original or fallback
|
||||
"whitelist": True,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"original unknown, Blues",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
"artist": ["Pop"],
|
||||
},
|
||||
("Blues, Pop", "keep + artist, whitelist"),
|
||||
),
|
||||
# 4 - don't force, disabled whitelist
|
||||
(
|
||||
{
|
||||
"force": False,
|
||||
"keep_existing": False,
|
||||
"source": "album",
|
||||
"whitelist": False,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"any genre",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("any genre", "keep any, no-force"),
|
||||
),
|
||||
# 5 - don't force and empty is regular last.fm fetch; no whitelist too
|
||||
(
|
||||
{
|
||||
"force": False,
|
||||
"keep_existing": False,
|
||||
"source": "album",
|
||||
"whitelist": False,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"",
|
||||
{
|
||||
"album": ["Jazzin"],
|
||||
},
|
||||
("Jazzin", "album, any"),
|
||||
),
|
||||
# 6 - fallback to next stages until found
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "track", # means track,album,artist,...
|
||||
"whitelist": False,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"unknown genre",
|
||||
{
|
||||
"track": None,
|
||||
"album": None,
|
||||
"artist": ["Jazz"],
|
||||
},
|
||||
("Unknown Genre, Jazz", "keep + artist, any"),
|
||||
),
|
||||
# 7 - fallback to original when nothing found
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "track",
|
||||
"whitelist": True,
|
||||
"fallback": "fallback genre",
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"original unknown",
|
||||
{
|
||||
"track": None,
|
||||
"album": None,
|
||||
"artist": None,
|
||||
},
|
||||
("original unknown", "original fallback"),
|
||||
),
|
||||
# 8 - fallback to fallback if no original
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "track",
|
||||
"whitelist": True,
|
||||
"fallback": "fallback genre",
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"",
|
||||
{
|
||||
"track": None,
|
||||
"album": None,
|
||||
"artist": None,
|
||||
},
|
||||
("fallback genre", "fallback"),
|
||||
),
|
||||
# 9 - null charachter as separator
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "album",
|
||||
"whitelist": True,
|
||||
"separator": "\u0000",
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
},
|
||||
"Blues",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Blues\u0000Jazz", "keep + album, whitelist"),
|
||||
),
|
||||
# 10 - limit a lot of results
|
||||
(
|
||||
{
|
||||
"force": True,
|
||||
"keep_existing": True,
|
||||
"source": "album",
|
||||
"whitelist": True,
|
||||
"count": 5,
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
"separator": ", ",
|
||||
},
|
||||
"original unknown, Blues, Rock, Folk, Metal",
|
||||
{
|
||||
"album": ["Jazz", "Bebop", "Hardbop"],
|
||||
},
|
||||
("Blues, Rock, Metal, Jazz, Bebop", "keep + album, whitelist"),
|
||||
),
|
||||
# 11 - force off does not rely on configured separator
|
||||
(
|
||||
{
|
||||
"force": False,
|
||||
"keep_existing": False,
|
||||
"source": "album",
|
||||
"whitelist": True,
|
||||
"count": 2,
|
||||
"separator": ", ",
|
||||
},
|
||||
"not ; configured | separator",
|
||||
{
|
||||
"album": ["Jazz", "Bebop"],
|
||||
},
|
||||
("not ; configured | separator", "keep any, no-force"),
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_get_genre(config_values, item_genre, mock_genres, expected_result):
|
||||
"""Test _get_genre with various configurations."""
|
||||
|
||||
def mock_fetch_track_genre(self, obj=None):
|
||||
return mock_genres["track"]
|
||||
|
||||
def mock_fetch_album_genre(self, obj):
|
||||
return mock_genres["album"]
|
||||
|
||||
def mock_fetch_artist_genre(self, obj):
|
||||
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.LastGenrePlugin.fetch_track_genre = mock_fetch_track_genre
|
||||
lastgenre.LastGenrePlugin.fetch_album_genre = mock_fetch_album_genre
|
||||
lastgenre.LastGenrePlugin.fetch_artist_genre = mock_fetch_artist_genre
|
||||
|
||||
# Configure
|
||||
config["lastgenre"] = config_values
|
||||
|
||||
# Initialize plugin instance and item
|
||||
plugin = lastgenre.LastGenrePlugin()
|
||||
item = _common.item()
|
||||
item.genre = item_genre
|
||||
|
||||
# Run
|
||||
res = plugin._get_genre(item)
|
||||
assert res == expected_result
|
||||
|
|
|
|||
Loading…
Reference in a new issue