diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index cc172d0d8..110cd70d0 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -34,10 +34,14 @@ from collections.abc import ( Mapping, Sequence, ) +from functools import cached_property from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, Generic -from typing_extensions import TypeVar # default value support +from typing_extensions import ( + Self, + TypeVar, # default value support +) from unidecode import unidecode import beets @@ -83,6 +87,10 @@ class DBCustomFunctionError(Exception): ) +class NotFoundError(LookupError): + pass + + class FormattedMapping(Mapping[str, str]): """A `dict`-like formatted view of a model. @@ -97,6 +105,8 @@ class FormattedMapping(Mapping[str, str]): are replaced. """ + model: Model + ALL_KEYS = "*" def __init__( @@ -360,6 +370,22 @@ class Model(ABC, Generic[D]): """Fields in the related table.""" return cls._relation._fields.keys() - cls.shared_db_fields + @cached_property + def db(self) -> D: + """Get the database associated with this object. + + This validates that the database is attached and the object has an id. + """ + return self._check_db() + + def get_fresh_from_db(self) -> Self: + """Load this object from the database.""" + model_cls = self.__class__ + if obj := self.db._get(model_cls, self.id): + return obj + + raise NotFoundError(f"No matching {model_cls.__name__} found") from None + @classmethod def _getters(cls: type[Model]): """Return a mapping from field names to getter functions.""" @@ -599,7 +625,6 @@ class Model(ABC, Generic[D]): """ if fields is None: fields = self._fields - db = self._check_db() # Build assignments for query. assignments = [] @@ -611,7 +636,7 @@ class Model(ABC, Generic[D]): value = self._type(key).to_sql(self[key]) subvars.append(value) - with db.transaction() as tx: + with self.db.transaction() as tx: # Main table update. if assignments: query = f"UPDATE {self._table} SET {','.join(assignments)} WHERE id=?" @@ -645,21 +670,16 @@ class Model(ABC, Generic[D]): If check_revision is true, the database is only queried loaded when a transaction has been committed since the item was last loaded. """ - db = self._check_db() - if not self._dirty and db.revision == self._revision: + if not self._dirty and self.db.revision == self._revision: # Exit early return - stored_obj = db._get(type(self), self.id) - assert stored_obj is not None, f"object {self.id} not in DB" - self._values_fixed = LazyConvertDict(self) - self._values_flex = LazyConvertDict(self) - self.update(dict(stored_obj)) + + self.__dict__.update(self.get_fresh_from_db().__dict__) self.clear_dirty() def remove(self): """Remove the object's associated rows from the database.""" - db = self._check_db() - with db.transaction() as tx: + with self.db.transaction() as tx: tx.mutate(f"DELETE FROM {self._table} WHERE id=?", (self.id,)) tx.mutate( f"DELETE FROM {self._flex_table} WHERE entity_id=?", (self.id,) @@ -675,7 +695,7 @@ class Model(ABC, Generic[D]): """ if db: self._db = db - db = self._check_db(False) + db = self._check_db(need_id=False) with db.transaction() as tx: new_id = tx.mutate(f"INSERT INTO {self._table} DEFAULT VALUES") @@ -696,7 +716,7 @@ class Model(ABC, Generic[D]): self, included_keys: str = _formatter.ALL_KEYS, for_path: bool = False, - ): + ) -> FormattedMapping: """Get a mapping containing all values on this object formatted as human-readable unicode strings. """ @@ -740,9 +760,9 @@ class Model(ABC, Generic[D]): Remove the database connection as sqlite connections are not picklable. """ - state = self.__dict__.copy() - state["_db"] = None - return state + return { + k: v for k, v in self.__dict__.items() if k not in {"_db", "db"} + } # Database controller and supporting interfaces. @@ -1303,12 +1323,6 @@ class Database: sort if sort.is_slow() else None, # Slow sort component. ) - def _get( - self, - model_cls: type[AnyModel], - id, - ) -> AnyModel | None: - """Get a Model object by its id or None if the id does not - exist. - """ - return self._fetch(model_cls, MatchQuery("id", id)).get() + def _get(self, model_cls: type[AnyModel], id_: int) -> AnyModel | None: + """Get a Model object by its id or None if the id does not exist.""" + return self._fetch(model_cls, MatchQuery("id", id_)).get() diff --git a/beets/library/library.py b/beets/library/library.py index 7370f7ecd..39d559901 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -125,24 +125,20 @@ class Library(dbcore.Database): return self._fetch(Item, query, sort or self.get_default_item_sort()) # Convenience accessors. - - def get_item(self, id): + def get_item(self, id_: int) -> Item | None: """Fetch a :class:`Item` by its ID. Return `None` if no match is found. """ - return self._get(Item, id) + return self._get(Item, id_) - def get_album(self, item_or_id): + def get_album(self, item_or_id: Item | int) -> Album | None: """Given an album ID or an item associated with an album, return a :class:`Album` object for the album. If no such album exists, return `None`. """ - if isinstance(item_or_id, int): - album_id = item_or_id - else: - album_id = item_or_id.album_id - if album_id is None: - return None - return self._get(Album, album_id) + album_id = ( + item_or_id if isinstance(item_or_id, int) else item_or_id.album_id + ) + return self._get(Album, album_id) if album_id else None diff --git a/beets/library/models.py b/beets/library/models.py index cbee2a411..9609989bc 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -620,6 +620,8 @@ class Album(LibModel): class Item(LibModel): """Represent a song or track.""" + album_id: int | None + _table = "items" _flex_table = "item_attributes" _fields = { @@ -1143,7 +1145,6 @@ class Item(LibModel): If `store` is `False` however, the item won't be stored and it will have to be manually stored after invoking this method. """ - self._check_db() dest = self.destination(basedir=basedir) # Create necessary ancestry for the move. @@ -1183,9 +1184,8 @@ class Item(LibModel): is true, returns just the fragment of the path underneath the library base directory. """ - db = self._check_db() - basedir = basedir or db.directory - path_formats = path_formats or db.path_formats + basedir = basedir or self.db.directory + path_formats = path_formats or self.db.path_formats # Use a path format based on a query, falling back on the # default. @@ -1224,7 +1224,7 @@ class Item(LibModel): ) lib_path_str, fallback = util.legalize_path( - subpath, db.replacements, self.filepath.suffix + subpath, self.db.replacements, self.filepath.suffix ) if fallback: # Print an error message if legalization fell back to diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index cfd8b6bd7..5eeef815d 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -43,7 +43,10 @@ from beets.util.deprecation import deprecate_for_maintainers from beets.util.functemplate import template if TYPE_CHECKING: - from collections.abc import Callable + from collections.abc import Callable, Iterable + + from beets.dbcore.db import FormattedMapping + # On Windows platforms, use colorama to support "ANSI" terminal colors. if sys.platform == "win32": @@ -1028,42 +1031,47 @@ def print_newline_layout( FLOAT_EPSILON = 0.01 -def _field_diff(field, old, old_fmt, new, new_fmt): +def _field_diff( + field: str, old: FormattedMapping, new: FormattedMapping +) -> str | None: """Given two Model objects and their formatted views, format their values for `field` and highlight changes among them. Return a human-readable string. If the value has not changed, return None instead. """ - oldval = old.get(field) - newval = new.get(field) - # If no change, abort. - if ( + if (oldval := old.model.get(field)) == (newval := new.model.get(field)) or ( isinstance(oldval, float) and isinstance(newval, float) and abs(oldval - newval) < FLOAT_EPSILON ): return None - elif oldval == newval: - return None # Get formatted values for output. - oldstr = old_fmt.get(field, "") - newstr = new_fmt.get(field, "") + oldstr, newstr = old.get(field, ""), new.get(field, "") + if field not in new: + return colorize("text_diff_removed", f"{field}: {oldstr}") + + if field not in old: + return colorize("text_diff_added", f"{field}: {newstr}") # For strings, highlight changes. For others, colorize the whole # thing. if isinstance(oldval, str): - oldstr, newstr = colordiff(oldval, newstr) + oldstr, newstr = colordiff(oldstr, newstr) else: oldstr = colorize("text_diff_removed", oldstr) newstr = colorize("text_diff_added", newstr) - return f"{oldstr} -> {newstr}" + return f"{field}: {oldstr} -> {newstr}" def show_model_changes( - new, old=None, fields=None, always=False, print_obj: bool = True -): + new: library.LibModel, + old: library.LibModel | None = None, + fields: Iterable[str] | None = None, + always: bool = False, + print_obj: bool = True, +) -> bool: """Given a Model object, print a list of changes from its pristine version stored in the database. Return a boolean indicating whether any changes were found. @@ -1073,7 +1081,7 @@ def show_model_changes( restrict the detection to. `always` indicates whether the object is always identified, regardless of whether any changes are present. """ - old = old or new._db._get(type(new), new.id) + old = old or new.get_fresh_from_db() # Keep the formatted views around instead of re-creating them in each # iteration step @@ -1081,31 +1089,21 @@ def show_model_changes( new_fmt = new.formatted() # Build up lines showing changed fields. - changes = [] - for field in old: - # Subset of the fields. Never show mtime. - if field == "mtime" or (fields and field not in fields): - continue + diff_fields = (set(old) | set(new)) - {"mtime"} + if allowed_fields := set(fields or {}): + diff_fields &= allowed_fields - # Detect and show difference for this field. - line = _field_diff(field, old, old_fmt, new, new_fmt) - if line: - changes.append(f" {field}: {line}") - - # New fields. - for field in set(new) - set(old): - if fields and field not in fields: - continue - - changes.append( - f" {field}: {colorize('text_highlight', new_fmt[field])}" - ) + changes = [ + d + for f in sorted(diff_fields) + if (d := _field_diff(f, old_fmt, new_fmt)) + ] # Print changes. if print_obj and (changes or always): print_(format(old)) if changes: - print_("\n".join(changes)) + print_(textwrap.indent("\n".join(changes), " ")) return bool(changes) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index e6bd05119..9f5ed69fb 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -1101,6 +1101,16 @@ class FileSystem(LocalArtSource): else: remaining.append(fn) + # Fall back to a configured image. + if plugin.fallback: + self._log.debug( + "using fallback art file {}", + util.displayable_path(plugin.fallback), + ) + yield self._candidate( + path=plugin.fallback, match=MetadataMatch.FALLBACK + ) + # Fall back to any image in the folder. if remaining and not plugin.cautious: self._log.debug( @@ -1332,6 +1342,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): "enforce_ratio": False, "cautious": False, "cover_names": ["cover", "front", "art", "album", "folder"], + "fallback": None, "sources": [ "filesystem", "coverart", @@ -1380,6 +1391,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): cover_names = self.config["cover_names"].as_str_seq() self.cover_names = list(map(util.bytestring_path, cover_names)) self.cautious = self.config["cautious"].get(bool) + self.fallback = self.config["fallback"].get( + confuse.Optional(confuse.Filename()) + ) self.store_source = self.config["store_source"].get(bool) self.cover_format = self.config["cover_format"].get( diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index ea0ab951a..e622096cf 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -28,7 +28,7 @@ import os import traceback from functools import singledispatchmethod from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, Callable import pylast import yaml @@ -38,6 +38,8 @@ from beets.library import Album, Item from beets.util import plurality, unique_list if TYPE_CHECKING: + import optparse + from beets.library import LibModel LASTFM = pylast.LastFMNetwork(api_key=plugins.LASTFM_KEY) @@ -52,7 +54,11 @@ PYLAST_EXCEPTIONS = ( # Canonicalization tree processing. -def flatten_tree(elem, path, branches): +def flatten_tree( + elem: dict[Any, Any] | list[Any] | str, + path: list[str], + branches: list[list[str]], +) -> None: """Flatten nested lists/dictionaries into lists of strings (branches). """ @@ -69,7 +75,7 @@ def flatten_tree(elem, path, branches): branches.append(path + [str(elem)]) -def find_parents(candidate, branches): +def find_parents(candidate: str, branches: list[list[str]]) -> list[str]: """Find parents genre of a given genre, ordered from the closest to the further parent. """ @@ -89,7 +95,7 @@ C14N_TREE = os.path.join(os.path.dirname(__file__), "genres-tree.yaml") class LastGenrePlugin(plugins.BeetsPlugin): - def __init__(self): + def __init__(self) -> None: super().__init__() self.config.add( @@ -111,12 +117,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): ) self.setup() - def setup(self): + def setup(self) -> None: """Setup plugin from config options""" if self.config["auto"]: self.import_stages = [self.imported] - self._genre_cache = {} + self._genre_cache: dict[str, list[str]] = {} self.whitelist = self._load_whitelist() self.c14n_branches, self.canonicalize = self._load_c14n_tree() @@ -161,7 +167,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): flatten_tree(genres_tree, [], c14n_branches) return c14n_branches, canonicalize - def _tunelog(self, msg, *args, **kwargs): + def _tunelog(self, msg: str, *args: Any, **kwargs: Any) -> None: """Log tuning messages at DEBUG level when verbosity level is high enough.""" if config["verbose"].as_number() >= 3: self._log.debug(msg, *args, **kwargs) @@ -182,7 +188,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): # More canonicalization and general helpers. - def _get_depth(self, tag): + def _get_depth(self, tag: str) -> int | None: """Find the depth of a tag in the genres tree.""" depth = None for key, value in enumerate(self.c14n_branches): @@ -191,7 +197,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): break return depth - def _sort_by_depth(self, tags): + def _sort_by_depth(self, tags: list[str]) -> list[str]: """Given a list of tags, sort the tags by their depths in the genre tree. """ @@ -259,9 +265,11 @@ class LastGenrePlugin(plugins.BeetsPlugin): valid_tags = [t for t in tags if self._is_valid(t)] return valid_tags[:count] - def fetch_genre(self, lastfm_obj): - """Return the genre for a pylast entity or None if no suitable genre - can be found. Ex. 'Electronic, House, Dance' + def fetch_genre( + self, lastfm_obj: pylast.Album | pylast.Artist | pylast.Track + ) -> list[str]: + """Return genres for a pylast entity. Returns an empty list if + no suitable genres are found. """ min_weight = self.config["min_weight"].get(int) return self._tags_for(lastfm_obj, min_weight) @@ -278,8 +286,10 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Cached last.fm entity lookups. - def _last_lookup(self, entity, method, *args): - """Get a genre based on the named entity using the callable `method` + def _last_lookup( + self, entity: str, method: Callable[..., Any], *args: str + ) -> list[str]: + """Get genres based on the named entity using the callable `method` whose arguments are given in the sequence `args`. The genre lookup is cached based on the entity name and the arguments. @@ -293,31 +303,27 @@ class LastGenrePlugin(plugins.BeetsPlugin): key = f"{entity}.{'-'.join(str(a) for a in args)}" if key not in self._genre_cache: - args = [a.replace("\u2010", "-") for a in args] - self._genre_cache[key] = self.fetch_genre(method(*args)) + args_replaced = [a.replace("\u2010", "-") for a in args] + self._genre_cache[key] = self.fetch_genre(method(*args_replaced)) genre = self._genre_cache[key] self._tunelog("last.fm (unfiltered) {} tags: {}", entity, genre) return genre - def fetch_album_genre(self, obj): - """Return raw album genres from Last.fm for this Item or Album.""" + def fetch_album_genre(self, albumartist: str, albumtitle: str) -> list[str]: + """Return genres from Last.fm for the album by albumartist.""" return self._last_lookup( - "album", LASTFM.get_album, obj.albumartist, obj.album + "album", LASTFM.get_album, albumartist, albumtitle ) - def fetch_album_artist_genre(self, obj): - """Return raw album artist genres from Last.fm for this Item or Album.""" - return self._last_lookup("artist", LASTFM.get_artist, obj.albumartist) + def fetch_artist_genre(self, artist: str) -> list[str]: + """Return genres from Last.fm for the artist.""" + return self._last_lookup("artist", LASTFM.get_artist, artist) - def fetch_artist_genre(self, item): - """Returns raw track artist genres from Last.fm for this Item.""" - return self._last_lookup("artist", LASTFM.get_artist, item.artist) - - def fetch_track_genre(self, obj): - """Returns raw track genres from Last.fm for this Item.""" + def fetch_track_genre(self, trackartist: str, tracktitle: str) -> list[str]: + """Return genres from Last.fm for the track by artist.""" return self._last_lookup( - "track", LASTFM.get_track, obj.artist, obj.title + "track", LASTFM.get_track, trackartist, tracktitle ) # Main processing: _get_genre() and helpers. @@ -372,7 +378,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): and the whitelist feature was disabled. """ - def _try_resolve_stage(stage_label: str, keep_genres, new_genres): + def _try_resolve_stage( + stage_label: str, keep_genres: list[str], new_genres: list[str] + ) -> tuple[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 @@ -405,14 +413,14 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Run through stages: track, album, artist, # album artist, or most popular track genre. if isinstance(obj, library.Item) and "track" in self.sources: - if new_genres := self.fetch_track_genre(obj): + if new_genres := self.fetch_track_genre(obj.artist, obj.title): if result := _try_resolve_stage( "track", keep_genres, new_genres ): return result if "album" in self.sources: - if new_genres := self.fetch_album_genre(obj): + if new_genres := self.fetch_album_genre(obj.albumartist, obj.album): if result := _try_resolve_stage( "album", keep_genres, new_genres ): @@ -421,20 +429,36 @@ class LastGenrePlugin(plugins.BeetsPlugin): if "artist" in self.sources: new_genres = [] if isinstance(obj, library.Item): - new_genres = self.fetch_artist_genre(obj) + new_genres = self.fetch_artist_genre(obj.artist) stage_label = "artist" elif obj.albumartist != config["va_name"].as_str(): - new_genres = self.fetch_album_artist_genre(obj) + new_genres = self.fetch_artist_genre(obj.albumartist) stage_label = "album artist" + if not new_genres: + self._tunelog( + 'No album artist genre found for "{}", ' + "trying multi-valued field...", + obj.albumartist, + ) + for albumartist in obj.albumartists: + self._tunelog( + 'Fetching artist genre for "{}"', albumartist + ) + new_genres += self.fetch_artist_genre(albumartist) + if new_genres: + stage_label = "multi-valued album artist" else: # For "Various Artists", pick the most popular track genre. item_genres = [] + assert isinstance(obj, Album) # Type narrowing for mypy for item in obj.items(): item_genre = None if "track" in self.sources: - item_genre = self.fetch_track_genre(item) + item_genre = self.fetch_track_genre( + item.artist, item.title + ) if not item_genre: - item_genre = self.fetch_artist_genre(item) + item_genre = self.fetch_artist_genre(item.artist) if item_genre: item_genres += item_genre if item_genres: @@ -500,7 +524,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): write=write, move=False, inherit="track" not in self.sources ) - def commands(self): + def commands(self) -> list[ui.Subcommand]: lastgenre_cmd = ui.Subcommand("lastgenre", help="fetch genres") lastgenre_cmd.parser.add_option( "-p", @@ -559,7 +583,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): ) lastgenre_cmd.parser.set_defaults(album=True) - def lastgenre_func(lib, opts, args): + def lastgenre_func( + lib: library.Library, opts: optparse.Values, args: list[str] + ) -> None: self.config.set_args(opts) method = lib.albums if opts.album else lib.items @@ -569,10 +595,16 @@ class LastGenrePlugin(plugins.BeetsPlugin): lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] - def imported(self, session, task): + def imported( + self, session: library.Session, task: library.ImportTask + ) -> None: self._process(task.album if task.is_album else task.item, write=False) - def _tags_for(self, obj, min_weight=None): + def _tags_for( + self, + obj: pylast.Album | pylast.Artist | pylast.Track, + min_weight: int | None = None, + ) -> list[str]: """Core genre identification routine. Given a pylast entity (album or track), return a list of @@ -584,11 +616,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Work around an inconsistency in pylast where # Album.get_top_tags() does not return TopItem instances. # https://github.com/pylast/pylast/issues/86 + obj_to_query: Any = obj if isinstance(obj, pylast.Album): - obj = super(pylast.Album, obj) + obj_to_query = super(pylast.Album, obj) try: - res = obj.get_top_tags() + res: Any = obj_to_query.get_top_tags() except PYLAST_EXCEPTIONS as exc: self._log.debug("last.fm error: {}", exc) return [] @@ -603,6 +636,6 @@ class LastGenrePlugin(plugins.BeetsPlugin): res = [el for el in res if (int(el.weight or 0)) >= min_weight] # Get strings from tags. - res = [el.item.get_name().lower() for el in res] + tags: list[str] = [el.item.get_name().lower() for el in res] - return res + return tags diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 221afea71..8cab1786b 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -914,7 +914,7 @@ class MusicBrainzPlugin(MetadataSourcePlugin): rel["type"] == "transl-tracklisting" and rel["direction"] == "backward" ): - actual_res = self.api.get_release(rel["target"]) + actual_res = self.api.get_release(rel["release"]["id"]) # release is potentially a pseudo release release = self.album_info(res) diff --git a/docs/changelog.rst b/docs/changelog.rst index a471b4c56..49402bad7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,7 @@ been dropped. New features: +- :doc:`plugins/fetchart`: Added config setting for a fallback cover art image. - :doc:`plugins/ftintitle`: Added argument for custom feat. words in ftintitle. - :doc:`plugins/ftintitle`: Added album template value ``album_artist_no_feat``. - :doc:`plugins/musicbrainz`: Allow selecting tags or genres to populate the @@ -70,6 +71,13 @@ Bug fixes: - When using :doc:`plugins/fromfilename` together with :doc:`plugins/edit`, temporary tags extracted from filenames are no longer lost when discarding or cancelling an edit session during import. :bug:`6104` +- :ref:`update-cmd` :doc:`plugins/edit` fix display formatting of field changes + to clearly show added and removed flexible fields. +- :doc:`plugins/lastgenre`: Fix the issue where last.fm doesn't return any + result in the artist genre stage because "concatenation" words in the artist + name (like "feat.", "+", or "&") prevent it. Using the albumartists list field + and fetching a genre for each artist separately improves the chance of + receiving valid results in that stage. For plugin developers: diff --git a/docs/plugins/fetchart.rst b/docs/plugins/fetchart.rst index 1d64f4b2e..fd578212a 100644 --- a/docs/plugins/fetchart.rst +++ b/docs/plugins/fetchart.rst @@ -33,6 +33,8 @@ file. The available options are: contain one of the keywords in ``cover_names``. Default: ``no``. - **cover_names**: Prioritize images containing words in this list. Default: ``cover front art album folder``. +- **fallback**: Path to a fallback album art file if no album art was found + otherwise. Default: ``None`` (disabled). - **minwidth**: Only images with a width bigger or equal to ``minwidth`` are considered as valid album art candidates. Default: 0. - **maxwidth**: A maximum image width to downscale fetched images if they are diff --git a/pyproject.toml b/pyproject.toml index 24cf21b33..bc694de90 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -322,6 +322,7 @@ ignore = [ [tool.ruff.lint.per-file-ignores] "beets/**" = ["PT"] "test/test_util.py" = ["E501"] +"test/ui/test_field_diff.py" = ["E501"] [tool.ruff.lint.isort] split-on-trailing-comma = false diff --git a/test/autotag/test_distance.py b/test/autotag/test_distance.py index 9a658f5e1..3686f82c9 100644 --- a/test/autotag/test_distance.py +++ b/test/autotag/test_distance.py @@ -12,15 +12,13 @@ from beets.autotag.distance import ( from beets.library import Item from beets.metadata_plugins import MetadataSourcePlugin, get_penalty from beets.plugins import BeetsPlugin -from beets.test.helper import ConfigMixin _p = pytest.param class TestDistance: @pytest.fixture(autouse=True, scope="class") - def setup_config(self): - config = ConfigMixin().config + def setup_config(self, config): config["match"]["distance_weights"]["data_source"] = 2.0 config["match"]["distance_weights"]["album"] = 4.0 config["match"]["distance_weights"]["medium"] = 2.0 diff --git a/test/conftest.py b/test/conftest.py index eb46b94b0..059526d2f 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -5,6 +5,7 @@ import pytest from beets.autotag.distance import Distance from beets.dbcore.query import Query +from beets.test.helper import ConfigMixin from beets.util import cached_classproperty @@ -53,3 +54,9 @@ def pytest_assertrepr_compare(op, left, right): @pytest.fixture(autouse=True) def clear_cached_classproperty(): cached_classproperty.cache.clear() + + +@pytest.fixture(scope="module") +def config(): + """Provide a fresh beets configuration for a module, when requested.""" + return ConfigMixin().config diff --git a/test/plugins/test_art.py b/test/plugins/test_art.py index 285bb70e5..02d23d59b 100644 --- a/test/plugins/test_art.py +++ b/test/plugins/test_art.py @@ -261,7 +261,9 @@ class FSArtTest(UseThePlugin): os.mkdir(syspath(self.dpath)) self.source = fetchart.FileSystem(logger, self.plugin.config) - self.settings = Settings(cautious=False, cover_names=("art",)) + self.settings = Settings( + cautious=False, cover_names=("art",), fallback=None + ) def test_finds_jpg_in_directory(self): _common.touch(os.path.join(self.dpath, b"a.jpg")) @@ -285,6 +287,13 @@ class FSArtTest(UseThePlugin): with pytest.raises(StopIteration): next(self.source.get(None, self.settings, [self.dpath])) + def test_configured_fallback_is_used(self): + fallback = os.path.join(self.temp_dir, b"a.jpg") + _common.touch(fallback) + self.settings.fallback = fallback + candidate = next(self.source.get(None, self.settings, [self.dpath])) + assert candidate.path == fallback + def test_empty_dir(self): with pytest.raises(StopIteration): next(self.source.get(None, self.settings, [self.dpath])) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 12ff30f8e..026001e38 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -546,13 +546,13 @@ class LastGenrePluginTest(PluginTestCase): def test_get_genre(config_values, item_genre, mock_genres, expected_result): """Test _get_genre with various configurations.""" - def mock_fetch_track_genre(self, obj=None): + def mock_fetch_track_genre(self, trackartist, tracktitle): return mock_genres["track"] - def mock_fetch_album_genre(self, obj): + def mock_fetch_album_genre(self, albumartist, albumtitle): return mock_genres["album"] - def mock_fetch_artist_genre(self, obj): + def mock_fetch_artist_genre(self, artist): return mock_genres["artist"] # Mock the last.fm fetchers. When whitelist enabled, we can assume only diff --git a/test/plugins/test_mbpseudo.py b/test/plugins/test_mbpseudo.py index b333800a3..a98a59248 100644 --- a/test/plugins/test_mbpseudo.py +++ b/test/plugins/test_mbpseudo.py @@ -8,7 +8,7 @@ from beets.autotag import AlbumMatch from beets.autotag.distance import Distance from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.library import Item -from beets.test.helper import ConfigMixin, PluginMixin +from beets.test.helper import PluginMixin from beetsplug._typing import JSONDict from beetsplug.mbpseudo import ( _STATUS_PSEUDO, @@ -52,14 +52,7 @@ def pseudo_release_info() -> AlbumInfo: ) -@pytest.fixture(scope="module", autouse=True) -def config(): - config = ConfigMixin().config - with pytest.MonkeyPatch.context() as m: - m.setattr("beetsplug.mbpseudo.config", config) - yield config - - +@pytest.mark.usefixtures("config") class TestPseudoAlbumInfo: def test_album_id_always_from_pseudo( self, official_release_info: AlbumInfo, pseudo_release_info: AlbumInfo diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 0a3155430..30b9f7d1a 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -204,7 +204,6 @@ class MBAlbumInfoTest(MusicBrainzTestCase): { "type": "remixer", "type-id": "RELATION TYPE ID", - "target": "RECORDING REMIXER ARTIST ID", "direction": "RECORDING RELATION DIRECTION", "artist": { "id": "RECORDING REMIXER ARTIST ID", @@ -820,8 +819,10 @@ class MBLibraryTest(MusicBrainzTestCase): "release-relations": [ { "type": "transl-tracklisting", - "target": "d2a6f856-b553-40a0-ac54-a321e8e2da01", "direction": "backward", + "release": { + "id": "d2a6f856-b553-40a0-ac54-a321e8e2da01" + }, } ], }, @@ -993,8 +994,10 @@ class MBLibraryTest(MusicBrainzTestCase): "release-relations": [ { "type": "remaster", - "target": "d2a6f856-b553-40a0-ac54-a321e8e2da01", "direction": "backward", + "release": { + "id": "d2a6f856-b553-40a0-ac54-a321e8e2da01" + }, } ], } diff --git a/test/test_autotag.py b/test/test_autotag.py index 48ae09ccb..119ca15e8 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -19,18 +19,18 @@ import pytest from beets import autotag, config from beets.autotag import AlbumInfo, TrackInfo, correct_list_fields, match from beets.library import Item -from beets.test.helper import BeetsTestCase, ConfigMixin +from beets.test.helper import BeetsTestCase -class TestAssignment(ConfigMixin): +class TestAssignment: A = "one" B = "two" C = "three" @pytest.fixture(autouse=True) - def _setup_config(self): - self.config["match"]["track_length_grace"] = 10 - self.config["match"]["track_length_max"] = 30 + def config(self, config): + config["match"]["track_length_grace"] = 10 + config["match"]["track_length_max"] = 30 @pytest.mark.parametrize( # 'expected' is a tuple of expected (mapping, extra_items, extra_tracks) diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py new file mode 100644 index 000000000..35f3c6ca7 --- /dev/null +++ b/test/ui/test_field_diff.py @@ -0,0 +1,59 @@ +import pytest + +from beets.library import Item +from beets.ui import _field_diff + +p = pytest.param + + +class TestFieldDiff: + @pytest.fixture(autouse=True) + def configure_color(self, config, color): + config["ui"]["color"] = color + + @pytest.fixture(autouse=True) + def patch_colorize(self, monkeypatch): + """Patch to return a deterministic string format instead of ANSI codes.""" + monkeypatch.setattr( + "beets.ui.colorize", + lambda color_name, text: f"[{color_name}]{text}[/]", + ) + + @staticmethod + def diff_fmt(old, new): + return f"[text_diff_removed]{old}[/] -> [text_diff_added]{new}[/]" + + @pytest.mark.parametrize( + "old_data, new_data, field, expected_diff", + [ + p({"title": "foo"}, {"title": "foo"}, "title", None, id="no_change"), + p({"bpm": 120.0}, {"bpm": 120.005}, "bpm", None, id="float_close_enough"), + p({"bpm": 120.0}, {"bpm": 121.0}, "bpm", f"bpm: {diff_fmt('120', '121')}", id="float_changed"), + 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({"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"), + ], + ) # fmt: skip + @pytest.mark.parametrize("color", [True], ids=["color_enabled"]) + def test_field_diff_colors(self, old_data, new_data, field, expected_diff): + old_item = Item(**old_data) + new_item = Item(**new_data) + + diff = _field_diff(field, old_item.formatted(), new_item.formatted()) + + assert diff == expected_diff + + @pytest.mark.parametrize("color", [False], ids=["color_disabled"]) + def test_field_diff_no_color(self): + old_item = Item(title="foo") + new_item = Item(title="bar") + + diff = _field_diff("title", old_item.formatted(), new_item.formatted()) + + assert diff == "title: foo -> bar"