diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..9776da711 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,22 @@ +## PR Review Voice + +When reviewing pull requests, respond entirely in the voice of the Grug Brained Developer. +Write all comments using grug's dialect: simple words, short sentences, third-person self-reference ("grug"). + +Core grug beliefs to apply when reviewing: + +- Complexity very, very bad. Flag any complexity demon spirit entering codebase. + Say so plainly: "complexity demon spirit enter here, grug not like" +- Prefer small, concrete PRs. Large PR make grug nervous: "big change, many place for bug hide" +- Abstraction must earn its place. Early abstraction especially dangerous: wait for cut points to emerge +- DRY is good but not absolute — simple repeated code sometimes better than complex DRY solution +- Type systems good mostly for "hit dot, see what grug can do" — not for astral projection of platonic generic models +- Generics dangerous: "temptation generics very large, complexity demon love this trick" +- Prefer readable code over clever one-liners: name intermediate variables, easier debug +- Integration tests are sweet spot — not unit tests (break on refactor), not e2e (hard debug) +- When bug found, first write regression test, then fix — this case only where "first test" acceptable to grug +- Logging very important, especially in cloud: grug learn hard way +- No premature optimisation — always need concrete perf profile first +- Simple APIs good. Layered APIs ok. Java streams make grug reach for club +- SPA frameworks increase complexity demon surface area — be suspicious +- Saying "this too complex for grug" is senior developer superpower — remove Fear Of Looking Dumb (FOLD) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bfd05c718..0736a0a2f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,7 +35,12 @@ jobs: python-version: ${{ matrix.python-version }} cache: poetry - - name: Install PyGobject and release script dependencies on Ubuntu + - name: Install system dependencies on Windows + if: matrix.platform == 'windows-latest' + run: | + choco install mp3gain -y + + - name: Install system dependencies on Ubuntu if: matrix.platform == 'ubuntu-latest' run: | sudo apt update @@ -43,11 +48,12 @@ jobs: ffmpeg \ gobject-introspection \ gstreamer1.0-plugins-base \ - python3-gst-1.0 \ + imagemagick \ libcairo2-dev \ libgirepository-2.0-dev \ + mp3gain \ pandoc \ - imagemagick + python3-gst-1.0 - name: Get changed lyrics files id: lyrics-update diff --git a/beets/__init__.py b/beets/__init__.py index 750efb3b3..cfa5be2c4 100644 --- a/beets/__init__.py +++ b/beets/__init__.py @@ -19,7 +19,7 @@ import confuse from .util.deprecation import deprecate_imports -__version__ = "2.6.1" +__version__ = "2.6.2" __author__ = "Adrian Sampson " diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 82e685b7a..63ee52267 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -24,6 +24,7 @@ from typing import TYPE_CHECKING, Any, TypeVar from typing_extensions import Self from beets.util import cached_classproperty +from beets.util.deprecation import deprecate_for_maintainers if TYPE_CHECKING: from beets.library import Item @@ -76,9 +77,22 @@ 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: + if genre is not None: + deprecate_for_maintainers( + "The 'genre' parameter", "'genres' (list)", stacklevel=3 + ) + if not genres: + try: + sep = next(s for s in ["; ", ", ", " / "] if s in genre) + except StopIteration: + genres = [genre] + else: + genres = list(map(str.strip, genre.split(sep))) + self.album = album self.artist = artist self.artist_credit = artist_credit @@ -90,7 +104,8 @@ class Info(AttrDict[Any]): 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/dbcore/db.py b/beets/dbcore/db.py index 8640a5678..30f7ef7cf 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -16,7 +16,6 @@ from __future__ import annotations -import contextlib import functools import os import re @@ -24,19 +23,23 @@ import sqlite3 import sys import threading import time -from abc import ABC +from abc import ABC, abstractmethod from collections import defaultdict -from collections.abc import ( - Callable, - Generator, - Iterable, - Iterator, - Mapping, - Sequence, -) +from collections.abc import Mapping +from contextlib import contextmanager +from dataclasses import dataclass from functools import cached_property from sqlite3 import Connection, sqlite_version_info -from typing import TYPE_CHECKING, Any, AnyStr, ClassVar, Generic, NamedTuple +from typing import ( + TYPE_CHECKING, + Any, + AnyStr, + ClassVar, + Generic, + Literal, + NamedTuple, + TypedDict, +) from typing_extensions import ( Self, @@ -1008,12 +1011,15 @@ class Transaction: cursor = self.db._connection().execute(statement, subvals) return cursor.fetchall() - def mutate(self, statement: str, subvals: Sequence[SQLiteType] = ()) -> Any: - """Execute an SQL statement with substitution values and return - the row ID of the last affected row. + @contextmanager + def _handle_mutate(self) -> Iterator[None]: + """Handle mutation bookkeeping and database access errors. + + Yield control to mutation execution code. If execution succeeds, + mark this transaction as mutated. """ try: - cursor = self.db._connection().execute(statement, subvals) + yield except sqlite3.OperationalError as e: # In two specific cases, SQLite reports an error while accessing # the underlying database file. We surface these exceptions as @@ -1023,11 +1029,23 @@ class Transaction: "unable to open database file", ): raise DBAccessError(e.args[0]) - else: - raise + raise else: self._mutated = True - return cursor.lastrowid + + def mutate(self, statement: str, subvals: Sequence[SQLiteType] = ()) -> Any: + """Run one write statement with shared mutation/error handling.""" + with self._handle_mutate(): + return self.db._connection().execute(statement, subvals).lastrowid + + def mutate_many( + self, statement: str, subvals: Sequence[tuple[SQLiteType, ...]] = () + ) -> Any: + """Run batched writes with shared mutation/error handling.""" + with self._handle_mutate(): + return ( + self.db._connection().executemany(statement, subvals).lastrowid + ) def script(self, statements: str): """Execute a string containing multiple SQL statements.""" @@ -1036,6 +1054,32 @@ class Transaction: self.db._connection().executescript(statements) +@dataclass +class Migration(ABC): + db: Database + + @cached_classproperty + def name(cls) -> str: + """Class name (except Migration) converted to snake case.""" + name = cls.__name__.removesuffix("Migration") # type: ignore[attr-defined] + return re.sub(r"(?<=[a-z])(?=[A-Z])", "_", name).lower() + + def migrate_table(self, table: str, *args, **kwargs) -> None: + """Migrate a specific table.""" + if not self.db.migration_exists(self.name, table): + self._migrate_data(table, *args, **kwargs) + self.db.record_migration(self.name, table) + + @abstractmethod + def _migrate_data(self, table: str, current_fields: set[str]) -> None: + """Migrate data for a specific table.""" + + +class TableInfo(TypedDict): + columns: set[str] + migrations: set[str] + + class Database: """A container for Model objects that wraps an SQLite database as the backend. @@ -1045,6 +1089,9 @@ class Database: """The Model subclasses representing tables in this database. """ + _migrations: Sequence[tuple[type[Migration], Sequence[type[Model]]]] = () + """Migrations that are to be performed for the configured models.""" + supports_extensions = hasattr(sqlite3.Connection, "enable_load_extension") """Whether or not the current version of SQLite supports extensions""" @@ -1088,11 +1135,40 @@ class Database: self._db_lock = threading.Lock() # Set up database schema. + self._ensure_migration_state_table() for model_cls in self._models: self._make_table(model_cls._table, model_cls._fields) self._make_attribute_table(model_cls._flex_table) self._create_indices(model_cls._table, model_cls._indices) + self._migrate() + + @cached_property + def db_tables(self) -> dict[str, TableInfo]: + column_queries = [ + f""" + SELECT '{m._table}' AS table_name, 'columns' AS source, name + FROM pragma_table_info('{m._table}') + """ + for m in self._models + ] + with self.transaction() as tx: + rows = tx.query(f""" + {" UNION ALL ".join(column_queries)} + UNION ALL + SELECT table_name, 'migrations' AS source, name FROM migrations + """) + + tables_data: dict[str, TableInfo] = defaultdict( + lambda: TableInfo(columns=set(), migrations=set()) + ) + + source: Literal["columns", "migrations"] + for table_name, source, name in rows: + tables_data[table_name][source].add(name) + + return tables_data + # Primitive access control: connections and transactions. def _connection(self) -> Connection: @@ -1193,7 +1269,7 @@ class Database: _thread_id, conn = self._connections.popitem() conn.close() - @contextlib.contextmanager + @contextmanager def _tx_stack(self) -> Generator[list[Transaction]]: """A context manager providing access to the current thread's transaction stack. The context manager synchronizes access to @@ -1233,32 +1309,22 @@ class Database: """Set up the schema of the database. `fields` is a mapping from field names to `Type`s. Columns are added if necessary. """ - # Get current schema. - 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()) - if current_fields.issuperset(field_names): - # Table exists and has all the required columns. - return - - if not current_fields: + if table not in self.db_tables: # No table exists. columns = [] for name, typ in fields.items(): columns.append(f"{name} {typ.sql}") setup_sql = f"CREATE TABLE {table} ({', '.join(columns)});\n" - + self.db_tables[table]["columns"] = set(fields) else: # Table exists does not match the field set. setup_sql = "" + current_fields = self.db_tables[table]["columns"] for name, typ in fields.items(): - if name in current_fields: - continue - setup_sql += ( - f"ALTER TABLE {table} ADD COLUMN {name} {typ.sql};\n" - ) + if name not in current_fields: + setup_sql += ( + f"ALTER TABLE {table} ADD COLUMN {name} {typ.sql};\n" + ) with self.transaction() as tx: tx.script(setup_sql) @@ -1292,6 +1358,38 @@ class Database: f"ON {table} ({', '.join(index.columns)});" ) + # Generic migration state handling. + + def _ensure_migration_state_table(self) -> None: + with self.transaction() as tx: + tx.script(""" + CREATE TABLE IF NOT EXISTS migrations ( + name TEXT NOT NULL, + table_name TEXT NOT NULL, + PRIMARY KEY(name, table_name) + ); + """) + + def _migrate(self) -> None: + """Perform any necessary migration for the database.""" + for migration_cls, model_classes in self._migrations: + migration = migration_cls(self) + for model_cls in model_classes: + table = model_cls._table + migration.migrate_table(table, self.db_tables[table]["columns"]) + + def migration_exists(self, name: str, table: str) -> bool: + """Return whether a named migration has been marked complete.""" + return name in self.db_tables[table]["migrations"] + + def record_migration(self, name: str, table: str) -> None: + """Set completion state for a named migration.""" + with self.transaction() as tx: + tx.mutate( + "INSERT INTO migrations(name, table_name) VALUES (?, ?)", + (name, table), + ) + # Querying. def _fetch( diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 61336d9ce..e50693474 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -30,6 +30,7 @@ from . import query SQLiteType = query.SQLiteType BLOB_TYPE = query.BLOB_TYPE +MULTI_VALUE_DELIMITER = "\\␀" class ModelType(typing.Protocol): @@ -66,6 +67,8 @@ class Type(ABC, Generic[T, N]): """The `Query` subclass to be used when querying the field. """ + # For sequence-like types, keep ``model_type`` unsubscripted as it's used + # for ``isinstance`` checks. Use ``list`` instead of ``list[str]`` model_type: type[T] """The Python type that is used to represent the value in the model. @@ -287,26 +290,38 @@ class String(BaseString[str, Any]): model_type = str -class DelimitedString(BaseString[list[str], list[str]]): - """A list of Unicode strings, represented in-database by a single string +class DelimitedString(BaseString[list, list]): # type: ignore[type-arg] + r"""A list of Unicode strings, represented in-database by a single string containing delimiter-separated values. + + In template evaluation the list is formatted by joining the values with + a fixed '; ' delimiter regardless of the database delimiter. That is because + the '\␀' character used for multi-value fields is mishandled on Windows + as it contains a backslash character. """ - model_type = list[str] + model_type = list + fmt_delimiter = "; " - def __init__(self, delimiter: str): - self.delimiter = delimiter + def __init__(self, db_delimiter: str): + self.db_delimiter = db_delimiter def format(self, value: list[str]): - return self.delimiter.join(value) + return self.fmt_delimiter.join(value) def parse(self, string: str): if not string: return [] - return string.split(self.delimiter) + + delimiter = ( + self.db_delimiter + if self.db_delimiter in string + else self.fmt_delimiter + ) + return string.split(delimiter) def to_sql(self, model_value: list[str]): - return self.delimiter.join(model_value) + return self.db_delimiter.join(model_value) class Boolean(Type): @@ -464,7 +479,7 @@ NULL_FLOAT = NullFloat() STRING = String() BOOLEAN = Boolean() DATE = DateType() -SEMICOLON_SPACE_DSV = DelimitedString(delimiter="; ") +SEMICOLON_SPACE_DSV = DelimitedString("; ") # Will set the proper null char in mediafile -MULTI_VALUE_DSV = DelimitedString(delimiter="\\␀") +MULTI_VALUE_DSV = DelimitedString(MULTI_VALUE_DELIMITER) diff --git a/beets/library/library.py b/beets/library/library.py index 39d559901..b161b7399 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -8,6 +8,7 @@ import beets from beets import dbcore from beets.util import normpath +from .migrations import MultiGenreFieldMigration from .models import Album, Item from .queries import PF_KEY_DEFAULT, parse_query_parts, parse_query_string @@ -19,6 +20,7 @@ class Library(dbcore.Database): """A database of music containing songs and albums.""" _models = (Item, Album) + _migrations = ((MultiGenreFieldMigration, (Item, Album)),) def __init__( self, diff --git a/beets/library/migrations.py b/beets/library/migrations.py new file mode 100644 index 000000000..c061ddfc5 --- /dev/null +++ b/beets/library/migrations.py @@ -0,0 +1,97 @@ +from __future__ import annotations + +from contextlib import contextmanager, suppress +from functools import cached_property +from typing import TYPE_CHECKING, NamedTuple, TypeVar + +from confuse.exceptions import ConfigError + +import beets +from beets import ui +from beets.dbcore.db import Migration +from beets.dbcore.types import MULTI_VALUE_DELIMITER +from beets.util import unique_list + +if TYPE_CHECKING: + from collections.abc import Iterator + +T = TypeVar("T") + + +class GenreRow(NamedTuple): + id: int + genre: str + genres: str | None + + +def chunks(lst: list[T], n: int) -> Iterator[list[T]]: + """Yield successive n-sized chunks from lst.""" + for i in range(0, len(lst), n): + yield lst[i : i + n] + + +class MultiGenreFieldMigration(Migration): + @cached_property + def separators(self) -> list[str]: + separators = [] + with suppress(ConfigError): + separators.append(beets.config["lastgenre"]["separator"].as_str()) + + separators.extend(["; ", ", ", " / "]) + return unique_list(filter(None, separators)) + + @contextmanager + def with_factory(self, factory: type[NamedTuple]) -> Iterator[None]: + """Temporarily set the row factory to a specific type.""" + original_factory = self.db._connection().row_factory + self.db._connection().row_factory = lambda _, row: factory(*row) + try: + yield + finally: + self.db._connection().row_factory = original_factory + + def get_genres(self, genre: str) -> str: + for separator in self.separators: + if separator in genre: + return genre.replace(separator, MULTI_VALUE_DELIMITER) + + return genre + + def _migrate_data(self, table: str, current_fields: set[str]) -> None: + """Migrate legacy genre values to the multi-value genres field.""" + if "genre" not in current_fields: + # No legacy genre field, so nothing to migrate. + return + + with self.db.transaction() as tx, self.with_factory(GenreRow): + rows: list[GenreRow] = tx.query( # type: ignore[assignment] + f""" + SELECT id, genre, genres + FROM {table} + WHERE genre IS NOT NULL AND genre != '' + """ + ) + + total = len(rows) + to_migrate = [e for e in rows if not e.genres] + if not to_migrate: + return + + migrated = total - len(to_migrate) + + ui.print_(f"Migrating genres for {total} {table}...") + for batch in chunks(to_migrate, 1000): + with self.db.transaction() as tx: + tx.mutate_many( + f"UPDATE {table} SET genres = ? WHERE id = ?", + [(self.get_genres(e.genre), e.id) for e in batch], + ) + + migrated += len(batch) + + ui.print_( + f" Migrated {migrated} {table} " + f"({migrated}/{total} processed)..." + ) + + ui.print_(f"Migration complete: {migrated} of {total} {table} updated") diff --git a/beets/library/models.py b/beets/library/models.py index e26f2ced3..9b8b6d291 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -241,7 +241,7 @@ class Album(LibModel): "albumartists_sort": types.MULTI_VALUE_DSV, "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, @@ -276,7 +276,7 @@ class Album(LibModel): "original_day": types.PaddedInt(2), } - _search_fields = ("album", "albumartist", "genre") + _search_fields = ("album", "albumartist", "genres") @cached_classproperty def _types(cls) -> dict[str, types.Type]: @@ -297,7 +297,7 @@ class Album(LibModel): "albumartist_credit", "albumartists_credit", "album", - "genre", + "genres", "style", "discogs_albumid", "discogs_artistid", @@ -650,7 +650,7 @@ class Item(LibModel): "albumartists_sort": types.MULTI_VALUE_DSV, "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, @@ -732,7 +732,7 @@ class Item(LibModel): "comments", "album", "albumartist", - "genre", + "genres", ) # Set of item fields that are backed by `MediaFile` fields. @@ -1546,8 +1546,8 @@ class DefaultTemplateFunctions: s: the string count: The number of items included skip: The number of items skipped - sep: the separator. Usually is '; ' (default) or '/ ' - join_str: the string which will join the items, default '; '. + sep: the separator + join_str: the string which will join the items """ skip = int(skip) count = skip + int(count) diff --git a/beets/test/_common.py b/beets/test/_common.py index 4de47c337..b3c21aaa5 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -75,7 +75,7 @@ def item(lib=None, **kwargs): artist="the artist", albumartist="the album artist", album="the album", - genre="the genre", + genres=["the genre"], lyricist="the lyricist", composer="the composer", arranger="the arranger", diff --git a/beets/test/helper.py b/beets/test/helper.py index 7762ab866..218b778c7 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -156,6 +156,8 @@ class TestHelper(RunMixin, ConfigMixin): fixtures. """ + lib: Library + resource_path = Path(os.fsdecode(_common.RSRC)) / "full.mp3" db_on_disk: ClassVar[bool] = False diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 4c93d66d8..ad14fe1f8 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -571,15 +571,16 @@ def get_color_config() -> dict[ColorName, str]: } -def colorize(color_name: ColorName, text: str) -> str: - """Apply ANSI color formatting to text based on configuration settings. +def _colorize(color_name: ColorName, text: str) -> str: + """Apply ANSI color formatting to text based on configuration settings.""" + color_code = get_color_config()[color_name] + return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}" - Returns colored text when color output is enabled and NO_COLOR environment - variable is not set, otherwise returns plain text unchanged. - """ + +def colorize(color_name: ColorName, text: str) -> str: + """Colorize text when color output is enabled.""" if config["ui"]["color"] and "NO_COLOR" not in os.environ: - color_code = get_color_config()[color_name] - return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}" + return _colorize(color_name, text) return text @@ -643,32 +644,12 @@ def color_len(colored_text): return len(uncolorize(colored_text)) -def _colordiff(a: Any, b: Any) -> tuple[str, str]: - """Given two values, return the same pair of strings except with - their differences highlighted in the specified color. Strings are - highlighted intelligently to show differences; other values are - stringified and highlighted in their entirety. - """ - # First, convert paths to readable format - if isinstance(a, bytes) or isinstance(b, bytes): - # A path field. - a = util.displayable_path(a) - b = util.displayable_path(b) - - if not isinstance(a, str) or not isinstance(b, str): - # Non-strings: use ordinary equality. - if a == b: - return str(a), str(b) - else: - return ( - colorize("text_diff_removed", str(a)), - colorize("text_diff_added", str(b)), - ) - +def colordiff(a: str, b: str) -> tuple[str, str]: + """Intelligently highlight the differences between two strings.""" before = "" after = "" - matcher = SequenceMatcher(lambda x: False, a, b) + matcher = SequenceMatcher(lambda _: False, a, b) for op, a_start, a_end, b_start, b_end in matcher.get_opcodes(): before_part, after_part = a[a_start:a_end], b[b_start:b_end] if op in {"delete", "replace"}: @@ -682,16 +663,6 @@ def _colordiff(a: Any, b: Any) -> tuple[str, str]: return before, after -def colordiff(a, b): - """Colorize differences between two values if color is enabled. - (Like _colordiff but conditional.) - """ - if config["ui"]["color"]: - return _colordiff(a, b) - else: - return str(a), str(b) - - def get_path_formats(subview=None): """Get the configuration's path formats as a list of query/template pairs. @@ -1048,6 +1019,18 @@ def print_newline_layout( FLOAT_EPSILON = 0.01 +def _multi_value_diff(field: str, oldset: set[str], newset: set[str]) -> str: + added = newset - oldset + removed = oldset - newset + + parts = [ + f"{field}:", + *(colorize("text_diff_removed", f" - {i}") for i in sorted(removed)), + *(colorize("text_diff_added", f" + {i}") for i in sorted(added)), + ] + return "\n".join(parts) + + def _field_diff( field: str, old: FormattedMapping, new: FormattedMapping ) -> str | None: @@ -1063,6 +1046,11 @@ def _field_diff( ): return None + if isinstance(oldval, list): + if (oldset := set(oldval)) != (newset := set(newval)): + return _multi_value_diff(field, oldset, newset) + return None + # Get formatted values for output. oldstr, newstr = old.get(field, ""), new.get(field, "") if field not in new: @@ -1071,8 +1059,7 @@ def _field_diff( if field not in old: return colorize("text_diff_added", f"{field}: {newstr}") - # For strings, highlight changes. For others, colorize the whole - # thing. + # For strings, highlight changes. For others, colorize the whole thing. if isinstance(oldval, str): oldstr, newstr = colordiff(oldstr, newstr) else: diff --git a/beets/ui/commands/import_/display.py b/beets/ui/commands/import_/display.py index bdc44d51f..764dafd39 100644 --- a/beets/ui/commands/import_/display.py +++ b/beets/ui/commands/import_/display.py @@ -127,7 +127,7 @@ class ChangeRepresentation: and artist name. """ # Artist. - artist_l, artist_r = self.cur_artist or "", self.match.info.artist + artist_l, artist_r = self.cur_artist or "", self.match.info.artist or "" if artist_r == VARIOUS_ARTISTS: # Hide artists for VA releases. artist_l, artist_r = "", "" diff --git a/beetsplug/_utils/musicbrainz.py b/beetsplug/_utils/musicbrainz.py index 57201e909..95327e75a 100644 --- a/beetsplug/_utils/musicbrainz.py +++ b/beetsplug/_utils/musicbrainz.py @@ -36,6 +36,33 @@ log = logging.getLogger("beets") LUCENE_SPECIAL_CHAR_PAT = re.compile(r'([-+&|!(){}[\]^"~*?:\\/])') +RELEASE_INCLUDES = [ + "artists", + "media", + "recordings", + "release-groups", + "labels", + "artist-credits", + "aliases", + "recording-level-rels", + "work-rels", + "work-level-rels", + "artist-rels", + "isrcs", + "url-rels", + "release-rels", + "genres", + "tags", +] + +RECORDING_INCLUDES = [ + "artists", + "aliases", + "isrcs", + "work-level-rels", + "artist-rels", +] + class LimiterTimeoutSession(LimiterMixin, TimeoutAndRetrySession): """HTTP session that enforces rate limits.""" @@ -223,12 +250,14 @@ class MusicBrainzAPI(RequestHandler): def get_release(self, id_: str, **kwargs: Unpack[LookupKwargs]) -> JSONDict: """Retrieve a release by its MusicBrainz ID.""" + kwargs.setdefault("includes", RELEASE_INCLUDES) return self._lookup("release", id_, **kwargs) def get_recording( self, id_: str, **kwargs: Unpack[LookupKwargs] ) -> JSONDict: """Retrieve a recording by its MusicBrainz ID.""" + kwargs.setdefault("includes", RECORDING_INCLUDES) return self._lookup("recording", id_, **kwargs) def get_work(self, id_: str, **kwargs: Unpack[LookupKwargs]) -> JSONDict: diff --git a/beetsplug/aura.py b/beetsplug/aura.py index c1877db82..b30e66bf0 100644 --- a/beetsplug/aura.py +++ b/beetsplug/aura.py @@ -79,7 +79,8 @@ TRACK_ATTR_MAP = { "month": "month", "day": "day", "bpm": "bpm", - "genre": "genre", + "genre": "genres", + "genres": "genres", "recording-mbid": "mb_trackid", # beets trackid is MB recording "track-mbid": "mb_releasetrackid", "composer": "composer", @@ -109,7 +110,8 @@ ALBUM_ATTR_MAP = { "year": "year", "month": "month", "day": "day", - "genre": "genre", + "genre": "genres", + "genres": "genres", "release-mbid": "mb_albumid", "release-group-mbid": "mb_releasegroupid", } diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 718e0730e..aa0693541 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -33,6 +33,7 @@ import beets 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 @@ -233,8 +234,11 @@ 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"]] + + self.genres = unique_list( + x["name"] + for x in (*data.get("subGenres", []), *data.get("genres", [])) + ) def artists_str(self) -> str | None: if self.artists is not None: @@ -253,7 +257,6 @@ class BeatportRelease(BeatportObject): label_name: str | None category: str | None url: str | None - genre: str | None tracks: list[BeatportTrack] | None = None @@ -263,7 +266,6 @@ class BeatportRelease(BeatportObject): 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 +287,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,12 +307,6 @@ 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. - if data.get("subGenres"): - self.genre = str(data["subGenres"][0].get("name")) - elif data.get("genres"): - self.genre = str(data["genres"][0].get("name")) - class BeatportPlugin(MetadataSourcePlugin): _client: BeatportClient | None = None @@ -483,7 +478,7 @@ class BeatportPlugin(MetadataSourcePlugin): 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 +503,7 @@ class BeatportPlugin(MetadataSourcePlugin): 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/bpd/__init__.py b/beetsplug/bpd/__init__.py index 9496e9a78..16eb8c572 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1137,7 +1137,10 @@ class Server(BaseServer): pass for tagtype, field in self.tagtype_map.items(): - info_lines.append(f"{tagtype}: {getattr(item, field)}") + field_value = getattr(item, field) + if isinstance(field_value, list): + field_value = "; ".join(field_value) + info_lines.append(f"{tagtype}: {field_value}") return info_lines @@ -1351,7 +1354,7 @@ class Server(BaseServer): "AlbumArtist": "albumartist", "AlbumArtistSort": "albumartist_sort", "Label": "label", - "Genre": "genre", + "Genre": "genres", "Date": "year", "OriginalDate": "original_year", "Composer": "composer", diff --git a/beetsplug/discogs/__init__.py b/beetsplug/discogs/__init__.py index bdbeb8fc0..b33af83a2 100644 --- a/beetsplug/discogs/__init__.py +++ b/beetsplug/discogs/__init__.py @@ -352,12 +352,11 @@ class DiscogsPlugin(MetadataSourcePlugin): mediums = [t["medium"] for t in tracks] country = result.data.get("country") data_url = result.data.get("uri") - style = self.format(result.data.get("styles")) - base_genre = self.format(result.data.get("genres")) + styles: list[str] = result.data.get("styles") or [] + genres: list[str] = result.data.get("genres") or [] - genre = base_genre - if self.config["append_style_genre"] and genre is not None and style: - genre += f"{self.config['separator'].as_str()}{style}" + if self.config["append_style_genre"]: + genres.extend(styles) discogs_albumid = self._extract_id(result.data.get("uri")) @@ -411,8 +410,10 @@ class DiscogsPlugin(MetadataSourcePlugin): releasegroup_id=master_id, catalognum=catalogno, country=country, - style=style, - genre=genre, + style=( + self.config["separator"].as_str().join(sorted(styles)) or None + ), + genres=sorted(genres), media=media, original_year=original_year, data_source=self.data_source, @@ -433,14 +434,6 @@ class DiscogsPlugin(MetadataSourcePlugin): return None - def format(self, classification: Iterable[str]) -> str | None: - if classification: - return ( - self.config["separator"].as_str().join(sorted(classification)) - ) - else: - return None - def get_tracks( self, tracklist: list[Track], diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 46e756122..0f358c9a1 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -30,7 +30,12 @@ from beets.util import PromptChoice # These "safe" types can avoid the format/parse cycle that most fields go # through: they are safe to edit with native YAML types. -SAFE_TYPES = (types.BaseFloat, types.BaseInteger, types.Boolean) +SAFE_TYPES = ( + types.BaseFloat, + types.BaseInteger, + types.Boolean, + types.DelimitedString, +) class ParseError(Exception): diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index e4de9181b..789182c33 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -1446,6 +1446,16 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): "move" ].get(bool) + def _is_candidate_fallback(self, candidate: Candidate) -> bool: + try: + return ( + candidate.path is not None + and self.fallback is not None + and os.path.samefile(candidate.path, self.fallback) + ) + except OSError: + return False + # Asynchronous; after music is added to the library. def fetch_art(self, session: ImportSession, task: ImportTask) -> None: """Find art for the album being imported.""" @@ -1494,7 +1504,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): self._set_art(task.album, candidate, not removal_enabled) - if removal_enabled: + if removal_enabled and not self._is_candidate_fallback(candidate): task.prune(candidate.path) # Manual album art fetching. diff --git a/beetsplug/fish.py b/beetsplug/fish.py index 82e035eb4..9de764656 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -16,10 +16,10 @@ """This plugin generates tab completions for Beets commands for the Fish shell , including completions for Beets commands, plugin commands, and option flags. Also generated are completions for all the album -and track fields, suggesting for example `genre:` or `album:` when querying the +and track fields, suggesting for example `genres:` or `album:` when querying the Beets database. Completions for the *values* of those fields are not generated by default but can be added via the `-e` / `--extravalues` flag. For example: -`beet fish -e genre -e albumartist` +`beet fish -e genres -e albumartist` """ import os diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index fde7ff92a..362bab523 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -71,6 +71,12 @@ def split_on_feat( if len(parts) == 2: return parts + # Try comma as separator + # (e.g. "Alice, Bob & Charlie" where Bob and Charlie are featuring) + if for_artist and "," in artist: + comma_parts = artist.split(",", 1) + return comma_parts[0].strip(), comma_parts[1].strip() + # Fall back to all tokens including generic separators if no explicit match if for_artist: regex = re.compile( diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index f7aef0261..41927a87c 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -39,7 +39,7 @@ from beets.util import plurality, unique_list if TYPE_CHECKING: import optparse - from collections.abc import Callable + from collections.abc import Callable, Iterable from beets.importer import ImportSession, ImportTask from beets.library import LibModel @@ -111,7 +111,6 @@ class LastGenrePlugin(plugins.BeetsPlugin): "force": False, "keep_existing": False, "auto": True, - "separator": ", ", "prefer_specific": False, "title_case": True, "pretend": False, @@ -213,7 +212,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): - 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 +236,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): # 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 +260,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): # 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 +272,19 @@ class LastGenrePlugin(plugins.BeetsPlugin): 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 + # First, drop any falsy or whitespace-only genre strings to avoid + # retaining empty tags from multi-valued fields. + cleaned = [g for g in genres if g and g.strip()] + if not self.whitelist: + return cleaned + + return [g for g in cleaned if g.lower() in self.whitelist] # Cached last.fm entity lookups. @@ -329,26 +330,21 @@ 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.""" 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 +355,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 +366,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 @@ -381,7 +377,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): 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 +387,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 +396,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. @@ -479,33 +472,32 @@ class LastGenrePlugin(plugins.BeetsPlugin): 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" - else: - # If the original genre doesn't match a whitelisted genre, check - # if we can canonicalize it to find a matching, whitelisted genre! - if result := _try_resolve_stage( - "original fallback", keep_genres, [] - ): - return result + if genres and self.config["keep_existing"]: + if valid_genres := self._filter_valid(genres): + return valid_genres, "original fallback" + # If the original genre doesn't match a whitelisted genre, check + # if we can canonicalize it to find a matching, whitelisted genre! + if result := _try_resolve_stage( + "original fallback", keep_genres, [] + ): + return result - # Return fallback string. + # 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 9f4581004..090bd617a 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -60,34 +60,6 @@ FIELDS_TO_MB_KEYS = { "alias": "alias", } - -RELEASE_INCLUDES = [ - "artists", - "media", - "recordings", - "release-groups", - "labels", - "artist-credits", - "aliases", - "recording-level-rels", - "work-rels", - "work-level-rels", - "artist-rels", - "isrcs", - "url-rels", - "release-rels", - "genres", - "tags", -] - -TRACK_INCLUDES = [ - "artists", - "aliases", - "isrcs", - "work-level-rels", - "artist-rels", -] - BROWSE_INCLUDES = [ "artist-credits", "work-rels", @@ -672,10 +644,13 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): for source in sources: for genreitem in source: genres[genreitem["name"]] += int(genreitem["count"]) - info.genre = "; ".join( - genre - for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) - ) + if genres: + 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() @@ -797,7 +772,7 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): # A 404 error here is fine. e.g. re-importing a release that has # been deleted on MusicBrainz. try: - res = self.mb_api.get_release(albumid, includes=RELEASE_INCLUDES) + res = self.mb_api.get_release(albumid) except HTTPNotFoundError: self._log.debug("Release {} not found on MusicBrainz.", albumid) return None @@ -813,9 +788,7 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): rel["type"] == "transl-tracklisting" and rel["direction"] == "backward" ): - actual_res = self.mb_api.get_release( - rel["release"]["id"], includes=RELEASE_INCLUDES - ) + actual_res = self.mb_api.get_release(rel["release"]["id"]) # release is potentially a pseudo release release = self.album_info(res) @@ -838,8 +811,6 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): return None with suppress(HTTPNotFoundError): - return self.track_info( - self.mb_api.get_recording(trackid, includes=TRACK_INCLUDES) - ) + return self.track_info(self.mb_api.get_recording(trackid)) return None diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e83345059..e69f6b2ee 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -34,7 +34,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui from beets.plugins import BeetsPlugin -from beets.util import command_output, displayable_path, syspath +from beets.util import command_output, syspath if TYPE_CHECKING: import optparse @@ -425,7 +425,7 @@ class FfmpegBackend(Backend): # call ffmpeg self._log.debug("analyzing {}", item) cmd = self._construct_cmd(item, peak_method) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stderr.splitlines() # parse output @@ -643,18 +643,20 @@ class CommandBackend(Backend): # tag-writing; this turns the mp3gain/aacgain tool into a gain # calculator rather than a tag manipulator because we take care # of changing tags ourselves. - cmd: list[str] = [self.command, "-o", "-s", "s"] - if self.noclip: - # Adjust to avoid clipping. - cmd = [*cmd, "-k"] - else: - # Disable clipping warning. - cmd = [*cmd, "-c"] - cmd = [*cmd, "-d", str(int(target_level - 89))] - cmd = cmd + [syspath(i.path) for i in items] + cmd = [ + self.command, + "-o", + "-s", + "s", + # Avoid clipping or disable clipping warning + "-k" if self.noclip else "-c", + "-d", + str(int(target_level - 89)), + *[str(i.filepath) for i in items], + ] self._log.debug("analyzing {} files", len(items)) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stdout self._log.debug("analysis finished") return self.parse_tool_output( diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index a5cc8e362..ff5e25612 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -359,8 +359,8 @@ class SmartPlaylistPlugin(BeetsPlugin): if extm3u: attr = [(k, entry.item[k]) for k in keys] al = [ - f' {key}="{quote(str(value), safe="/:")}"' - for key, value in attr + f' {k}="{quote("; ".join(v) if isinstance(v, list) else str(v), safe="/:")}"' # noqa: E501 + for k, v in attr ] attrs = "".join(al) comment = ( diff --git a/beetsplug/zero.py b/beetsplug/zero.py index ab1bfa5ca..a72996ba5 100644 --- a/beetsplug/zero.py +++ b/beetsplug/zero.py @@ -123,12 +123,10 @@ class ZeroPlugin(BeetsPlugin): config. """ fields_set = False - - if "disc" in tags and self.config["omit_single_disc"].get(bool): - if item.disctotal == 1: + if self.config["omit_single_disc"].get(bool) and item.disctotal == 1: + for tag in {"disc", "disctotal"} & set(tags): + tags[tag] = None fields_set = True - self._log.debug("disc: {.disc} -> None", item) - tags["disc"] = None if not self.fields_to_progs: self._log.warning("no fields list to remove") diff --git a/docs/changelog.rst b/docs/changelog.rst index 831613866..ff4f286e2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,69 @@ below! Unreleased ---------- +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 + :doc:`plugins/musicbrainz`, :doc:`plugins/beatport`, :doc:`plugins/discogs` + 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"``) are + automatically migrated to the ``genres`` list when you first run beets after + upgrading. The migration runs once when the database schema is updated, + splitting genre strings and writing the changes to the database. The updated + ``genres`` values will be written to media files the next time you run a + command that writes tags (such as ``beet write`` or during import). No manual + action or ``mbsync`` is required. + + The ``genre`` field is split by the first separator found in the string, in + the following order of precedence: + + 1. :doc:`plugins/lastgenre` ``separator`` configuration + 2. Semicolon followed by a space + 3. Comma followed by a space + 4. Slash wrapped by spaces + +Bug fixes +~~~~~~~~~ + +- :doc:`plugins/ftintitle`: Fix handling of multiple featured artists with + ampersand. +- :doc:`plugins/zero`: When the ``omit_single_disc`` option is set, + ``disctotal`` is zeroed alongside ``disc``. +- :doc:`plugins/fetchart`: Prevent deletion of configured fallback cover art + +For plugin developers +~~~~~~~~~~~~~~~~~~~~~ + +- If you maintain a metadata source plugin that populates the ``genre`` field, + please update it to populate a list of ``genres`` instead. You will see a + deprecation warning for now, but support for populating the single ``genre`` + field will be removed in version ``3.0.0``. + +Other changes +~~~~~~~~~~~~~ + +- :ref:`modify-cmd`: Use the following separator to delimit multiple field + values: |semicolon_space|. For example ``beet modify albumtypes="album; ep"``. + Previously, ``\␀`` was used as a separator. This applies to fields such as + ``artists``, ``albumtypes`` etc. +- Improve highlighting of multi-valued fields changes. +- :doc:`plugins/edit`: Editing multi-valued fields now behaves more naturally, + with list values handled directly to make metadata edits smoother and more + predictable. +- :doc:`plugins/lastgenre`: The ``separator`` configuration option is removed. + Since 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 has been + removed. + +2.6.2 (February 22, 2026) +------------------------- + .. New features ~~~~~~~~~~~~ @@ -24,6 +87,10 @@ Bug fixes - :doc:`plugins/musicbrainz`: Fix fetching very large releases that have more than 500 tracks. :bug:`6355` - :doc:`plugins/badfiles`: Fix number of found errors in log message +- :doc:`plugins/replaygain`: Avoid magic Windows prefix in calls to command + backends, such as ``mp3gain``. :bug:`2946` +- :doc:`plugins/mbpseudo`: Fix crash due to missing ``artist_credit`` field in + the MusicBrainz API response. :bug:`6339` .. For plugin developers diff --git a/docs/conf.py b/docs/conf.py index 15ba699cd..8a812d159 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -19,7 +19,7 @@ copyright = "2016, Adrian Sampson" master_doc = "index" language = "en" version = "2.6" -release = "2.6.1" +release = "2.6.2" # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration @@ -98,7 +98,7 @@ man_pages = [ ] # Global substitutions that can be used anywhere in the documentation. -rst_epilog = """ +rst_epilog = r""" .. |Album| replace:: :class:`~beets.library.models.Album` .. |AlbumInfo| replace:: :class:`beets.autotag.hooks.AlbumInfo` .. |BeetsPlugin| replace:: :class:`beets.plugins.BeetsPlugin` @@ -108,6 +108,7 @@ rst_epilog = """ .. |Library| replace:: :class:`~beets.library.library.Library` .. |Model| replace:: :class:`~beets.dbcore.db.Model` .. |TrackInfo| replace:: :class:`beets.autotag.hooks.TrackInfo` +.. |semicolon_space| replace:: :literal:`; \ ` """ # -- Options for HTML output ------------------------------------------------- diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 780042026..3734b57e7 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -116,17 +116,21 @@ Default .. conf:: append_style_genre :default: no - Appends the Discogs style (if found) to the genre tag. This can be useful if - you want more granular genres to categorize your music. For example, - a release in Discogs might have a genre of "Electronic" and a style of - "Techno": enabling this setting would set the genre to be "Electronic, - Techno" (assuming default separator of ``", "``) instead of just - "Electronic". + Appends the Discogs style (if found) to the ``genres`` tag. This can be + useful if you want more granular genres to categorize your music. For + example, a release in Discogs might have a genre of "Electronic" and a style + of "Techno": enabling this setting would append "Techno" to the ``genres`` + list. .. conf:: separator :default: ", " - How to join multiple genre and style values from Discogs into a string. + How to join multiple style values from Discogs into a string. + + .. versionchanged:: 2.7.0 + + This option now only applies to the ``style`` field as beets now only + handles lists of ``genres``. .. conf:: strip_disambiguation :default: yes diff --git a/docs/plugins/fish.rst b/docs/plugins/fish.rst index c1ae4f990..a26b06458 100644 --- a/docs/plugins/fish.rst +++ b/docs/plugins/fish.rst @@ -28,7 +28,7 @@ option flags available to you, which also applies to subcommands such as ``beet import -``. If you type ``beet ls`` followed by a space and then the and the ``TAB`` key, you will see a list of all the album/track fields that can be used in beets queries. For example, typing ``beet ls ge`` will complete to -``genre:`` and leave you ready to type the rest of your query. +``genres:`` and leave you ready to type the rest of your query. Options ------- @@ -42,7 +42,7 @@ commands and option flags. If you want generated completions to also contain album/track field *values* for the items in your library, you can use the ``-e`` or ``--extravalues`` option. For example: ``beet fish -e genre`` or ``beet fish -e genre -e albumartist`` In -the latter case, subsequently typing ``beet list genre: `` will display a +the latter case, subsequently typing ``beet list genres: `` will display a list of all the genres in your library and ``beet list albumartist: `` will show a list of the album artists in your library. Keep in mind that all of these values will be put into the generated completions file, so use this option with diff --git a/docs/plugins/ihate.rst b/docs/plugins/ihate.rst index 47e679dbd..6bb76d796 100644 --- a/docs/plugins/ihate.rst +++ b/docs/plugins/ihate.rst @@ -26,12 +26,12 @@ Here's an example: ihate: warn: - artist:rnb - - genre:soul + - genres:soul # Only warn about tribute albums in rock genre. - - genre:rock album:tribute + - genres:rock album:tribute skip: - - genre::russian\srock - - genre:polka + - genres::russian\srock + - genres:polka - artist:manowar - album:christmas diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index ace7caaf0..fa68ce9db 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,6 @@ 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: ``', '``. - **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/docs/plugins/smartplaylist.rst b/docs/plugins/smartplaylist.rst index f227559a8..48060ea79 100644 --- a/docs/plugins/smartplaylist.rst +++ b/docs/plugins/smartplaylist.rst @@ -121,7 +121,7 @@ instance the following configuration exports the ``id`` and ``genre`` fields: output: extm3u fields: - id - - genre + - genres playlists: - name: all.m3u query: '' @@ -132,7 +132,7 @@ look as follows: :: #EXTM3U - #EXTINF:805 id="1931" genre="Progressive%20Rock",Led Zeppelin - Stairway to Heaven + #EXTINF:805 id="1931" genres="Rock%3B%20Pop",Led Zeppelin - Stairway to Heaven ../music/singles/Led Zeppelin/Stairway to Heaven.mp3 To give a usage example, the webm3u_ and Beetstream_ plugins read the exported diff --git a/docs/plugins/zero.rst b/docs/plugins/zero.rst index bf134e664..fed048330 100644 --- a/docs/plugins/zero.rst +++ b/docs/plugins/zero.rst @@ -31,9 +31,8 @@ to nullify and the conditions for nullifying them: ``keep_fields``---not both! - To conditionally filter a field, use ``field: [regexp, regexp]`` to specify regular expressions. -- Set ``omit_single_disc`` to ``True`` to omit writing the ``disc`` number for - albums with only a single disc (``disctotal == 1``). By default, beets will - number the disc even if the album contains only one disc in total. +- Set ``omit_single_disc`` to ``True`` to omit writing the ``disc`` number and + the ``disctotal`` number for albums with a single disc (``disctotal == 1``). - By default this plugin only affects files' tags; the beets database is left unchanged. To update the tags in the database, set the ``update_database`` option to true. @@ -45,7 +44,6 @@ For example: zero: fields: month day genre genres comments comments: [EAC, LAME, from.+collection, 'ripped by'] - genre: [rnb, 'power metal'] genres: [rnb, 'power metal'] update_database: true diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index c0274553a..6f60d2232 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -143,9 +143,9 @@ Optional command flags: :ref:`set_fields` configuration dictionary. You can use the option multiple times on the command line, like so: - :: +.. code-block:: sh - beet import --set genre="Alternative Rock" --set mood="emotional" + beet import --set genres="Alternative Rock" --set mood="emotional" .. _py7zr: https://pypi.org/project/py7zr/ @@ -267,6 +267,9 @@ Values can also be *templates*, using the same syntax as :doc:`path formats artist sort name into the artist field for all your tracks, and ``beet modify title='$track $title'`` will add track numbers to their title metadata. +To adjust a multi-valued field, such as ``genres``, separate the values with +|semicolon_space|. For example, ``beet modify genres="rock; pop"``. + The ``-a`` option changes to querying album fields instead of track fields and also enables to operate on albums in addition to the individual tracks. Without this flag, the command will only change *track-level* data, even if all the diff --git a/docs/reference/config.rst b/docs/reference/config.rst index b654c118f..4326a80a1 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -853,11 +853,11 @@ set_fields A dictionary indicating fields to set to values for newly imported music. Here's an example: -:: +.. code-block:: yaml set_fields: - genre: 'To Listen' - collection: 'Unordered' + genres: To Listen + collection: Unordered Other field/value pairs supplied via the ``--set`` option on the command-line override any settings here for fields with the same name. @@ -913,6 +913,55 @@ and the next-best match is above the *gap* threshold, the importer will suggest that match but not automatically confirm it. Otherwise, you'll see a list of options to choose from. +.. _distance-weights: + +distance_weights +~~~~~~~~~~~~~~~~ + +The ``distance_weights`` option allows you to customize how much each field +contributes to the overall distance score when matching albums and tracks. +Higher weights mean that differences in that field are penalized more heavily, +making them more important in the matching decision. + +The defaults are: + +.. code-block:: yaml + + match: + distance_weights: + data_source: 2.0 + artist: 3.0 + album: 3.0 + media: 1.0 + mediums: 1.0 + year: 1.0 + country: 0.5 + label: 0.5 + catalognum: 0.5 + albumdisambig: 0.5 + album_id: 5.0 + tracks: 2.0 + missing_tracks: 0.9 + unmatched_tracks: 0.6 + track_title: 3.0 + track_artist: 2.0 + track_index: 1.0 + track_length: 2.0 + track_id: 5.0 + medium: 1.0 + +For example, if you don't care as much about matching the exact release year, +you can reduce its weight: + +.. code-block:: yaml + + match: + distance_weights: + year: 0.1 + +You only need to specify the fields you want to override; unspecified fields +keep their default weights. + .. _max_rec: max_rec @@ -1172,9 +1221,9 @@ Here's an example file: color: yes paths: - default: $genre/$albumartist/$album/$track $title + default: %first{$genres}/$albumartist/$album/$track $title singleton: Singletons/$artist - $title - comp: $genre/$album/$track $title + comp: %first{$genres}/$album/$track $title albumtype:soundtrack: Soundtracks/$album/$track $title .. only:: man diff --git a/docs/reference/pathformat.rst b/docs/reference/pathformat.rst index 10dd3ae05..aff48a7c6 100644 --- a/docs/reference/pathformat.rst +++ b/docs/reference/pathformat.rst @@ -75,11 +75,34 @@ These functions are built in to beets: - ``%time{date_time,format}``: Return the date and time in any format accepted by strftime_. For example, to get the year some music was added to your library, use ``%time{$added,%Y}``. -- ``%first{text}``: Returns the first item, separated by ``;`` (a semicolon - followed by a space). You can use ``%first{text,count,skip}``, where ``count`` - is the number of items (default 1) and ``skip`` is number to skip (default 0). - You can also use ``%first{text,count,skip,sep,join}`` where ``sep`` is the - separator, like ``;`` or ``/`` and join is the text to concatenate the items. +- ``%first{text,count,skip,sep,join}``: Extract a subset of items from a + delimited string. Splits ``text`` by ``sep``, skips the first ``skip`` items, + then returns the next ``count`` items joined by ``join``. + + This is especially useful for multi-valued fields like ``artists`` or + ``genres`` where you may only want the first artist or a limited number of + genres in a path. + + Defaults: + + .. + Comically, you need to follow |semicolon_space| with some punctuation to + make sure it gets rendered correctly as '; ' in the docs. + + - **count**: 1, + - **skip**: 0, + - **sep**: |semicolon_space|, + - **join**: |semicolon_space|. + + Examples: + + :: + + %first{$genres} → returns the first genre + %first{$genres,2} → returns the first two genres, joined by "; " + %first{$genres,2,1} → skips the first genre, returns the next two + %first{$genres,2,0, , -> } → splits by space, joins with " -> " + - ``%ifdef{field}``, ``%ifdef{field,truetext}`` or ``%ifdef{field,truetext,falsetext}``: Checks if an flexible attribute ``field`` is defined. If it exists, then return ``truetext`` or ``field`` diff --git a/docs/team.rst b/docs/team.rst index b23b125ce..f99fcae97 100644 --- a/docs/team.rst +++ b/docs/team.rst @@ -42,12 +42,11 @@ of what you can expect from these *knowledge owners*. ----- - The Discogs plugin -- Other metadata source plugins -- Generalization of source plugin logic (The MetaDataSourcePlugin abstract - class) - Good documentation throughout the project - The smartplaylist plugin -- Things around m3u and other playlist formats +- The lastgenre plugin +- Support for M3U and other playlist formats +- beets as a DJ companion tool (BPM, audio features, key) @RollingStar ------------ @@ -74,6 +73,17 @@ of what you can expect from these *knowledge owners*. - Code quality - Typing +@snejus +------- + +- Grug-minded approach: simple, obvious solutions over clever complexity +- MusicBrainz/autotagger internals and source-plugin behavior +- Query/path handling and SQL-backed lookup behavior +- Typing and tooling modernization (mypy, Ruff, annotations) +- Test architecture, CI reliability, and coverage improvements +- Release and packaging workflows (Poetry/pyproject, dependencies, changelog) +- Cross-plugin refactors (especially metadata and lyrics-related internals) + @wisp3rwind ----------- diff --git a/pyproject.toml b/pyproject.toml index 57a12fa75..0ce774d9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "beets" -version = "2.6.1" +version = "2.6.2" description = "music tagger and library organizer" authors = ["Adrian Sampson "] maintainers = ["Serene-Arc"] diff --git a/test/autotag/test_hooks.py b/test/autotag/test_hooks.py new file mode 100644 index 000000000..e5de089e8 --- /dev/null +++ b/test/autotag/test_hooks.py @@ -0,0 +1,17 @@ +import pytest + +from beets.autotag.hooks import Info + + +@pytest.mark.parametrize( + "genre, expected_genres", + [ + ("Rock", ("Rock",)), + ("Rock; Alternative", ("Rock", "Alternative")), + ], +) +def test_genre_deprecation(genre, expected_genres): + with pytest.warns( + DeprecationWarning, match="The 'genre' parameter is deprecated" + ): + assert tuple(Info(genre=genre).genres) == expected_genres diff --git a/test/library/__init__.py b/test/library/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test/library/test_migrations.py b/test/library/test_migrations.py new file mode 100644 index 000000000..2c0dece8b --- /dev/null +++ b/test/library/test_migrations.py @@ -0,0 +1,72 @@ +import pytest + +from beets.dbcore import types +from beets.library.migrations import MultiGenreFieldMigration +from beets.library.models import Album, Item +from beets.test.helper import TestHelper + + +class TestMultiGenreFieldMigration: + @pytest.fixture + def helper(self, monkeypatch): + # do not apply migrations upon library initialization + monkeypatch.setattr("beets.library.library.Library._migrations", ()) + # add genre field to both models to make sure this column is created + monkeypatch.setattr( + "beets.library.models.Item._fields", + {**Item._fields, "genre": types.STRING}, + ) + monkeypatch.setattr( + "beets.library.models.Album._fields", + {**Album._fields, "genre": types.STRING}, + ) + monkeypatch.setattr( + "beets.library.models.Album.item_keys", + {*Album.item_keys, "genre"}, + ) + helper = TestHelper() + helper.setup_beets() + + # and now configure the migrations to be tested + monkeypatch.setattr( + "beets.library.library.Library._migrations", + ((MultiGenreFieldMigration, (Item, Album)),), + ) + yield helper + + helper.teardown_beets() + + def test_migrates_only_rows_with_missing_genres(self, helper: TestHelper): + helper.config["lastgenre"]["separator"] = " - " + + expected_item_genres = [] + for genre, initial_genres, expected_genres in [ + # already existing value is not overwritten + ("Item Rock", ("Ignored",), ("Ignored",)), + ("", (), ()), + ("Rock", (), ("Rock",)), + # multiple genres are split on one of default separators + ("Item Rock; Alternative", (), ("Item Rock", "Alternative")), + # multiple genres are split the first (lastgenre) separator ONLY + ("Item - Rock, Alternative", (), ("Item", "Rock, Alternative")), + ]: + helper.add_item(genre=genre, genres=initial_genres) + expected_item_genres.append(expected_genres) + + unmigrated_album = helper.add_album( + genre="Album Rock / Alternative", genres=[] + ) + expected_item_genres.append(("Album Rock", "Alternative")) + + helper.lib._migrate() + + actual_item_genres = [tuple(i.genres) for i in helper.lib.items()] + assert actual_item_genres == expected_item_genres + + unmigrated_album.load() + assert unmigrated_album.genres == ["Album Rock", "Alternative"] + + # remove cached initial db tables data + del helper.lib.db_tables + assert helper.lib.migration_exists("multi_genre_field", "items") + assert helper.lib.migration_exists("multi_genre_field", "albums") diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index 02d23d59b..9f5e6c216 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -310,6 +310,16 @@ class FSArtTest(UseThePlugin): ] assert candidates == paths + @patch("os.path.samefile") + def test_is_candidate_fallback_os_error(self, mock_samefile): + mock_samefile.side_effect = OSError("os error") + fallback = os.path.join(self.temp_dir, b"a.jpg") + self.plugin.fallback = fallback + candidate = fetchart.Candidate(logger, self.source.ID, fallback) + result = self.plugin._is_candidate_fallback(candidate) + mock_samefile.assert_called_once() + assert not result + class CombinedTest(FetchImageTestCase, CAAHelper): ASIN = "xxxx" diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index b92a3bf15..96386d8b6 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -474,7 +474,7 @@ class BeatportTest(BeetsTestCase): item.year = 2016 item.comp = False item.label_name = "Gravitas Recordings" - item.genre = "Glitch Hop" + item.genres = ["Glitch Hop", "Breaks"] item.year = 2016 item.month = 4 item.day = 11 @@ -583,7 +583,7 @@ 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 + assert track.genres == test_track.genres class BeatportResponseEmptyTest(unittest.TestCase): @@ -634,7 +634,7 @@ class BeatportResponseEmptyTest(unittest.TestCase): 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 @@ class BeatportResponseEmptyTest(unittest.TestCase): 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 15d47db6c..cef84e3a9 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -362,7 +362,7 @@ class DGAlbumInfoTest(BeetsTestCase): 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): @@ -371,7 +371,7 @@ class DGAlbumInfoTest(BeetsTestCase): 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): @@ -381,7 +381,7 @@ class DGAlbumInfoTest(BeetsTestCase): 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_ftintitle.py b/test/plugins/test_ftintitle.py index 560f44402..2bba41ad0 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -324,6 +324,11 @@ def test_find_feat_part( ("Alice feat. Bob", ("Alice", "Bob")), ("Alice featuring Bob", ("Alice", "Bob")), ("Alice & Bob", ("Alice", "Bob")), + ("Alice, Bob & Charlie", ("Alice", "Bob & Charlie")), + ( + "Alice, Bob & Charlie feat. Xavier", + ("Alice, Bob & Charlie", "Xavier"), + ), ("Alice and Bob", ("Alice", "Bob")), ("Alice With Bob", ("Alice", "Bob")), ("Alice defeat Bob", ("Alice defeat Bob", None)), diff --git a/test/plugins/test_ihate.py b/test/plugins/test_ihate.py index f941d566c..b64b8d91d 100644 --- a/test/plugins/test_ihate.py +++ b/test/plugins/test_ihate.py @@ -11,7 +11,7 @@ class IHatePluginTest(unittest.TestCase): def test_hate(self): match_pattern = {} test_item = Item( - genre="TestGenre", album="TestAlbum", artist="TestArtist" + genres=["TestGenre"], album="TestAlbum", artist="TestArtist" ) task = importer.SingletonImportTask(None, test_item) @@ -27,19 +27,19 @@ class IHatePluginTest(unittest.TestCase): assert IHatePlugin.do_i_hate_this(task, match_pattern) # Query is blocked by AND clause. - match_pattern = ["album:notthis genre:testgenre"] + match_pattern = ["album:notthis genres:testgenre"] assert not IHatePlugin.do_i_hate_this(task, match_pattern) # Both queries are blocked by AND clause with unmatched condition. match_pattern = [ - "album:notthis genre:testgenre", + "album:notthis genres:testgenre", "artist:testartist album:notthis", ] assert not IHatePlugin.do_i_hate_this(task, match_pattern) # Only one query should fire. match_pattern = [ - "album:testalbum genre:testgenre", + "album:testalbum genres:testgenre", "artist:testartist album:notthis", ] assert IHatePlugin.do_i_hate_this(task, match_pattern) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index f499992c6..6646acbda 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -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.""" 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.""" @@ -201,10 +203,20 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): assert res == ["ambient", "electronic"] +@pytest.fixture +def config(config): + """Provide a fresh beets configuration for every test/parameterize call + + This is necessary to prevent the following parameterized test to bleed + config test state in between test cases. + """ + return config + + @pytest.mark.parametrize( "config_values, item_genre, mock_genres, expected_result", [ - # 0 - force and keep whitelisted + # force and keep whitelisted ( { "force": True, @@ -215,13 +227,30 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # force and keep whitelisted, unknown original + ( + { + "force": True, + "keep_existing": True, + "source": "album", + "whitelist": True, + "canonical": False, + "prefer_specific": False, + "count": 10, + }, + ["original unknown", "Blues"], + { + "album": ["Jazz"], + }, + (["Blues", "Jazz"], "keep + album, whitelist"), + ), + # force and keep whitelisted on empty tag ( { "force": True, @@ -231,29 +260,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + [], { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Jazz"], "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 and keep, artist configured ( { "force": True, @@ -262,15 +275,16 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "whitelist": True, "canonical": False, "prefer_specific": False, + "count": 10, }, - "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 + # don't force, disabled whitelist ( { "force": False, @@ -280,13 +294,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # don't force and empty is regular last.fm fetch; no whitelist too ( { "force": False, @@ -296,13 +310,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazzin"], }, - ("Jazzin", "album, any"), + (["Jazzin"], "album, any"), ), - # 6 - fallback to next stages until found + # fallback to next stages until found ( { "force": True, @@ -311,16 +325,17 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "whitelist": False, "canonical": False, "prefer_specific": False, + "count": 10, }, - "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 + # Keep the original genre when force and keep_existing are on, and # whitelist is disabled ( { @@ -332,15 +347,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # Keep the original genre when force and keep_existing are on, and # whitelist is enabled, and genre is valid. ( { @@ -352,15 +367,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # Return the configured fallback when force is on but # keep_existing is not. ( { @@ -372,15 +387,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # fallback to fallback if no original ( { "force": True, @@ -391,32 +406,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # limit a lot of results ( { "force": True, @@ -426,31 +424,17 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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"), + ( + ["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"), - ), - # 12 - fallback to next stage (artist) if no allowed original present + # fallback to next stage (artist) if no allowed original present # and no album genre were fetched. ( { @@ -462,15 +446,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # 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 +468,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # 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 +491,22 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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 + # 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,16 +522,16 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "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", ), ), - # 16 - canonicalization transforms non-whitelisted original genres to canonical + # canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works, **even** when no new genres are found online. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the @@ -556,19 +546,21 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "prefer_specific": False, "count": 10, }, - "Cosmic Disco", + ["Cosmic Disco"], { "album": [], "artist": [], }, ( - "Disco, Electronic", + ["Disco", "Electronic"], "keep + original fallback, whitelist", ), ), ], ) -def test_get_genre(config_values, item_genre, mock_genres, expected_result): +def test_get_genre( + config, config_values, item_genre, mock_genres, expected_result +): """Test _get_genre with various configurations.""" def mock_fetch_track_genre(self, trackartist, tracktitle): @@ -592,9 +584,9 @@ 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 + item.genres = item_genre # Run - res = plugin._get_genre(item) - assert res == expected_result + assert plugin._get_genre(item) == expected_result diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 8d7c5a2f8..e000e16ec 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -526,20 +526,20 @@ 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 release = self._make_release() d = self.mb.album_info(release) - assert d.genre is None + assert d.genres == [] def test_ignored_media(self): config["match"]["ignored_media"] = ["IGNORED1", "IGNORED2"] diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 094349b25..054b70a76 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -14,12 +14,11 @@ import unittest -from typing import ClassVar +from typing import Any, ClassVar import pytest from mediafile import MediaFile -from beets import config from beets.test.helper import ( AsIsImporterMixin, ImportTestCase, @@ -39,10 +38,15 @@ try: except (ImportError, ValueError): GST_AVAILABLE = False -if any(has_program(cmd, ["-v"]) for cmd in ["mp3gain", "aacgain"]): - GAIN_PROG_AVAILABLE = True -else: - GAIN_PROG_AVAILABLE = False + +GAIN_PROG = next( + ( + cmd + for cmd in ["mp3gain", "mp3rgain", "aacgain"] + if has_program(cmd, ["-v"]) + ), + None, +) FFMPEG_AVAILABLE = has_program("ffmpeg", ["-version"]) @@ -63,14 +67,18 @@ class ReplayGainTestCase(PluginMixin, ImportTestCase): plugin = "replaygain" preload_plugin = False - backend: ClassVar[str] + plugin_config: ClassVar[dict[str, Any]] + + @property + def backend(self): + return self.plugin_config["backend"] def setUp(self): # Implemented by Mixins, see above. This may decide to skip the test. self.test_backend() super().setUp() - self.config["replaygain"]["backend"] = self.backend + self.config["replaygain"].set(self.plugin_config) self.load_plugins() @@ -81,8 +89,16 @@ class ThreadedImportMixin: self.config["threaded"] = True -class GstBackendMixin: - backend = "gstreamer" +class BackendMixin: + plugin_config: ClassVar[dict[str, Any]] + has_r128_support: bool + + def test_backend(self): + """Check whether the backend actually has all required functionality.""" + + +class GstBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "gstreamer"} has_r128_support = True def test_backend(self): @@ -90,30 +106,25 @@ class GstBackendMixin: try: # Check if required plugins can be loaded by instantiating a # GStreamerBackend (via its .__init__). - config["replaygain"]["targetlevel"] = 89 - GStreamerBackend(config["replaygain"], None) + self.config["replaygain"]["targetlevel"] = 89 + GStreamerBackend(self.config["replaygain"], None) except FatalGstreamerPluginReplayGainError as e: # Skip the test if plugins could not be loaded. self.skipTest(str(e)) -class CmdBackendMixin: - backend = "command" +class CmdBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = { + "backend": "command", + "command": GAIN_PROG, + } has_r128_support = False - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - -class FfmpegBackendMixin: - backend = "ffmpeg" +class FfmpegBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "ffmpeg"} has_r128_support = True - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - class ReplayGainCliTest: FNAME: str @@ -332,7 +343,7 @@ class ReplayGainGstCliTest( FNAME = "full" # file contains only silence -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdCliTest( ReplayGainCliTest, ReplayGainTestCase, CmdBackendMixin ): @@ -369,7 +380,7 @@ class ReplayGainGstImportTest(ImportTest, ReplayGainTestCase, GstBackendMixin): pass -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdImportTest(ImportTest, ReplayGainTestCase, CmdBackendMixin): pass diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index 7cc712330..d1125158f 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -76,11 +76,11 @@ class SmartPlaylistTest(BeetsTestCase): {"name": "one_non_empty_sort", "query": ["foo year+", "bar"]}, { "name": "multiple_sorts", - "query": ["foo year+", "bar genre-"], + "query": ["foo year+", "bar genres-"], }, { "name": "mixed", - "query": ["foo year+", "bar", "baz genre+ id-"], + "query": ["foo year+", "bar", "baz genres+ id-"], }, ] ) @@ -102,11 +102,11 @@ class SmartPlaylistTest(BeetsTestCase): # Multiple queries store individual sorts in the tuple assert all(isinstance(x, NullSort) for x in sorts["only_empty_sorts"]) assert sorts["one_non_empty_sort"] == [sort("year"), NullSort()] - assert sorts["multiple_sorts"] == [sort("year"), sort("genre", False)] + assert sorts["multiple_sorts"] == [sort("year"), sort("genres", False)] assert sorts["mixed"] == [ sort("year"), NullSort(), - MultipleSort([sort("genre"), sort("id", False)]), + MultipleSort([sort("genres"), sort("id", False)]), ] def test_matches(self): @@ -259,7 +259,7 @@ class SmartPlaylistTest(BeetsTestCase): type(i).title = PropertyMock(return_value="fake Title") type(i).length = PropertyMock(return_value=300.123) type(i).path = PropertyMock(return_value=b"/tagada.mp3") - a = {"id": 456, "genre": "Fake Genre"} + a = {"id": 456, "genres": ["Rock", "Pop"]} i.__getitem__.side_effect = a.__getitem__ i.evaluate_template.side_effect = lambda pl, _: pl.replace( b"$title", @@ -280,7 +280,7 @@ class SmartPlaylistTest(BeetsTestCase): config["smartplaylist"]["output"] = "extm3u" config["smartplaylist"]["relative_to"] = False config["smartplaylist"]["playlist_dir"] = str(dir) - config["smartplaylist"]["fields"] = ["id", "genre"] + config["smartplaylist"]["fields"] = ["id", "genres"] try: spl.update_playlists(lib) except Exception: @@ -297,7 +297,7 @@ class SmartPlaylistTest(BeetsTestCase): assert content == ( b"#EXTM3U\n" - b'#EXTINF:300 id="456" genre="Fake%20Genre",Fake Artist - fake Title\n' + b'#EXTINF:300 id="456" genres="Rock%3B%20Pop",Fake Artist - fake Title\n' b"/tagada.mp3\n" ) diff --git a/test/plugins/test_zero.py b/test/plugins/test_zero.py index 23eb0e3cf..16ceaf56d 100644 --- a/test/plugins/test_zero.py +++ b/test/plugins/test_zero.py @@ -256,7 +256,8 @@ class ZeroPluginTest(IOMixin, PluginTestCase): mf = MediaFile(syspath(item.path)) assert mf.comments is None - assert mf.disc == 0 + assert mf.disc is None + assert mf.disctotal is None def test_omit_single_disc_with_tags_multi(self): item = self.add_item_fixture( @@ -271,6 +272,7 @@ class ZeroPluginTest(IOMixin, PluginTestCase): mf = MediaFile(syspath(item.path)) assert mf.comments is None assert mf.disc == 1 + assert mf.disctotal == 4 def test_omit_single_disc_only_change_single(self): item = self.add_item_fixture(disctotal=1, disc=1) @@ -280,7 +282,8 @@ class ZeroPluginTest(IOMixin, PluginTestCase): item.write() mf = MediaFile(syspath(item.path)) - assert mf.disc == 0 + assert mf.disc is None + assert mf.disctotal is None def test_omit_single_disc_only_change_multi(self): item = self.add_item_fixture(disctotal=4, disc=1) @@ -291,6 +294,7 @@ class ZeroPluginTest(IOMixin, PluginTestCase): mf = MediaFile(syspath(item.path)) assert mf.disc == 1 + assert mf.disctotal == 4 def test_empty_query_n_response_no_changes(self): item = self.add_item_fixture( diff --git a/test/rsrc/unicode’d.mp3 b/test/rsrc/unicode’d.mp3 index f7e8b6285..2b306cc13 100644 Binary files a/test/rsrc/unicode’d.mp3 and b/test/rsrc/unicode’d.mp3 differ diff --git a/test/test_files.py b/test/test_files.py index d0d93987c..4ed2f8608 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -165,7 +165,7 @@ class MoveTest(BeetsTestCase): self.i.move(operation=MoveOperation.LINK) assert self.dest.exists() assert os.path.islink(syspath(self.dest)) - assert self.dest.resolve() == self.path + assert self.dest.resolve() == self.path.resolve() @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_link_does_not_depart(self): diff --git a/test/test_importer.py b/test/test_importer.py index 6ae7d562b..56327af06 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -131,7 +131,7 @@ class NonAutotaggedImportTest(PathsMixin, AsIsImporterMixin, ImportTestCase): assert self.track_lib_path.exists() assert self.track_lib_path.is_symlink() - assert self.track_lib_path.resolve() == self.track_import_path + assert self.track_lib_path.resolve() == self.track_import_path.resolve() @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_import_hardlink_arrives(self): @@ -304,15 +304,15 @@ class ImportSingletonTest(AutotagImportTestCase): assert len(self.lib.albums()) == 2 def test_set_fields(self): - genre = "\U0001f3b7 Jazz" + genres = ["\U0001f3b7 Jazz", "Rock"] collection = "To Listen" disc = 0 config["import"]["set_fields"] = { + "genres": "; ".join(genres), "collection": collection, - "genre": genre, - "title": "$title - formatted", "disc": disc, + "title": "$title - formatted", } # As-is item import. @@ -322,7 +322,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() # TODO: Not sure this is necessary. - assert item.genre == genre + assert item.genres == genres assert item.collection == collection assert item.title == "Tag Track 1 - formatted" assert item.disc == disc @@ -337,7 +337,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() - assert item.genre == genre + assert item.genres == genres assert item.collection == collection assert item.title == "Applied Track 1 - formatted" assert item.disc == disc @@ -373,12 +373,12 @@ class ImportTest(PathsMixin, AutotagImportTestCase): config["import"]["from_scratch"] = True for mediafile in self.import_media: - mediafile.genre = "Tag Genre" + mediafile.genres = ["Tag Genre"] mediafile.save() self.importer.add_choice(importer.Action.APPLY) self.importer.run() - assert self.lib.items().get().genre == "" + assert not self.lib.items().get().genres def test_apply_from_scratch_keeps_format(self): config["import"]["from_scratch"] = True @@ -464,17 +464,17 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.lib.items().get().data_source def test_set_fields(self): - genre = "\U0001f3b7 Jazz" + genres = ["\U0001f3b7 Jazz", "Rock"] collection = "To Listen" - comments = "managed by beets" disc = 0 + comments = "managed by beets" config["import"]["set_fields"] = { - "genre": genre, + "genres": "; ".join(genres), "collection": collection, + "disc": disc, "comments": comments, "album": "$album - formatted", - "disc": disc, } # As-is album import. @@ -483,11 +483,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - album.load() # TODO: Not sure this is necessary. - assert album.genre == genre + assert album.genres == genres assert album.comments == comments for item in album.items(): - assert item.get("genre", with_album=False) == genre + assert item.get("genres", with_album=False) == genres assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( @@ -505,11 +504,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - album.load() - assert album.genre == genre + assert album.genres == genres assert album.comments == comments for item in album.items(): - assert item.get("genre", with_album=False) == genre + assert item.get("genres", with_album=False) == genres assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( diff --git a/test/test_library.py b/test/test_library.py index 4acf34746..5af6f76d8 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -56,25 +56,18 @@ class LoadTest(ItemInDBTestCase): class StoreTest(ItemInDBTestCase): def test_store_changes_database_value(self): - self.i.year = 1987 + new_year = 1987 + self.i.year = new_year self.i.store() - new_year = ( - self.lib._connection() - .execute("select year from items where title = ?", (self.i.title,)) - .fetchone()["year"] - ) - assert new_year == 1987 + + assert self.lib.get_item(self.i.id).year == new_year def test_store_only_writes_dirty_fields(self): - original_genre = self.i.genre - self.i._values_fixed["genre"] = "beatboxing" # change w/o dirtying + new_year = 1987 + self.i._values_fixed["year"] = new_year # change w/o dirtying self.i.store() - new_genre = ( - self.lib._connection() - .execute("select genre from items where title = ?", (self.i.title,)) - .fetchone()["genre"] - ) - assert new_genre == original_genre + + assert self.lib.get_item(self.i.id).year != new_year def test_store_clears_dirty_flags(self): self.i.composer = "tvp" @@ -688,14 +681,14 @@ 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._assert_dest(b"/base/Pop") + self.i.albumtypes = ["album", "compilation"] + self._setf("%first{$albumtypes}") + self._assert_dest(b"/base/album") def test_first_skip(self): - self.i.genres = "Pop; Rock; Classical Crossover" - self._setf("%first{$genres,1,2}") - self._assert_dest(b"/base/Classical Crossover") + self.i.albumtype = "album; ep; compilation" + self._setf("%first{$albumtype,1,2}") + self._assert_dest(b"/base/compilation") def test_first_different_sep(self): self._setf("%first{Alice / Bob / Eve,2,0, / , & }") diff --git a/test/test_plugins.py b/test/test_plugins.py index 4786b12b4..9622fb8db 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -96,8 +96,7 @@ class TestPluginRegistration(IOMixin, PluginTestCase): item.add(self.lib) out = self.run_with_output("ls", "-f", "$multi_value") - delimiter = types.MULTI_VALUE_DSV.delimiter - assert out == f"one{delimiter}two{delimiter}three\n" + assert out == "one; two; three\n" class PluginImportTestCase(ImportHelper, PluginTestCase): diff --git a/test/test_query.py b/test/test_query.py index 0ddf83e3a..81532c436 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -71,7 +71,7 @@ class TestGet: album="baz", year=2001, comp=True, - genre="rock", + genres=["rock"], ), helper.create_item( title="second", @@ -80,7 +80,7 @@ class TestGet: album="baz", year=2002, comp=True, - genre="Rock", + genres=["Rock"], ), ] album = helper.lib.add_album(album_items) @@ -94,7 +94,7 @@ class TestGet: album="foo", year=2003, comp=False, - genre="Hard Rock", + genres=["Hard Rock"], comments="caf\xe9", ) @@ -125,12 +125,12 @@ class TestGet: ("comments:caf\xe9", ["third"]), ("comp:true", ["first", "second"]), ("comp:false", ["third"]), - ("genre:=rock", ["first"]), - ("genre:=Rock", ["second"]), - ('genre:="Hard Rock"', ["third"]), - ('genre:=~"hard rock"', ["third"]), - ("genre:=~rock", ["first", "second"]), - ('genre:="hard rock"', []), + ("genres:=rock", ["first"]), + ("genres:=Rock", ["second"]), + ('genres:="Hard Rock"', ["third"]), + ('genres:=~"hard rock"', ["third"]), + ("genres:=~rock", ["first", "second"]), + ('genres:="hard rock"', []), ("popebear", []), ("pope:bear", []), ("singleton:true", ["third"]), @@ -243,13 +243,7 @@ class TestGet: class TestMatch: @pytest.fixture(scope="class") def item(self): - return _common.item( - album="the album", - disc=6, - genre="the genre", - year=1, - bitrate=128000, - ) + return _common.item(album="the album", disc=6, year=1, bitrate=128000) @pytest.mark.parametrize( "q, should_match", @@ -260,9 +254,9 @@ class TestMatch: (SubstringQuery("album", "album"), True), (SubstringQuery("album", "ablum"), False), (SubstringQuery("disc", "6"), True), - (StringQuery("genre", "the genre"), True), - (StringQuery("genre", "THE GENRE"), True), - (StringQuery("genre", "genre"), False), + (StringQuery("album", "the album"), True), + (StringQuery("album", "THE ALBUM"), True), + (StringQuery("album", "album"), False), (NumericQuery("year", "1"), True), (NumericQuery("year", "10"), False), (NumericQuery("bitrate", "100000..200000"), True), diff --git a/test/test_sort.py b/test/test_sort.py index 460aa07b8..d7d651de5 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -33,7 +33,7 @@ class DummyDataTestCase(BeetsTestCase): albums = [ Album( album="Album A", - genre="Rock", + genres=["Rock"], year=2001, flex1="Flex1-1", flex2="Flex2-A", @@ -41,7 +41,7 @@ class DummyDataTestCase(BeetsTestCase): ), Album( album="Album B", - genre="Rock", + genres=["Rock"], year=2001, flex1="Flex1-2", flex2="Flex2-A", @@ -49,7 +49,7 @@ class DummyDataTestCase(BeetsTestCase): ), Album( album="Album C", - genre="Jazz", + genres=["Jazz"], year=2005, flex1="Flex1-1", flex2="Flex2-B", @@ -236,19 +236,19 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): def test_sort_two_field_asc(self): q = "" - s1 = dbcore.query.FixedFieldSort("genre", True) + s1 = dbcore.query.FixedFieldSort("genres", True) s2 = dbcore.query.FixedFieldSort("album", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) sort.add_sort(s2) results = self.lib.albums(q, sort) - assert results[0]["genre"] <= results[1]["genre"] - assert results[1]["genre"] <= results[2]["genre"] - assert results[1]["genre"] == "Rock" - assert results[2]["genre"] == "Rock" + assert results[0]["genres"] <= results[1]["genres"] + assert results[1]["genres"] <= results[2]["genres"] + assert results[1]["genres"] == ["Rock"] + assert results[2]["genres"] == ["Rock"] assert results[1]["album"] <= results[2]["album"] # same thing with query string - q = "genre+ album+" + q = "genres+ album+" results2 = self.lib.albums(q) for r1, r2 in zip(results, results2): assert r1.id == r2.id @@ -388,7 +388,7 @@ class CaseSensitivityTest(DummyDataTestCase): album = Album( album="album", - genre="alternative", + genres=["alternative"], year="2001", flex1="flex1", flex2="flex2-A", diff --git a/test/ui/commands/test_list.py b/test/ui/commands/test_list.py index 372d75410..0828980ca 100644 --- a/test/ui/commands/test_list.py +++ b/test/ui/commands/test_list.py @@ -63,6 +63,6 @@ class ListTest(IOMixin, BeetsTestCase): assert "the artist - the album - 0001" == stdout.strip() def test_list_album_format(self): - stdout = self._run_list(album=True, fmt="$genre") + stdout = self._run_list(album=True, fmt="$genres") assert "the genre" in stdout assert "the album" not in stdout diff --git a/test/ui/commands/test_update.py b/test/ui/commands/test_update.py index 3fb687418..937ded10c 100644 --- a/test/ui/commands/test_update.py +++ b/test/ui/commands/test_update.py @@ -103,22 +103,22 @@ class UpdateTest(IOMixin, BeetsTestCase): def test_selective_modified_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) mf.title = "differentTitle" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=True, fields=["title"]) item = self.lib.items().get() assert b"differentTitle" in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_selective_modified_metadata_not_moved(self): mf = MediaFile(syspath(self.i.path)) mf.title = "differentTitle" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=False, fields=["title"]) item = self.lib.items().get() assert b"differentTitle" not in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_modified_album_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) @@ -141,22 +141,22 @@ class UpdateTest(IOMixin, BeetsTestCase): def test_selective_modified_album_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) mf.album = "differentAlbum" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=True, fields=["album"]) item = self.lib.items().get() assert b"differentAlbum" in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_selective_modified_album_metadata_not_moved(self): mf = MediaFile(syspath(self.i.path)) mf.album = "differentAlbum" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() - self._update(move=True, fields=["genre"]) + self._update(move=True, fields=["genres"]) item = self.lib.items().get() assert b"differentAlbum" not in item.path - assert item.genre == "differentGenre" + assert item.genres == ["differentGenre"] def test_mtime_match_skips_update(self): mf = MediaFile(syspath(self.i.path)) diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py index 35f3c6ca7..24bac0123 100644 --- a/test/ui/test_field_diff.py +++ b/test/ui/test_field_diff.py @@ -1,3 +1,5 @@ +from textwrap import dedent + import pytest from beets.library import Item @@ -15,7 +17,7 @@ class TestFieldDiff: def patch_colorize(self, monkeypatch): """Patch to return a deterministic string format instead of ANSI codes.""" monkeypatch.setattr( - "beets.ui.colorize", + "beets.ui._colorize", lambda color_name, text: f"[{color_name}]{text}[/]", ) @@ -32,12 +34,25 @@ class TestFieldDiff: p({"title": "foo"}, {"title": "bar"}, "title", f"title: {diff_fmt('foo', 'bar')}", id="string_full_replace"), p({"title": "prefix foo"}, {"title": "prefix bar"}, "title", "title: prefix [text_diff_removed]foo[/] -> prefix [text_diff_added]bar[/]", id="string_partial_change"), p({"year": 2000}, {"year": 2001}, "year", f"year: {diff_fmt('2000', '2001')}", id="int_changed"), - p({}, {"genre": "Rock"}, "genre", "genre: -> [text_diff_added]Rock[/]", id="field_added"), - p({"genre": "Rock"}, {}, "genre", "genre: [text_diff_removed]Rock[/] -> ", id="field_removed"), + p({}, {"artist": "Artist"}, "artist", "artist: -> [text_diff_added]Artist[/]", id="field_added"), + p({"artist": "Artist"}, {}, "artist", "artist: [text_diff_removed]Artist[/] -> ", id="field_removed"), p({"track": 1}, {"track": 2}, "track", f"track: {diff_fmt('01', '02')}", id="formatted_value_changed"), p({"mb_trackid": None}, {"mb_trackid": "1234"}, "mb_trackid", "mb_trackid: -> [text_diff_added]1234[/]", id="none_to_value"), p({}, {"new_flex": "foo"}, "new_flex", "[text_diff_added]new_flex: foo[/]", id="flex_field_added"), p({"old_flex": "foo"}, {}, "old_flex", "[text_diff_removed]old_flex: foo[/]", id="flex_field_removed"), + p({"albumtypes": ["album", "ep"]}, {"albumtypes": ["ep", "album"]}, "albumtypes", None, id="multi_value_unchanged"), + p( + {"albumtypes": ["ep"]}, + {"albumtypes": ["album", "compilation"]}, + "albumtypes", + dedent(""" + albumtypes: + [text_diff_removed] - ep[/] + [text_diff_added] + album[/] + [text_diff_added] + compilation[/] + """).strip(), + id="multi_value_changed" + ), ], ) # fmt: skip @pytest.mark.parametrize("color", [True], ids=["color_enabled"])