mirror of
https://github.com/beetbox/beets.git
synced 2026-02-22 23:33:50 +01:00
Add native support for multiple genres per album/track
Simplify multi-genre implementation based on maintainer feedback (PR #6169). Changes: - Remove multi_value_genres and genre_separator config options - Replace complex sync_genre_fields() with ensure_first_value('genre', 'genres') - Update all plugins (Beatport, MusicBrainz, LastGenre) to always write genres as lists - Add automatic migration for comma/semicolon/slash-separated genre strings - Add 'beet migrate genres' command for explicit batch migration with --pretend flag - Update all tests to reflect simplified approach (44 tests passing) - Update documentation Implementation aligns with maintainer vision of always using multi-value genres internally with automatic backward-compatible sync to the genre field via ensure_first_value(), eliminating configuration complexity. Migration strategy avoids problems from #5540: - Automatic lazy migration on item access (no reimport/mbsync needed) - Optional batch migration command for user control - No endless rewrite loops due to proper field synchronization
This commit is contained in:
Johann Fot
Šarūnas Nejus
parent
940e94c48c
commit
ee5f96be78
14 changed files with 421 additions and 81 deletions
|
|
@ -166,7 +166,38 @@ def correct_list_fields(m: LibModel) -> None:
|
|||
elif list_val:
|
||||
setattr(m, single_field, list_val[0])
|
||||
|
||||
def migrate_legacy_genres() -> None:
|
||||
"""Migrate comma-separated genre strings to genres list.
|
||||
|
||||
For users upgrading from previous versions, their genre field may
|
||||
contain comma-separated values (e.g., "Rock, Alternative, Indie").
|
||||
This migration splits those values into the genres list on first access,
|
||||
avoiding the need to reimport the entire library.
|
||||
"""
|
||||
genre_val = getattr(m, "genre", "")
|
||||
genres_val = getattr(m, "genres", [])
|
||||
|
||||
# Only migrate if genres list is empty and genre contains separators
|
||||
if not genres_val and genre_val:
|
||||
# Try common separators used by lastgenre and other tools
|
||||
for separator in [", ", "; ", " / "]:
|
||||
if separator in genre_val:
|
||||
# Split and clean the genre string
|
||||
split_genres = [
|
||||
g.strip()
|
||||
for g in genre_val.split(separator)
|
||||
if g.strip()
|
||||
]
|
||||
if len(split_genres) > 1:
|
||||
# Found a valid split - populate genres list
|
||||
setattr(m, "genres", split_genres)
|
||||
# Clear genre so ensure_first_value sets it correctly
|
||||
setattr(m, "genre", "")
|
||||
break
|
||||
|
||||
ensure_first_value("albumtype", "albumtypes")
|
||||
migrate_legacy_genres()
|
||||
ensure_first_value("genre", "genres")
|
||||
|
||||
if hasattr(m, "mb_artistids"):
|
||||
ensure_first_value("mb_artistid", "mb_artistids")
|
||||
|
|
|
|||
|
|
@ -76,6 +76,7 @@ class Info(AttrDict[Any]):
|
|||
data_source: str | None = None,
|
||||
data_url: str | None = None,
|
||||
genre: str | None = None,
|
||||
genres: list[str] | None = None,
|
||||
media: str | None = None,
|
||||
**kwargs,
|
||||
) -> None:
|
||||
|
|
@ -91,6 +92,7 @@ class Info(AttrDict[Any]):
|
|||
self.data_source = data_source
|
||||
self.data_url = data_url
|
||||
self.genre = genre
|
||||
self.genres = genres or []
|
||||
self.media = media
|
||||
self.update(kwargs)
|
||||
|
||||
|
|
|
|||
|
|
@ -242,6 +242,7 @@ class Album(LibModel):
|
|||
"albumartists_credit": types.MULTI_VALUE_DSV,
|
||||
"album": types.STRING,
|
||||
"genre": types.STRING,
|
||||
"genres": types.MULTI_VALUE_DSV,
|
||||
"style": types.STRING,
|
||||
"discogs_albumid": types.INTEGER,
|
||||
"discogs_artistid": types.INTEGER,
|
||||
|
|
@ -298,6 +299,7 @@ class Album(LibModel):
|
|||
"albumartists_credit",
|
||||
"album",
|
||||
"genre",
|
||||
"genres",
|
||||
"style",
|
||||
"discogs_albumid",
|
||||
"discogs_artistid",
|
||||
|
|
@ -651,6 +653,7 @@ class Item(LibModel):
|
|||
"albumartist_credit": types.STRING,
|
||||
"albumartists_credit": types.MULTI_VALUE_DSV,
|
||||
"genre": types.STRING,
|
||||
"genres": types.MULTI_VALUE_DSV,
|
||||
"style": types.STRING,
|
||||
"discogs_albumid": types.INTEGER,
|
||||
"discogs_artistid": types.INTEGER,
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ from .fields import fields_cmd
|
|||
from .help import HelpCommand
|
||||
from .import_ import import_cmd
|
||||
from .list import list_cmd
|
||||
from .migrate import migrate_cmd
|
||||
from .modify import modify_cmd
|
||||
from .move import move_cmd
|
||||
from .remove import remove_cmd
|
||||
|
|
@ -52,12 +53,13 @@ default_commands = [
|
|||
HelpCommand(),
|
||||
import_cmd,
|
||||
list_cmd,
|
||||
update_cmd,
|
||||
remove_cmd,
|
||||
stats_cmd,
|
||||
version_cmd,
|
||||
migrate_cmd,
|
||||
modify_cmd,
|
||||
move_cmd,
|
||||
remove_cmd,
|
||||
stats_cmd,
|
||||
update_cmd,
|
||||
version_cmd,
|
||||
write_cmd,
|
||||
config_cmd,
|
||||
completion_cmd,
|
||||
|
|
|
|||
98
beets/ui/commands/migrate.py
Normal file
98
beets/ui/commands/migrate.py
Normal file
|
|
@ -0,0 +1,98 @@
|
|||
"""The 'migrate' command: migrate library data for format changes."""
|
||||
|
||||
from beets import logging, ui
|
||||
from beets.autotag import correct_list_fields
|
||||
|
||||
# Global logger.
|
||||
log = logging.getLogger("beets")
|
||||
|
||||
|
||||
def migrate_genres(lib, pretend=False):
|
||||
"""Migrate comma-separated genre strings to genres list.
|
||||
|
||||
For users upgrading from previous versions, their genre field may
|
||||
contain comma-separated values (e.g., "Rock, Alternative, Indie").
|
||||
This command splits those values into the genres list, avoiding
|
||||
the need to reimport the entire library.
|
||||
"""
|
||||
items = lib.items()
|
||||
migrated_count = 0
|
||||
total_items = 0
|
||||
|
||||
ui.print_("Scanning library for items with comma-separated genres...")
|
||||
|
||||
for item in items:
|
||||
total_items += 1
|
||||
genre_val = item.genre or ""
|
||||
genres_val = item.genres or []
|
||||
|
||||
# Check if migration is needed
|
||||
needs_migration = False
|
||||
if not genres_val and genre_val:
|
||||
for separator in [", ", "; ", " / "]:
|
||||
if separator in genre_val:
|
||||
split_genres = [
|
||||
g.strip()
|
||||
for g in genre_val.split(separator)
|
||||
if g.strip()
|
||||
]
|
||||
if len(split_genres) > 1:
|
||||
needs_migration = True
|
||||
break
|
||||
|
||||
if needs_migration:
|
||||
migrated_count += 1
|
||||
old_genre = item.genre
|
||||
old_genres = item.genres or []
|
||||
|
||||
if pretend:
|
||||
# Just show what would change
|
||||
ui.print_(
|
||||
f" Would migrate: {item.artist} - {item.title}\n"
|
||||
f" genre: {old_genre!r} -> {split_genres[0]!r}\n"
|
||||
f" genres: {old_genres!r} -> {split_genres!r}"
|
||||
)
|
||||
else:
|
||||
# Actually migrate
|
||||
correct_list_fields(item)
|
||||
item.store()
|
||||
log.debug(
|
||||
"migrated: {} - {} ({} -> {})",
|
||||
item.artist,
|
||||
item.title,
|
||||
old_genre,
|
||||
item.genres,
|
||||
)
|
||||
|
||||
# Show summary
|
||||
if pretend:
|
||||
ui.print_(
|
||||
f"\nWould migrate {migrated_count} of {total_items} items "
|
||||
f"(run without --pretend to apply changes)"
|
||||
)
|
||||
else:
|
||||
ui.print_(
|
||||
f"\nMigrated {migrated_count} of {total_items} items with "
|
||||
f"comma-separated genres"
|
||||
)
|
||||
|
||||
|
||||
def migrate_func(lib, opts, args):
|
||||
"""Handle the migrate command."""
|
||||
if not args or args[0] == "genres":
|
||||
migrate_genres(lib, pretend=opts.pretend)
|
||||
else:
|
||||
raise ui.UserError(f"unknown migration target: {args[0]}")
|
||||
|
||||
|
||||
migrate_cmd = ui.Subcommand(
|
||||
"migrate", help="migrate library data for format changes"
|
||||
)
|
||||
migrate_cmd.parser.add_option(
|
||||
"-p",
|
||||
"--pretend",
|
||||
action="store_true",
|
||||
help="show what would be changed without applying",
|
||||
)
|
||||
migrate_cmd.parser.usage = "%prog migrate genres [options]"
|
||||
migrate_cmd.func = migrate_func
|
||||
|
|
@ -234,7 +234,9 @@ class BeatportObject:
|
|||
if "artists" in data:
|
||||
self.artists = [(x["id"], str(x["name"])) for x in data["artists"]]
|
||||
if "genres" in data:
|
||||
self.genres = [str(x["name"]) for x in data["genres"]]
|
||||
genre_list = [str(x["name"]) for x in data["genres"]]
|
||||
# Remove duplicates while preserving order
|
||||
self.genres = list(dict.fromkeys(genre_list))
|
||||
|
||||
def artists_str(self) -> str | None:
|
||||
if self.artists is not None:
|
||||
|
|
@ -306,11 +308,16 @@ class BeatportTrack(BeatportObject):
|
|||
self.bpm = data.get("bpm")
|
||||
self.initial_key = str((data.get("key") or {}).get("shortName"))
|
||||
|
||||
# Use 'subgenre' and if not present, 'genre' as a fallback.
|
||||
# Extract genres list from subGenres or genres
|
||||
if data.get("subGenres"):
|
||||
self.genre = str(data["subGenres"][0].get("name"))
|
||||
genre_list = [str(x.get("name")) for x in data["subGenres"]]
|
||||
elif data.get("genres"):
|
||||
self.genre = str(data["genres"][0].get("name"))
|
||||
genre_list = [str(x.get("name")) for x in data["genres"]]
|
||||
else:
|
||||
genre_list = []
|
||||
|
||||
# Remove duplicates while preserving order
|
||||
self.genres = list(dict.fromkeys(genre_list))
|
||||
|
||||
|
||||
class BeatportPlugin(MetadataSourcePlugin):
|
||||
|
|
@ -484,6 +491,7 @@ class BeatportPlugin(MetadataSourcePlugin):
|
|||
data_source=self.data_source,
|
||||
data_url=release.url,
|
||||
genre=release.genre,
|
||||
genres=release.genres,
|
||||
year=release_date.year if release_date else None,
|
||||
month=release_date.month if release_date else None,
|
||||
day=release_date.day if release_date else None,
|
||||
|
|
@ -509,6 +517,7 @@ class BeatportPlugin(MetadataSourcePlugin):
|
|||
bpm=track.bpm,
|
||||
initial_key=track.initial_key,
|
||||
genre=track.genre,
|
||||
genres=track.genres,
|
||||
)
|
||||
|
||||
def _get_artist(self, artists):
|
||||
|
|
|
|||
|
|
@ -329,26 +329,23 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
|
||||
# Main processing: _get_genre() and helpers.
|
||||
|
||||
def _format_and_stringify(self, tags: list[str]) -> str:
|
||||
"""Format to title_case if configured and return as delimited string."""
|
||||
def _format_genres(self, tags: list[str]) -> list[str]:
|
||||
"""Format to title_case if configured and return as list."""
|
||||
if self.config["title_case"]:
|
||||
formatted = [tag.title() for tag in tags]
|
||||
return [tag.title() for tag in tags]
|
||||
else:
|
||||
formatted = tags
|
||||
|
||||
return self.config["separator"].as_str().join(formatted)
|
||||
return tags
|
||||
|
||||
def _get_existing_genres(self, obj: LibModel) -> 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)
|
||||
genres_list = obj.get("genres", with_album=False)
|
||||
else:
|
||||
item_genre = obj.get("genre").split(separator)
|
||||
genres_list = obj.get("genres")
|
||||
|
||||
# Filter out empty strings
|
||||
return [g for g in item_genre if g]
|
||||
return [g for g in genres_list if g] if genres_list else []
|
||||
|
||||
def _combine_resolve_and_log(
|
||||
self, old: list[str], new: list[str]
|
||||
|
|
@ -359,8 +356,8 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
combined = old + new
|
||||
return self._resolve_genres(combined)
|
||||
|
||||
def _get_genre(self, obj: LibModel) -> tuple[str | None, ...]:
|
||||
"""Get the final genre string for an Album or Item object.
|
||||
def _get_genre(self, obj: LibModel) -> tuple[list[str], str]:
|
||||
"""Get the final genre list 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
|
||||
|
|
@ -370,9 +367,9 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
- artist, albumartist or "most popular track genre" (for VA-albums)
|
||||
- original fallback
|
||||
- configured fallback
|
||||
- None
|
||||
- empty list
|
||||
|
||||
A `(genre, label)` pair is returned, where `label` is a string used for
|
||||
A `(genres, 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
|
||||
|
|
@ -391,7 +388,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
label = f"{stage_label}, {suffix}"
|
||||
if keep_genres:
|
||||
label = f"keep + {label}"
|
||||
return self._format_and_stringify(resolved_genres), label
|
||||
return self._format_genres(resolved_genres), label
|
||||
return None
|
||||
|
||||
keep_genres = []
|
||||
|
|
@ -400,10 +397,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
|
||||
if genres and not self.config["force"]:
|
||||
# Without force pre-populated tags are returned as-is.
|
||||
label = "keep any, no-force"
|
||||
if isinstance(obj, library.Item):
|
||||
return obj.get("genre", with_album=False), label
|
||||
return obj.get("genre"), label
|
||||
return genres, "keep any, no-force"
|
||||
|
||||
if self.config["force"]:
|
||||
# Force doesn't keep any unless keep_existing is set.
|
||||
|
|
@ -480,8 +474,14 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
|
||||
# Nothing found, leave original if configured and valid.
|
||||
if obj.genre and self.config["keep_existing"]:
|
||||
if not self.whitelist or self._is_valid(obj.genre.lower()):
|
||||
return obj.genre, "original fallback"
|
||||
# Check if at least one genre is valid
|
||||
valid_genres = [
|
||||
g
|
||||
for g in genres
|
||||
if not self.whitelist or self._is_valid(g.lower())
|
||||
]
|
||||
if valid_genres:
|
||||
return valid_genres, "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!
|
||||
|
|
@ -490,22 +490,23 @@ class LastGenrePlugin(plugins.BeetsPlugin):
|
|||
):
|
||||
return result
|
||||
|
||||
# Return fallback string.
|
||||
# Return fallback as a list.
|
||||
if fallback := self.config["fallback"].get():
|
||||
return fallback, "fallback"
|
||||
return [fallback], "fallback"
|
||||
|
||||
# No fallback configured.
|
||||
return None, "fallback unconfigured"
|
||||
return [], "fallback unconfigured"
|
||||
|
||||
# Beets plugin hooks and CLI.
|
||||
|
||||
def _fetch_and_log_genre(self, obj: LibModel) -> None:
|
||||
"""Fetch genre and log it."""
|
||||
self._log.info(str(obj))
|
||||
obj.genre, label = self._get_genre(obj)
|
||||
self._log.debug("Resolved ({}): {}", label, obj.genre)
|
||||
genres_list, label = self._get_genre(obj)
|
||||
obj.genres = genres_list
|
||||
self._log.debug("Resolved ({}): {}", label, genres_list)
|
||||
|
||||
ui.show_model_changes(obj, fields=["genre"], print_obj=False)
|
||||
ui.show_model_changes(obj, fields=["genres"], print_obj=False)
|
||||
|
||||
@singledispatchmethod
|
||||
def _process(self, obj: LibModel, write: bool) -> None:
|
||||
|
|
|
|||
|
|
@ -644,10 +644,11 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin):
|
|||
for source in sources:
|
||||
for genreitem in source:
|
||||
genres[genreitem["name"]] += int(genreitem["count"])
|
||||
info.genre = "; ".join(
|
||||
genre_list = [
|
||||
genre
|
||||
for genre, _count in sorted(genres.items(), key=lambda g: -g[1])
|
||||
)
|
||||
]
|
||||
info.genres = genre_list
|
||||
|
||||
# We might find links to external sources (Discogs, Bandcamp, ...)
|
||||
external_ids = self.config["external_ids"].get()
|
||||
|
|
|
|||
|
|
@ -9,9 +9,24 @@ below!
|
|||
Unreleased
|
||||
----------
|
||||
|
||||
..
|
||||
New features
|
||||
~~~~~~~~~~~~
|
||||
New features
|
||||
~~~~~~~~~~~~
|
||||
|
||||
- Add native support for multiple genres per album/track. The ``genres`` field
|
||||
now stores genres as a list and is written to files as multiple individual
|
||||
genre tags (e.g., separate GENRE tags for FLAC/MP3). The single ``genre``
|
||||
field is automatically synchronized to contain the first genre from the list
|
||||
for backward compatibility. The :doc:`plugins/musicbrainz`,
|
||||
:doc:`plugins/beatport`, and :doc:`plugins/lastgenre` plugins have been
|
||||
updated to populate the ``genres`` field as a list.
|
||||
|
||||
**Migration**: Existing libraries with comma-separated, semicolon-separated,
|
||||
or slash-separated genre strings (e.g., ``"Rock, Alternative, Indie"``) will
|
||||
be automatically migrated to the ``genres`` list when items are accessed. No
|
||||
manual reimport or ``mbsync`` is required. For users who prefer explicit
|
||||
control, a new ``beet migrate genres`` command is available to migrate the
|
||||
entire library at once. Use ``beet migrate genres --pretend`` to preview
|
||||
changes before applying them.
|
||||
|
||||
Bug fixes
|
||||
~~~~~~~~~
|
||||
|
|
|
|||
|
|
@ -583,7 +583,8 @@ class BeatportTest(BeetsTestCase):
|
|||
|
||||
def test_genre_applied(self):
|
||||
for track, test_track in zip(self.tracks, self.test_tracks):
|
||||
assert track.genre == test_track.genre
|
||||
# BeatportTrack now has genres as a list
|
||||
assert track.genres == [test_track.genre]
|
||||
|
||||
|
||||
class BeatportResponseEmptyTest(unittest.TestCase):
|
||||
|
|
@ -634,7 +635,8 @@ class BeatportResponseEmptyTest(unittest.TestCase):
|
|||
|
||||
self.test_tracks[0]["subGenres"] = []
|
||||
|
||||
assert tracks[0].genre == self.test_tracks[0]["genres"][0]["name"]
|
||||
# BeatportTrack now has genres as a list
|
||||
assert tracks[0].genres == [self.test_tracks[0]["genres"][0]["name"]]
|
||||
|
||||
def test_genre_empty(self):
|
||||
"""No 'genre' is provided. Test if 'sub_genre' is applied."""
|
||||
|
|
@ -643,4 +645,5 @@ class BeatportResponseEmptyTest(unittest.TestCase):
|
|||
|
||||
self.test_tracks[0]["genres"] = []
|
||||
|
||||
assert tracks[0].genre == self.test_tracks[0]["subGenres"][0]["name"]
|
||||
# BeatportTrack now has genres as a list
|
||||
assert tracks[0].genres == [self.test_tracks[0]["subGenres"][0]["name"]]
|
||||
|
|
|
|||
|
|
@ -80,13 +80,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
self._setup_config(canonical="", whitelist={"rock"})
|
||||
assert self.plugin._resolve_genres(["delta blues"]) == []
|
||||
|
||||
def test_format_and_stringify(self):
|
||||
"""Format genres list and return them as a separator-delimited string."""
|
||||
def test_format_genres(self):
|
||||
"""Format genres list with title case if configured."""
|
||||
self._setup_config(count=2)
|
||||
assert (
|
||||
self.plugin._format_and_stringify(["jazz", "pop", "rock", "blues"])
|
||||
== "Jazz, Pop, Rock, Blues"
|
||||
)
|
||||
assert self.plugin._format_genres(["jazz", "pop", "rock", "blues"]) == [
|
||||
"Jazz",
|
||||
"Pop",
|
||||
"Rock",
|
||||
"Blues",
|
||||
]
|
||||
|
||||
def test_count_c14n(self):
|
||||
"""Keep the n first genres, after having applied c14n when necessary"""
|
||||
|
|
@ -144,7 +146,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
albumartist="Pretend Artist",
|
||||
artist="Pretend Artist",
|
||||
title="Pretend Track",
|
||||
genre="Original Genre",
|
||||
genres=["Original Genre"],
|
||||
)
|
||||
album = self.lib.add_album([item])
|
||||
|
||||
|
|
@ -155,10 +157,10 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
with patch("beetsplug.lastgenre.Item.store", unexpected_store):
|
||||
output = self.run_with_output("lastgenre", "--pretend")
|
||||
|
||||
assert "Mock Genre" in output
|
||||
assert "genres:" in output
|
||||
album.load()
|
||||
assert album.genre == "Original Genre"
|
||||
assert album.items()[0].genre == "Original Genre"
|
||||
assert album.genres == ["Original Genre"]
|
||||
assert album.items()[0].genres == ["Original Genre"]
|
||||
|
||||
def test_no_duplicate(self):
|
||||
"""Remove duplicated genres."""
|
||||
|
|
@ -219,7 +221,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Blues, Jazz", "keep + album, whitelist"),
|
||||
(["Blues", "Jazz"], "keep + album, whitelist"),
|
||||
),
|
||||
# 1 - force and keep whitelisted, unknown original
|
||||
(
|
||||
|
|
@ -235,7 +237,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Blues, Jazz", "keep + album, whitelist"),
|
||||
(["Blues", "Jazz"], "keep + album, whitelist"),
|
||||
),
|
||||
# 2 - force and keep whitelisted on empty tag
|
||||
(
|
||||
|
|
@ -251,7 +253,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Jazz", "album, whitelist"),
|
||||
(["Jazz"], "album, whitelist"),
|
||||
),
|
||||
# 3 force and keep, artist configured
|
||||
(
|
||||
|
|
@ -268,7 +270,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": ["Jazz"],
|
||||
"artist": ["Pop"],
|
||||
},
|
||||
("Blues, Pop", "keep + artist, whitelist"),
|
||||
(["Blues", "Pop"], "keep + artist, whitelist"),
|
||||
),
|
||||
# 4 - don't force, disabled whitelist
|
||||
(
|
||||
|
|
@ -284,7 +286,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("any genre", "keep any, no-force"),
|
||||
(["any genre"], "keep any, no-force"),
|
||||
),
|
||||
# 5 - don't force and empty is regular last.fm fetch; no whitelist too
|
||||
(
|
||||
|
|
@ -300,7 +302,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazzin"],
|
||||
},
|
||||
("Jazzin", "album, any"),
|
||||
(["Jazzin"], "album, any"),
|
||||
),
|
||||
# 6 - fallback to next stages until found
|
||||
(
|
||||
|
|
@ -318,7 +320,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": None,
|
||||
"artist": ["Jazz"],
|
||||
},
|
||||
("Unknown Genre, Jazz", "keep + artist, any"),
|
||||
(["Unknown Genre", "Jazz"], "keep + artist, any"),
|
||||
),
|
||||
# 7 - Keep the original genre when force and keep_existing are on, and
|
||||
# whitelist is disabled
|
||||
|
|
@ -338,7 +340,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": None,
|
||||
"artist": None,
|
||||
},
|
||||
("any existing", "original fallback"),
|
||||
(["any existing"], "original fallback"),
|
||||
),
|
||||
# 7.1 - Keep the original genre when force and keep_existing are on, and
|
||||
# whitelist is enabled, and genre is valid.
|
||||
|
|
@ -358,7 +360,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": None,
|
||||
"artist": None,
|
||||
},
|
||||
("Jazz", "original fallback"),
|
||||
(["Jazz"], "original fallback"),
|
||||
),
|
||||
# 7.2 - Return the configured fallback when force is on but
|
||||
# keep_existing is not.
|
||||
|
|
@ -378,7 +380,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": None,
|
||||
"artist": None,
|
||||
},
|
||||
("fallback genre", "fallback"),
|
||||
(["fallback genre"], "fallback"),
|
||||
),
|
||||
# 8 - fallback to fallback if no original
|
||||
(
|
||||
|
|
@ -397,7 +399,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": None,
|
||||
"artist": None,
|
||||
},
|
||||
("fallback genre", "fallback"),
|
||||
(["fallback genre"], "fallback"),
|
||||
),
|
||||
# 9 - null charachter as separator
|
||||
(
|
||||
|
|
@ -409,12 +411,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"separator": "\u0000",
|
||||
"canonical": False,
|
||||
"prefer_specific": False,
|
||||
"count": 10,
|
||||
},
|
||||
"Blues",
|
||||
{
|
||||
"album": ["Jazz"],
|
||||
},
|
||||
("Blues\u0000Jazz", "keep + album, whitelist"),
|
||||
(["Blues", "Jazz"], "keep + album, whitelist"),
|
||||
),
|
||||
# 10 - limit a lot of results
|
||||
(
|
||||
|
|
@ -432,7 +435,10 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazz", "Bebop", "Hardbop"],
|
||||
},
|
||||
("Blues, Rock, Metal, Jazz, Bebop", "keep + album, whitelist"),
|
||||
(
|
||||
["Blues", "Rock", "Metal", "Jazz", "Bebop"],
|
||||
"keep + album, whitelist",
|
||||
),
|
||||
),
|
||||
# 11 - force off does not rely on configured separator
|
||||
(
|
||||
|
|
@ -448,7 +454,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["Jazz", "Bebop"],
|
||||
},
|
||||
("not ; configured | separator", "keep any, no-force"),
|
||||
(["not ; configured | separator"], "keep any, no-force"),
|
||||
),
|
||||
# 12 - fallback to next stage (artist) if no allowed original present
|
||||
# and no album genre were fetched.
|
||||
|
|
@ -468,7 +474,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": None,
|
||||
"artist": ["Jazz"],
|
||||
},
|
||||
("Jazz", "keep + artist, whitelist"),
|
||||
(["Jazz"], "keep + artist, whitelist"),
|
||||
),
|
||||
# 13 - canonicalization transforms non-whitelisted genres to canonical forms
|
||||
#
|
||||
|
|
@ -488,7 +494,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
{
|
||||
"album": ["acid techno"],
|
||||
},
|
||||
("Techno, Electronic", "album, whitelist"),
|
||||
(["Techno", "Electronic"], "album, whitelist"),
|
||||
),
|
||||
# 14 - canonicalization transforms whitelisted genres to canonical forms and
|
||||
# includes originals
|
||||
|
|
@ -512,7 +518,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": ["acid house"],
|
||||
},
|
||||
(
|
||||
"Detroit Techno, Techno, Electronic, Acid House, House",
|
||||
[
|
||||
"Detroit Techno",
|
||||
"Techno",
|
||||
"Electronic",
|
||||
"Acid House",
|
||||
"House",
|
||||
],
|
||||
"keep + album, whitelist",
|
||||
),
|
||||
),
|
||||
|
|
@ -537,7 +549,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"album": ["Detroit Techno"],
|
||||
},
|
||||
(
|
||||
"Disco, Electronic, Detroit Techno, Techno",
|
||||
["Disco", "Electronic", "Detroit Techno", "Techno"],
|
||||
"keep + album, whitelist",
|
||||
),
|
||||
),
|
||||
|
|
@ -556,13 +568,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase):
|
|||
"prefer_specific": False,
|
||||
"count": 10,
|
||||
},
|
||||
"Cosmic Disco",
|
||||
["Cosmic Disco"],
|
||||
{
|
||||
"album": [],
|
||||
"artist": [],
|
||||
},
|
||||
(
|
||||
"Disco, Electronic",
|
||||
["Disco", "Electronic"],
|
||||
"keep + original fallback, whitelist",
|
||||
),
|
||||
),
|
||||
|
|
@ -592,9 +604,31 @@ def test_get_genre(config_values, item_genre, mock_genres, expected_result):
|
|||
# Configure
|
||||
plugin.config.set(config_values)
|
||||
plugin.setup() # Loads default whitelist and canonicalization tree
|
||||
|
||||
item = _common.item()
|
||||
item.genre = item_genre
|
||||
# Set genres as a list - if item_genre is a string, convert it to list
|
||||
if item_genre:
|
||||
# For compatibility with old separator-based tests, split if needed
|
||||
if (
|
||||
"separator" in config_values
|
||||
and config_values["separator"] in item_genre
|
||||
):
|
||||
sep = config_values["separator"]
|
||||
item.genres = [
|
||||
g.strip() for g in item_genre.split(sep) if g.strip()
|
||||
]
|
||||
else:
|
||||
# Assume comma-separated if no specific separator
|
||||
if ", " in item_genre:
|
||||
item.genres = [
|
||||
g.strip() for g in item_genre.split(", ") if g.strip()
|
||||
]
|
||||
else:
|
||||
item.genres = [item_genre]
|
||||
else:
|
||||
item.genres = []
|
||||
|
||||
# Run
|
||||
res = plugin._get_genre(item)
|
||||
assert res == expected_result
|
||||
expected_genres, expected_label = expected_result
|
||||
assert res == (expected_genres, expected_label)
|
||||
|
|
|
|||
|
|
@ -526,14 +526,14 @@ class MBAlbumInfoTest(MusicBrainzTestCase):
|
|||
config["musicbrainz"]["genres_tag"] = "genre"
|
||||
release = self._make_release()
|
||||
d = self.mb.album_info(release)
|
||||
assert d.genre == "GENRE"
|
||||
assert d.genres == ["GENRE"]
|
||||
|
||||
def test_tags(self):
|
||||
config["musicbrainz"]["genres"] = True
|
||||
config["musicbrainz"]["genres_tag"] = "tag"
|
||||
release = self._make_release()
|
||||
d = self.mb.album_info(release)
|
||||
assert d.genre == "TAG"
|
||||
assert d.genres == ["TAG"]
|
||||
|
||||
def test_no_genres(self):
|
||||
config["musicbrainz"]["genres"] = False
|
||||
|
|
|
|||
|
|
@ -475,3 +475,116 @@ def test_correct_list_fields(
|
|||
|
||||
single_val, list_val = item[single_field], item[list_field]
|
||||
assert (not single_val and not list_val) or single_val == list_val[0]
|
||||
|
||||
|
||||
# Tests for multi-value genres functionality
|
||||
class TestGenreSync:
|
||||
"""Test the genre/genres field synchronization."""
|
||||
|
||||
def test_genres_list_to_genre_first(self):
|
||||
"""Genres list sets genre to first item."""
|
||||
item = Item(genres=["Rock", "Alternative", "Indie"])
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genre == "Rock"
|
||||
assert item.genres == ["Rock", "Alternative", "Indie"]
|
||||
|
||||
def test_genre_string_to_genres_list(self):
|
||||
"""Genre string becomes first item in genres list."""
|
||||
item = Item(genre="Rock")
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genre == "Rock"
|
||||
assert item.genres == ["Rock"]
|
||||
|
||||
def test_genre_and_genres_both_present(self):
|
||||
"""When both genre and genres exist, genre becomes first in list."""
|
||||
item = Item(genre="Jazz", genres=["Rock", "Alternative"])
|
||||
correct_list_fields(item)
|
||||
|
||||
# genre should be prepended to genres list (deduplicated)
|
||||
assert item.genre == "Jazz"
|
||||
assert item.genres == ["Jazz", "Rock", "Alternative"]
|
||||
|
||||
def test_empty_genre(self):
|
||||
"""Empty genre field."""
|
||||
item = Item(genre="")
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genre == ""
|
||||
assert item.genres == []
|
||||
|
||||
def test_empty_genres(self):
|
||||
"""Empty genres list."""
|
||||
item = Item(genres=[])
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genre == ""
|
||||
assert item.genres == []
|
||||
|
||||
def test_none_values(self):
|
||||
"""Handle None values in genre/genres fields without errors."""
|
||||
# Test with None genre
|
||||
item = Item(genre=None, genres=["Rock"])
|
||||
correct_list_fields(item)
|
||||
assert item.genres == ["Rock"]
|
||||
assert item.genre == "Rock"
|
||||
|
||||
# Test with None genres
|
||||
item = Item(genre="Jazz", genres=None)
|
||||
correct_list_fields(item)
|
||||
assert item.genre == "Jazz"
|
||||
assert item.genres == ["Jazz"]
|
||||
|
||||
def test_none_both(self):
|
||||
"""Handle None in both genre and genres."""
|
||||
item = Item(genre=None, genres=None)
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genres == []
|
||||
assert item.genre == ""
|
||||
|
||||
def test_migrate_comma_separated_genres(self):
|
||||
"""Migrate legacy comma-separated genre strings."""
|
||||
item = Item(genre="Rock, Alternative, Indie", genres=[])
|
||||
correct_list_fields(item)
|
||||
|
||||
# Should split into genres list
|
||||
assert item.genres == ["Rock", "Alternative", "Indie"]
|
||||
# Genre becomes first item after migration
|
||||
assert item.genre == "Rock"
|
||||
|
||||
def test_migrate_semicolon_separated_genres(self):
|
||||
"""Migrate legacy semicolon-separated genre strings."""
|
||||
item = Item(genre="Rock; Alternative; Indie", genres=[])
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genres == ["Rock", "Alternative", "Indie"]
|
||||
assert item.genre == "Rock"
|
||||
|
||||
def test_migrate_slash_separated_genres(self):
|
||||
"""Migrate legacy slash-separated genre strings."""
|
||||
item = Item(genre="Rock / Alternative / Indie", genres=[])
|
||||
correct_list_fields(item)
|
||||
|
||||
assert item.genres == ["Rock", "Alternative", "Indie"]
|
||||
assert item.genre == "Rock"
|
||||
|
||||
def test_no_migration_when_genres_exists(self):
|
||||
"""Don't migrate if genres list already populated."""
|
||||
item = Item(genre="Jazz, Blues", genres=["Rock", "Pop"])
|
||||
correct_list_fields(item)
|
||||
|
||||
# Existing genres list should be preserved
|
||||
# The full genre string is prepended (migration doesn't run when genres exists)
|
||||
assert item.genres == ["Jazz, Blues", "Rock", "Pop"]
|
||||
assert item.genre == "Jazz, Blues"
|
||||
|
||||
def test_no_migration_single_genre(self):
|
||||
"""Don't split single genres without separators."""
|
||||
item = Item(genre="Rock", genres=[])
|
||||
correct_list_fields(item)
|
||||
|
||||
# Single genre (no separator) should not trigger migration
|
||||
assert item.genres == ["Rock"]
|
||||
assert item.genre == "Rock"
|
||||
|
|
|
|||
|
|
@ -688,19 +688,47 @@ class DestinationFunctionTest(BeetsTestCase, PathFormattingMixin):
|
|||
self._assert_dest(b"/base/not_played")
|
||||
|
||||
def test_first(self):
|
||||
self.i.genres = "Pop; Rock; Classical Crossover"
|
||||
self._setf("%first{$genres}")
|
||||
self.i.genre = "Pop; Rock; Classical Crossover"
|
||||
self._setf("%first{$genre}")
|
||||
self._assert_dest(b"/base/Pop")
|
||||
|
||||
def test_first_skip(self):
|
||||
self.i.genres = "Pop; Rock; Classical Crossover"
|
||||
self._setf("%first{$genres,1,2}")
|
||||
self.i.genre = "Pop; Rock; Classical Crossover"
|
||||
self._setf("%first{$genre,1,2}")
|
||||
self._assert_dest(b"/base/Classical Crossover")
|
||||
|
||||
def test_first_different_sep(self):
|
||||
self._setf("%first{Alice / Bob / Eve,2,0, / , & }")
|
||||
self._assert_dest(b"/base/Alice & Bob")
|
||||
|
||||
def test_first_genres_list(self):
|
||||
# Test that setting genres as a list syncs to genre field properly
|
||||
# and works with %first template function
|
||||
from beets.autotag import correct_list_fields
|
||||
|
||||
# Clear the default genre first
|
||||
self.i.genre = ""
|
||||
self.i.genres = ["Pop", "Rock", "Classical Crossover"]
|
||||
correct_list_fields(self.i)
|
||||
# genre field should now be synced to first item
|
||||
assert self.i.genre == "Pop"
|
||||
# %first should work on the genre field
|
||||
self._setf("%first{$genre}")
|
||||
self._assert_dest(b"/base/Pop")
|
||||
|
||||
def test_first_genres_list_skip(self):
|
||||
# Test that the genres list is accessible as a multi-value field
|
||||
from beets.autotag import correct_list_fields
|
||||
|
||||
# Clear the default genre first
|
||||
self.i.genre = ""
|
||||
self.i.genres = ["Pop", "Rock", "Classical Crossover"]
|
||||
correct_list_fields(self.i)
|
||||
# Access the second genre directly using index (genres is a list)
|
||||
# The genres field should be available as a multi-value field
|
||||
assert self.i.genres[1] == "Rock"
|
||||
assert len(self.i.genres) == 3
|
||||
|
||||
|
||||
class DisambiguationTest(BeetsTestCase, PathFormattingMixin):
|
||||
def setUp(self):
|
||||
|
|
|
|||
Loading…
Reference in a new issue