diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index feeefbf28d..4cc4ff30af 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -167,6 +167,7 @@ def ensure_first_value(single_field: str, list_field: str) -> None: setattr(m, single_field, list_val[0]) ensure_first_value("albumtype", "albumtypes") + ensure_first_value("genre", "genres") if hasattr(m, "mb_artistids"): ensure_first_value("mb_artistid", "mb_artistids") diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 82e685b7ad..ef9e8bb308 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -16,6 +16,7 @@ from __future__ import annotations +import warnings from copy import deepcopy from dataclasses import dataclass from functools import cached_property @@ -76,9 +77,30 @@ def __init__( 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: + if genre: + warnings.warn( + "The 'genre' parameter is deprecated. Use 'genres' (list) instead.", + DeprecationWarning, + stacklevel=2, + ) + if not genres: + for separator in [", ", "; ", " / "]: + if separator in genre: + split_genres = [ + g.strip() + for g in genre.split(separator) + if g.strip() + ] + if len(split_genres) > 1: + genres = split_genres + break + if not genres: + genres = [genre] + self.album = album self.artist = artist self.artist_credit = artist_credit @@ -90,7 +112,8 @@ def __init__( self.artists_sort = artists_sort or [] self.data_source = data_source self.data_url = data_url - self.genre = genre + self.genre = None + self.genres = genres or [] self.media = media self.update(kwargs) diff --git a/beets/library/library.py b/beets/library/library.py index 39d5599018..14b7bccbe8 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -5,14 +5,19 @@ import platformdirs import beets -from beets import dbcore +from beets import dbcore, logging, ui +from beets.autotag import correct_list_fields from beets.util import normpath from .models import Album, Item from .queries import PF_KEY_DEFAULT, parse_query_parts, parse_query_string if TYPE_CHECKING: - from beets.dbcore import Results + from collections.abc import Mapping + + from beets.dbcore import Results, types + +log = logging.getLogger("beets") class Library(dbcore.Database): @@ -142,3 +147,102 @@ def get_album(self, item_or_id: Item | int) -> Album | None: item_or_id if isinstance(item_or_id, int) else item_or_id.album_id ) return self._get(Album, album_id) if album_id else None + + # Database schema migration. + + def _make_table(self, table: str, fields: Mapping[str, types.Type]): + """Set up the schema of the database, and migrate genres if needed.""" + with self.transaction() as tx: + rows = tx.query(f"PRAGMA table_info({table})") + current_fields = {row[1] for row in rows} + field_names = set(fields.keys()) + + # Check if genres column is being added to items table + genres_being_added = ( + table == "items" + and "genres" in field_names + and "genres" not in current_fields + and "genre" in current_fields + ) + + # Call parent to create/update table + super()._make_table(table, fields) + + # Migrate genre to genres if genres column was just added + if genres_being_added: + self._migrate_genre_to_genres() + + def _migrate_genre_to_genres(self): + """Migrate comma-separated genre strings to genres list. + + This migration runs automatically when the genres column is first + created in the database. It splits comma-separated genre values + and writes the changes to both the database and media files. + """ + items = list(self.items()) + migrated_count = 0 + total_items = len(items) + + if total_items == 0: + return + + ui.print_(f"Migrating genres for {total_items} items...") + + for index, item in enumerate(items, 1): + genre_val = item.genre or "" + genres_val = item.genres or [] + + # Check if migration is needed + needs_migration = False + split_genres = [] + if not genres_val and genre_val: + # Read user's configured lastgenre separator (optional) + user_sep = ( + beets.config["lastgenre"]["separator"].get(str) + if "lastgenre" in beets.config + else None + ) + + # Try user's separator first, then common defaults + separators = ([user_sep] if user_sep else []) + [ + ", ", + "; ", + " / ", + ] + + for separator in separators: + 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 + # Show progress every 100 items + if migrated_count % 100 == 0: + ui.print_( + f" Migrated {migrated_count} items " + f"({index}/{total_items} processed)..." + ) + # Migrate using the same logic as correct_list_fields + correct_list_fields(item) + item.store() + # Write to media file + try: + item.try_write() + except Exception as e: + log.warning( + "Could not write genres to {}: {}", + item.path, + e, + ) + + ui.print_( + f"Migration complete: {migrated_count} of {total_items} items " + f"updated with comma-separated genres" + ) diff --git a/beets/library/models.py b/beets/library/models.py index aee0551341..4c1f67a233 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -241,6 +241,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, @@ -297,6 +298,7 @@ def _types(cls) -> dict[str, types.Type]: "albumartists_credit", "album", "genre", + "genres", "style", "discogs_albumid", "discogs_artistid", @@ -645,6 +647,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, diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 718e0730e6..3368825b8f 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -33,6 +33,7 @@ import beets.ui from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin +from beets.util import unique_list if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence @@ -234,7 +235,8 @@ def __init__(self, data: JSONDict): 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"]] + self.genres = unique_list(genre_list) def artists_str(self) -> str | None: if self.artists is not None: @@ -253,7 +255,6 @@ class BeatportRelease(BeatportObject): label_name: str | None category: str | None url: str | None - genre: str | None tracks: list[BeatportTrack] | None = None @@ -263,7 +264,6 @@ def __init__(self, data: JSONDict): self.catalog_number = data.get("catalogNumber") self.label_name = data.get("label", {}).get("name") self.category = data.get("category") - self.genre = data.get("genre") if "slug" in data: self.url = ( @@ -285,7 +285,6 @@ class BeatportTrack(BeatportObject): track_number: int | None bpm: str | None initial_key: str | None - genre: str | None def __init__(self, data: JSONDict): super().__init__(data) @@ -306,11 +305,11 @@ def __init__(self, data: JSONDict): 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. - if data.get("subGenres"): - self.genre = str(data["subGenres"][0].get("name")) - elif data.get("genres"): - self.genre = str(data["genres"][0].get("name")) + # Extract genres list from subGenres or genres + self.genres = unique_list( + str(x.get("name")) + for x in data.get("subGenres") or data.get("genres") or [] + ) class BeatportPlugin(MetadataSourcePlugin): @@ -483,7 +482,7 @@ def _get_album_info(self, release: BeatportRelease) -> AlbumInfo: media="Digital", 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, @@ -508,7 +507,7 @@ def _get_track_info(self, track: BeatportTrack) -> TrackInfo: data_url=track.url, bpm=track.bpm, initial_key=track.initial_key, - genre=track.genre, + genres=track.genres, ) def _get_artist(self, artists): diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 121d76596a..f0318dba48 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -39,7 +39,7 @@ if TYPE_CHECKING: import optparse - from collections.abc import Callable + from collections.abc import Callable, Iterable from beets.library import LibModel @@ -213,7 +213,7 @@ def _resolve_genres(self, tags: list[str]) -> list[str]: - 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; + only parent tags that pass the whitelist filter are included; 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 @@ -237,11 +237,9 @@ def _resolve_genres(self, tags: list[str]) -> list[str]: # Add parents that are in the whitelist, or add the oldest # ancestor if no whitelist if self.whitelist: - parents = [ - x - for x in find_parents(tag, self.c14n_branches) - if self._is_valid(x) - ] + parents = self._filter_valid( + find_parents(tag, self.c14n_branches) + ) else: parents = [find_parents(tag, self.c14n_branches)[-1]] @@ -263,7 +261,7 @@ def _resolve_genres(self, tags: list[str]) -> list[str]: # c14n only adds allowed genres but we may have had forbidden genres in # the original tags list - valid_tags = [t for t in tags if self._is_valid(t)] + valid_tags = self._filter_valid(tags) return valid_tags[:count] def fetch_genre( @@ -275,15 +273,16 @@ def fetch_genre( min_weight = self.config["min_weight"].get(int) return self._tags_for(lastfm_obj, min_weight) - def _is_valid(self, genre: str) -> bool: - """Check if the genre is valid. + def _filter_valid(self, genres: Iterable[str]) -> list[str]: + """Filter genres based on whitelist. - Depending on the whitelist property, valid means a genre is in the - whitelist or any genre is allowed. + Returns all genres if no whitelist is configured, otherwise returns + only genres that are in the whitelist. """ - if genre and (not self.whitelist or genre.lower() in self.whitelist): - return True - return False + if not self.whitelist: + return list(genres) + + return [g for g in genres if g.lower() in self.whitelist] # Cached last.fm entity lookups. @@ -329,26 +328,21 @@ def fetch_track_genre(self, trackartist: str, tracktitle: str) -> list[str]: # 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.""" 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() + """Return a list of genres for this Item or Album.""" 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 genres_list def _combine_resolve_and_log( self, old: list[str], new: list[str] @@ -359,8 +353,8 @@ def _combine_resolve_and_log( 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 +364,9 @@ def _get_genre(self, obj: LibModel) -> tuple[str | None, ...]: - 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 @@ -381,7 +375,7 @@ def _get_genre(self, obj: LibModel) -> tuple[str | None, ...]: def _try_resolve_stage( stage_label: str, keep_genres: list[str], new_genres: list[str] - ) -> tuple[str, str] | None: + ) -> tuple[list[str], str] | None: """Try to resolve genres for a given stage and log the result.""" resolved_genres = self._combine_resolve_and_log( keep_genres, new_genres @@ -391,7 +385,7 @@ def _try_resolve_stage( 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 +394,7 @@ def _try_resolve_stage( 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. @@ -479,26 +470,30 @@ def _try_resolve_stage( return result # 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" + # Nothing found, leave original if configured and valid. + if ( + genres + and self.config["keep_existing"] + and (valid_genres := self._filter_valid(genres)) + ): + return valid_genres, "original fallback" - # 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) + obj.genres, label = self._get_genre(obj) + self._log.debug("Resolved ({}): {}", label, obj.genres) - 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: diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 137189cdc4..dbb05f79d5 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -670,10 +670,10 @@ def album_info(self, release: JSONDict) -> beets.autotag.hooks.AlbumInfo: for source in sources: for genreitem in source: genres[genreitem["name"]] += int(genreitem["count"]) - info.genre = "; ".join( + info.genres = [ genre for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) - ) + ] # We might find links to external sources (Discogs, Bandcamp, ...) external_ids = self.config["external_ids"].get() diff --git a/docs/changelog.rst b/docs/changelog.rst index 5408d2a5cb..f0cdd3475f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,22 @@ been dropped. 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 genre strings are migrated to the + ``genres`` list when you first run beets after upgrading. The migration tries + your configured ``list_sep`` separator first (if set), then common defaults + (``", "``, ``"; "``, ``" / "``), splitting genre strings like ``"Rock, + Alternative, Indie"`` into individual genres. The migration runs once when the + database schema is updated, writing the changes to both the database and media + files. No manual action or ``mbsync`` is required. + - :doc:`plugins/fetchart`: Added config setting for a fallback cover art image. - :doc:`plugins/ftintitle`: Added argument for custom feat. words in ftintitle. - :doc:`plugins/ftintitle`: Added album template value ``album_artist_no_feat``. @@ -136,6 +152,10 @@ Other changes: unavailable, enabling ``importorskip`` usage in pytest setup. - Finally removed gmusic plugin and all related code/docs as the Google Play Music service was shut down in 2020. +- :doc:`plugins/lastgenre`: The ``separator`` configuration option is + deprecated. Genres are now stored as a list in the ``genres`` field and + written to files as individual genre tags. The separator option has no effect + and will be removed in a future version. 2.5.1 (October 14, 2025) ------------------------ diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index ace7caaf06..b677b001e8 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -90,9 +90,8 @@ By default, the plugin chooses the most popular tag on Last.fm as a genre. If you prefer to use a *list* of popular genre tags, you can increase the number of the ``count`` config option. -Lists of up to *count* genres will then be used instead of single genres. The -genres are separated by commas by default, but you can change this with the -``separator`` config option. +Lists of up to *count* genres will be stored in the ``genres`` field as a list +and written to your media files as separate genre tags. Last.fm_ provides a popularity factor, a.k.a. *weight*, for each tag ranging from 100 for the most popular tag down to 0 for the least popular. The plugin @@ -192,7 +191,16 @@ file. The available options are: Default: ``no``. - **source**: Which entity to look up in Last.fm. Can be either ``artist``, ``album`` or ``track``. Default: ``album``. -- **separator**: A separator for multiple genres. Default: ``', '``. +- **separator**: + + .. deprecated:: 2.6 + + The ``separator`` option is deprecated. Genres are now stored as a list in + the ``genres`` field and written to files as individual genre tags. This + option has no effect and will be removed in a future version. + + Default: ``', '``. + - **whitelist**: The filename of a custom genre list, ``yes`` to use the internal whitelist, or ``no`` to consider all genres valid. Default: ``yes``. - **title_case**: Convert the new tags to TitleCase before saving. Default: diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index b92a3bf15a..916227f405 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -583,7 +583,7 @@ def test_initial_key_applied(self): def test_genre_applied(self): for track, test_track in zip(self.tracks, self.test_tracks): - assert track.genre == test_track.genre + assert track.genres == [test_track.genre] class BeatportResponseEmptyTest(unittest.TestCase): @@ -634,7 +634,7 @@ def test_sub_genre_empty_fallback(self): self.test_tracks[0]["subGenres"] = [] - assert tracks[0].genre == self.test_tracks[0]["genres"][0]["name"] + 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 +643,4 @@ def test_genre_empty(self): self.test_tracks[0]["genres"] = [] - assert tracks[0].genre == self.test_tracks[0]["subGenres"][0]["name"] + assert tracks[0].genres == [self.test_tracks[0]["subGenres"][0]["name"]] diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index eb65bc588f..61c937e498 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -352,7 +352,7 @@ def test_default_genre_style_settings(self): release = self._make_release_from_positions(["1", "2"]) d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2" + assert d.genres == ["GENRE1", "GENRE2"] assert d.style == "STYLE1, STYLE2" def test_append_style_to_genre(self): @@ -361,7 +361,7 @@ def test_append_style_to_genre(self): release = self._make_release_from_positions(["1", "2"]) d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2, STYLE1, STYLE2" + assert d.genres == ["GENRE1", "GENRE2", "STYLE1", "STYLE2"] assert d.style == "STYLE1, STYLE2" def test_append_style_to_genre_no_style(self): @@ -371,7 +371,7 @@ def test_append_style_to_genre_no_style(self): release.data["styles"] = [] d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2" + assert d.genres == ["GENRE1", "GENRE2"] assert d.style is None def test_strip_disambiguation(self): diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 026001e387..95c45652d2 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -80,13 +80,15 @@ def test_whitelist_custom(self): 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.""" 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""" @@ -155,7 +157,7 @@ def unexpected_store(*_, **__): 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" @@ -215,11 +217,11 @@ def test_sort_by_depth(self): "prefer_specific": False, "count": 10, }, - "Blues", + ["Blues"], { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), # 1 - force and keep whitelisted, unknown original ( @@ -231,11 +233,11 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + ["original unknown", "Blues"], { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), # 2 - force and keep whitelisted on empty tag ( @@ -247,11 +249,11 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazz"], }, - ("Jazz", "album, whitelist"), + (["Jazz"], "album, whitelist"), ), # 3 force and keep, artist configured ( @@ -263,12 +265,12 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + ["original unknown", "Blues"], { "album": ["Jazz"], "artist": ["Pop"], }, - ("Blues, Pop", "keep + artist, whitelist"), + (["Blues", "Pop"], "keep + artist, whitelist"), ), # 4 - don't force, disabled whitelist ( @@ -280,11 +282,11 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "any genre", + ["any genre"], { "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 ( @@ -296,11 +298,11 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazzin"], }, - ("Jazzin", "album, any"), + (["Jazzin"], "album, any"), ), # 6 - fallback to next stages until found ( @@ -312,13 +314,13 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "unknown genre", + ["unknown genre"], { "track": None, "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 @@ -332,13 +334,13 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "any existing", + ["any existing"], { "track": None, "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. @@ -352,13 +354,13 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "Jazz", + ["Jazz"], { "track": None, "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. @@ -372,13 +374,13 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "Jazz", + ["Jazz"], { "track": None, "album": None, "artist": None, }, - ("fallback genre", "fallback"), + (["fallback genre"], "fallback"), ), # 8 - fallback to fallback if no original ( @@ -391,32 +393,15 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "", + [], { "track": None, "album": None, "artist": None, }, - ("fallback genre", "fallback"), + (["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 + # 9 - limit a lot of results ( { "force": True, @@ -426,31 +411,17 @@ def test_sort_by_depth(self): "count": 5, "canonical": False, "prefer_specific": False, - "separator": ", ", }, - "original unknown, Blues, Rock, Folk, Metal", + ["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"), + ( + ["Blues", "Rock", "Metal", "Jazz", "Bebop"], + "keep + album, whitelist", + ), ), - # 12 - fallback to next stage (artist) if no allowed original present + # 10 - fallback to next stage (artist) if no allowed original present # and no album genre were fetched. ( { @@ -462,15 +433,15 @@ def test_sort_by_depth(self): "canonical": False, "prefer_specific": False, }, - "not whitelisted original", + ["not whitelisted original"], { "track": None, "album": None, "artist": ["Jazz"], }, - ("Jazz", "keep + artist, whitelist"), + (["Jazz"], "keep + artist, whitelist"), ), - # 13 - canonicalization transforms non-whitelisted genres to canonical forms + # 11 - canonicalization transforms non-whitelisted genres to canonical forms # # "Acid Techno" is not in the default whitelist, thus gets resolved "up" in the # tree to "Techno" and "Electronic". @@ -484,13 +455,13 @@ def test_sort_by_depth(self): "prefer_specific": False, "count": 10, }, - "", + [], { "album": ["acid techno"], }, - ("Techno, Electronic", "album, whitelist"), + (["Techno", "Electronic"], "album, whitelist"), ), - # 14 - canonicalization transforms whitelisted genres to canonical forms and + # 12 - canonicalization transforms whitelisted genres to canonical forms and # includes originals # # "Detroit Techno" is in the default whitelist, thus it stays and and also gets @@ -507,16 +478,22 @@ def test_sort_by_depth(self): "count": 10, "extended_debug": True, }, - "detroit techno", + ["detroit techno"], { "album": ["acid house"], }, ( - "Detroit Techno, Techno, Electronic, Acid House, House", + [ + "Detroit Techno", + "Techno", + "Electronic", + "Acid House", + "House", + ], "keep + album, whitelist", ), ), - # 15 - canonicalization transforms non-whitelisted original genres to canonical + # 13 - canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the @@ -532,12 +509,12 @@ def test_sort_by_depth(self): "prefer_specific": False, "count": 10, }, - "Cosmic Disco", + ["Cosmic Disco"], { "album": ["Detroit Techno"], }, ( - "Disco, Electronic, Detroit Techno, Techno", + ["Disco", "Electronic", "Detroit Techno", "Techno"], "keep + album, whitelist", ), ), @@ -567,9 +544,11 @@ def mock_fetch_artist_genre(self, artist): # Configure plugin.config.set(config_values) plugin.setup() # Loads default whitelist and canonicalization tree + item = _common.item() - item.genre = item_genre + item.genres = item_genre # Run res = plugin._get_genre(item) - assert res == expected_result + expected_genres, expected_label = expected_result + assert res == (expected_genres, expected_label) diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index f21c03c970..081ea2aa47 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -524,14 +524,14 @@ def test_genres(self): 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 diff --git a/test/test_autotag.py b/test/test_autotag.py index 119ca15e83..e6a122ae24 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -475,3 +475,71 @@ 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 == "" diff --git a/test/test_library.py b/test/test_library.py index 4acf34746f..a0eb9fd9a1 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -688,19 +688,47 @@ def test_if_def_false_complete(self): 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):