From c7ba399dd1057021f6e7344511705ede6955fa28 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Mon, 15 Sep 2025 23:32:04 +0200 Subject: [PATCH 01/53] fix incorrect matches when album is missing or empty closes #5189 --- beets/metadata_plugins.py | 4 +++- beetsplug/spotify.py | 8 +++++++- docs/changelog.rst | 5 +++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/beets/metadata_plugins.py b/beets/metadata_plugins.py index 381881b51..6a75d0f79 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -371,7 +371,9 @@ class SearchApiMetadataSourcePlugin( album: str, va_likely: bool, ) -> Iterable[AlbumInfo]: - query_filters: SearchFilter = {"album": album} + query_filters: SearchFilter = {} + if album: + query_filters["album"] = album if not va_likely: query_filters["artist"] = artist diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index a0a5c4358..e97484bc1 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -569,7 +569,13 @@ class SpotifyPlugin( query_string = item[self.config["track_field"].get()] # Query the Web API for each track, look for the items' JSON data - query_filters: SearchFilter = {"artist": artist, "album": album} + + query_filters: SearchFilter = {} + if artist: + query_filters["artist"] = artist + if album: + query_filters["album"] = album + response_data_tracks = self._search_api( query_type="track", query_string=query_string, diff --git a/docs/changelog.rst b/docs/changelog.rst index 67c284a88..f4ac99d27 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,11 @@ New features: Bug fixes: +- :doc:`plugins/spotify` Fixed an issue where track matching and lookups could + return incorrect or misleading results when using the Spotify plugin. The + problem occurred primarily when no album was provided or when the album field + was an empty string. :bug:`5189` + For packagers: Other changes: From efbfc23931748773675dd2939aa1be4861918c85 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Wed, 17 Sep 2025 12:47:22 +0200 Subject: [PATCH 02/53] Removed config options and fixed a bug with `beet spotify command` --- beetsplug/spotify.py | 11 +++-------- docs/changelog.rst | 3 +++ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py index e97484bc1..0f6e0012b 100644 --- a/beetsplug/spotify.py +++ b/beetsplug/spotify.py @@ -130,9 +130,6 @@ class SpotifyPlugin( "mode": "list", "tiebreak": "popularity", "show_failures": False, - "artist_field": "albumartist", - "album_field": "album", - "track_field": "title", "region_filter": None, "regex": [], "client_id": "4e414367a1d14c75a5c5129a627fcab8", @@ -563,13 +560,11 @@ class SpotifyPlugin( regex["search"], regex["replace"], value ) - # Custom values can be passed in the config (just in case) - artist = item[self.config["artist_field"].get()] - album = item[self.config["album_field"].get()] - query_string = item[self.config["track_field"].get()] + artist = item["artist"] or item["albumartist"] + album = item["album"] + query_string = item["title"] # Query the Web API for each track, look for the items' JSON data - query_filters: SearchFilter = {} if artist: query_filters["artist"] = artist diff --git a/docs/changelog.rst b/docs/changelog.rst index f4ac99d27..8a35569c9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,9 @@ Bug fixes: return incorrect or misleading results when using the Spotify plugin. The problem occurred primarily when no album was provided or when the album field was an empty string. :bug:`5189` +- :doc:`plugins/spotify` Removed old and undocumented config options + `artist_field`, `album_field` and `track` that were causing issues with track + matching. :bug:`5189` For packagers: From b0caac871a36107ce8119e49bcfde32168edaa67 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Fri, 9 Aug 2024 18:36:11 -0400 Subject: [PATCH 03/53] fix: enable tracebacks for "user"/custom sqlite functions A bit niche but I tried setting my bareasc prefix to an empty string, and was getting an obtuse error. This should help make clearer what is happening when queries fail. The exception is not properly raised up the stack in the first place because it happens across 2 FFI boundaries: the DB query (Python -> SQLite), and the custom DB function (SQLite -> Python). Thus Python cannot forwarded it back to itself through SQLite, and it's treated as an "unraisable" exception. We could override `sys.unraisablehook` to not print anything for the original exception, and store it in a global for the outer Python interpreter to fetch and raise properly, but that's pretty hacky, limited to a single DB instance and query at once, and risks swallowing other "unraisable" exceptions. Instead we just tell the user to look above for what Python prints. Sample output: ``` Exception ignored in: Traceback (most recent call last): File "site-packages/unidecode/__init__.py", line 60, in unidecode_expect_ascii bytestring = string.encode('ASCII') ^^^^^^^^^^^^^ AttributeError: 'bytes' object has no attribute 'encode' Traceback (most recent call last): File "site-packages/beets/dbcore/db.py", line 988, in query cursor = self.db._connection().execute(statement, subvals) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sqlite3.OperationalError: user-defined function raised exception During handling of the above exception, another exception occurred: Traceback (most recent call last): File "site-packages/beets/__main__.py", line 9, in sys.exit(main()) ^^^^^^ File "site-packages/beets/ui/__init__.py", line 1865, in main _raw_main(args) File "site-packages/beets/ui/__init__.py", line 1852, in _raw_main subcommand.func(lib, suboptions, subargs) File "site-packages/beets/ui/commands.py", line 1599, in list_func list_items(lib, decargs(args), opts.album) File "site-packages/beets/ui/commands.py", line 1594, in list_items for item in lib.items(query): ^^^^^^^^^^^^^^^^ File "site-packages/beets/library.py", line 1695, in items return self._fetch(Item, query, sort or self.get_default_item_sort()) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/beets/library.py", line 1673, in _fetch return super()._fetch(model_cls, query, sort) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/beets/dbcore/db.py", line 1301, in _fetch rows = tx.query(sql, subvals) ^^^^^^^^^^^^^^^^^^^^^^ File "site-packages/beets/dbcore/db.py", line 991, in query raise DBCustomFunctionError() beets.dbcore.db.DBCustomFunctionError: beets defined SQLite function failed; see the other errors above for details ``` --- beets/dbcore/db.py | 23 +++++++++++++++++++++++ test/test_dbcore.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 8cd89111e..b3a6c7dd8 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -64,6 +64,16 @@ class DBAccessError(Exception): """ +class DBCustomFunctionError(Exception): + """A sqlite function registered by beets failed.""" + + def __init__(self): + super().__init__( + "beets defined SQLite function failed; " + "see the other errors above for details" + ) + + class FormattedMapping(Mapping[str, str]): """A `dict`-like formatted view of a model. @@ -947,6 +957,12 @@ class Transaction: self._mutated = False self.db._db_lock.release() + if ( + isinstance(exc_value, sqlite3.OperationalError) + and exc_value.args[0] == "user-defined function raised exception" + ): + raise DBCustomFunctionError() + def query( self, statement: str, subvals: Sequence[SQLiteType] = () ) -> list[sqlite3.Row]: @@ -1007,6 +1023,13 @@ class Database: "sqlite3 must be compiled with multi-threading support" ) + # Print tracebacks for exceptions in user defined functions + # See also `self.add_functions` and `DBCustomFunctionError`. + # + # `if`: use feature detection because PyPy doesn't support this. + if hasattr(sqlite3, "enable_callback_tracebacks"): + sqlite3.enable_callback_tracebacks(True) + self.path = path self.timeout = timeout diff --git a/test/test_dbcore.py b/test/test_dbcore.py index b2ec2e968..d2c76d852 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -23,6 +23,7 @@ from tempfile import mkstemp import pytest from beets import dbcore +from beets.dbcore.db import DBCustomFunctionError from beets.library import LibModel from beets.test import _common from beets.util import cached_classproperty @@ -31,6 +32,13 @@ from beets.util import cached_classproperty # have multiple models with different numbers of fields. +@pytest.fixture +def db(model): + db = model(":memory:") + yield db + db._connection().close() + + class SortFixture(dbcore.query.FieldSort): pass @@ -784,3 +792,25 @@ class ResultsIteratorTest(unittest.TestCase): self.db._fetch(ModelFixture1, dbcore.query.FalseQuery()).get() is None ) + + +class TestException: + @pytest.mark.parametrize("model", [DatabaseFixture1]) + @pytest.mark.filterwarnings( + "ignore: .*plz_raise.*: pytest.PytestUnraisableExceptionWarning" + ) + @pytest.mark.filterwarnings( + "error: .*: pytest.PytestUnraisableExceptionWarning" + ) + def test_custom_function_error(self, db: DatabaseFixture1): + def plz_raise(): + raise Exception("i haz raized") + + db._connection().create_function("plz_raise", 0, plz_raise) + + with db.transaction() as tx: + tx.mutate("insert into test (field_one) values (1)") + + with pytest.raises(DBCustomFunctionError): + with db.transaction() as tx: + tx.query("select * from test where plz_raise()") From e7e22ebb3d2bd71b0d60fdd3ed1947fbaa6d389d Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Fri, 9 Aug 2024 19:33:43 -0400 Subject: [PATCH 04/53] feat: mark SQLite custom functions as deterministic to allow caching --- beets/dbcore/db.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index b3a6c7dd8..192cfac70 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -17,15 +17,17 @@ from __future__ import annotations import contextlib +import functools import os import re import sqlite3 +import sys import threading import time from abc import ABC from collections import defaultdict from collections.abc import Generator, Iterable, Iterator, Mapping, Sequence -from sqlite3 import Connection +from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, Callable, Generic from typing_extensions import TypeVar # default value support @@ -1125,9 +1127,16 @@ class Database: return bytestring - conn.create_function("regexp", 2, regexp) - conn.create_function("unidecode", 1, unidecode) - conn.create_function("bytelower", 1, bytelower) + create_function = conn.create_function + if sys.version_info >= (3, 8) and sqlite_version_info >= (3, 8, 3): + # Let sqlite make extra optimizations + create_function = functools.partial( + conn.create_function, deterministic=True + ) + + create_function("regexp", 2, regexp) + create_function("unidecode", 1, unidecode) + create_function("bytelower", 1, bytelower) def _close(self): """Close the all connections to the underlying SQLite database From eb83058b13a1ddb8d78f9794acaacce0eb2f1f38 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Sat, 9 Aug 2025 09:38:12 -0400 Subject: [PATCH 05/53] style: remove extraneous `pass` statements --- test/test_dbcore.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/test_dbcore.py b/test/test_dbcore.py index d2c76d852..653adf298 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -89,7 +89,6 @@ class ModelFixture1(LibModel): class DatabaseFixture1(dbcore.Database): _models = (ModelFixture1,) - pass class ModelFixture2(ModelFixture1): @@ -102,7 +101,6 @@ class ModelFixture2(ModelFixture1): class DatabaseFixture2(dbcore.Database): _models = (ModelFixture2,) - pass class ModelFixture3(ModelFixture1): @@ -116,7 +114,6 @@ class ModelFixture3(ModelFixture1): class DatabaseFixture3(dbcore.Database): _models = (ModelFixture3,) - pass class ModelFixture4(ModelFixture1): @@ -131,7 +128,6 @@ class ModelFixture4(ModelFixture1): class DatabaseFixture4(dbcore.Database): _models = (ModelFixture4,) - pass class AnotherModelFixture(ModelFixture1): @@ -153,12 +149,10 @@ class ModelFixture5(ModelFixture1): class DatabaseFixture5(dbcore.Database): _models = (ModelFixture5,) - pass class DatabaseFixtureTwoModels(dbcore.Database): _models = (ModelFixture2, AnotherModelFixture) - pass class ModelFixtureWithGetters(dbcore.Model): From 23e46315e3a957f2c9ca969535e9455780045ccc Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Sat, 20 Sep 2025 01:52:53 +0200 Subject: [PATCH 06/53] Remove Discogs Disambiguation stripping from metadata_plugins --- beets/metadata_plugins.py | 2 -- docs/changelog.rst | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/beets/metadata_plugins.py b/beets/metadata_plugins.py index 6a75d0f79..0372790af 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -298,8 +298,6 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): if not artist_id: artist_id = artist[id_key] name = artist[name_key] - # Strip disambiguation number. - name = re.sub(r" \(\d+\)$", "", name) # Move articles to the front. name = re.sub(r"^(.*?), (a|an|the)$", r"\2 \1", name, flags=re.I) # Use a join keyword if requested and available. diff --git a/docs/changelog.rst b/docs/changelog.rst index 8a35569c9..4ff7b7088 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,7 +22,7 @@ Bug fixes: For packagers: Other changes: - +- :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping - :doc:`plugins/index`: Clarify that musicbrainz must be mentioned if plugin list modified :bug:`6020` - :doc:`/faq`: Add check for musicbrainz plugin if auto-tagger can't find a From 24fbc566f61d86d1fb2e6c1fcde98630b82e7828 Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Sat, 20 Sep 2025 01:58:56 +0200 Subject: [PATCH 07/53] initial changes, changelog adjusted, TODO: test for various artists and update docs --- beetsplug/discogs.py | 9 +++++++ docs/changelog.rst | 6 ++++- test/plugins/test_discogs.py | 49 ++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index c1c782f3e..d8f1d6ba5 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -76,6 +76,8 @@ TRACK_INDEX_RE = re.compile( re.VERBOSE, ) +DISAMBIGUATION_RE = re.compile(r" \(\d+\)$") + class ReleaseFormat(TypedDict): name: str @@ -96,6 +98,7 @@ class DiscogsPlugin(MetadataSourcePlugin): "separator": ", ", "index_tracks": False, "append_style_genre": False, + "strip_disambiguation": True, } ) self.config["apikey"].redact = True @@ -373,6 +376,12 @@ class DiscogsPlugin(MetadataSourcePlugin): artist = config["va_name"].as_str() if catalogno == "none": catalogno = None + + # Remove Discogs specific artist disambiguation 'Artist (2)' or 'Label (3)' + if self.config["strip_disambiguation"]: + artist = DISAMBIGUATION_RE.sub("", artist) + if label is not None: + label = DISAMBIGUATION_RE.sub("", label) # Explicitly set the `media` for the tracks, since it is expected by # `autotag.apply_metadata`, and set `medium_total`. for track in tracks: diff --git a/docs/changelog.rst b/docs/changelog.rst index 4ff7b7088..0814686f8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,17 +18,21 @@ Bug fixes: - :doc:`plugins/spotify` Removed old and undocumented config options `artist_field`, `album_field` and `track` that were causing issues with track matching. :bug:`5189` +- :doc:`plugins/discogs` Added config option `strip_disambiguation` to allow choice of removing discogs numeric disambiguation :bug:`5366` +- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels :bug:`5366` For packagers: Other changes: -- :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping + - :doc:`plugins/index`: Clarify that musicbrainz must be mentioned if plugin list modified :bug:`6020` - :doc:`/faq`: Add check for musicbrainz plugin if auto-tagger can't find a match :bug:`6020` - :doc:`guides/tagger`: Section on no matching release found, related to possibly disabled musicbrainz plugin :bug:`6020` +- :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific + disambiguation stripping 2.4.0 (September 13, 2025) -------------------------- diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index e3e51042c..c279ff128 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -374,6 +374,55 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.genre == "GENRE1, GENRE2" assert d.style is None + def test_strip_disambiguation_label_artist(self): + """Test removing discogs disambiguation""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [self._make_track("A", "1", "01:01")], + "artists": [{"name": "ARTIST NAME (2)", "id": 321, "join": ""}], + "title": "TITLE", + "labels": [ + { + "name": "LABEL NAME (5)", + "catno": "CATALOG NUMBER", + } + ], + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.artist == "ARTIST NAME" + assert d.label == "LABEL NAME" + + def test_strip_disambiguation_off_label_artist(self): + """Test not removing discogs disambiguation""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [self._make_track("A", "1", "01:01")], + "artists": [{"name": "ARTIST NAME (2)", "id": 321, "join": ""}], + "title": "TITLE", + "labels": [ + { + "name": "LABEL NAME (5)", + "catno": "CATALOG NUMBER", + } + ], + } + config["discogs"]["strip_disambiguation"] = False + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.artist == "ARTIST NAME (2)" + assert d.label == "LABEL NAME (5)" + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", From dda265dc77a9765274a2f484047123a06761d6a2 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 19 Sep 2025 20:46:07 -0700 Subject: [PATCH 08/53] Disambiguation fix implemented & tested --- beets/metadata_plugins.py | 7 ++-- beetsplug/discogs.py | 26 +++++++++------ docs/changelog.rst | 8 +++-- docs/plugins/discogs.rst | 3 ++ test/plugins/test_discogs.py | 63 ++++++++++++++++++++++++++++++++---- 5 files changed, 86 insertions(+), 21 deletions(-) diff --git a/beets/metadata_plugins.py b/beets/metadata_plugins.py index 0372790af..0b8d81c95 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -271,10 +271,9 @@ class MetadataSourcePlugin(BeetsPlugin, metaclass=abc.ABCMeta): """Returns an artist string (all artists) and an artist_id (the main artist) for a list of artist object dicts. - For each artist, this function moves articles (such as 'a', 'an', - and 'the') to the front and strips trailing disambiguation numbers. It - returns a tuple containing the comma-separated string of all - normalized artists and the ``id`` of the main/first artist. + For each artist, this function moves articles (such as 'a', 'an', and 'the') + to the front. It returns a tuple containing the comma-separated string + of all normalized artists and the ``id`` of the main/first artist. Alternatively a keyword can be used to combine artists together into a single string by passing the join_key argument. diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d8f1d6ba5..0ffbb0e3e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -76,7 +76,7 @@ TRACK_INDEX_RE = re.compile( re.VERBOSE, ) -DISAMBIGUATION_RE = re.compile(r" \(\d+\)$") +DISAMBIGUATION_RE = re.compile(r" \(\d+\)") class ReleaseFormat(TypedDict): @@ -365,23 +365,22 @@ class DiscogsPlugin(MetadataSourcePlugin): label = catalogno = labelid = None if result.data.get("labels"): - label = result.data["labels"][0].get("name") + label = self.strip_disambiguation( + result.data["labels"][0].get("name") + ) catalogno = result.data["labels"][0].get("catno") labelid = result.data["labels"][0].get("id") cover_art_url = self.select_cover_art(result) - # Additional cleanups (various artists name, catalog number, media). + # Additional cleanups + # (various artists name, catalog number, media, disambiguation). if va: artist = config["va_name"].as_str() + else: + artist = self.strip_disambiguation(artist) if catalogno == "none": catalogno = None - - # Remove Discogs specific artist disambiguation 'Artist (2)' or 'Label (3)' - if self.config["strip_disambiguation"]: - artist = DISAMBIGUATION_RE.sub("", artist) - if label is not None: - label = DISAMBIGUATION_RE.sub("", label) # Explicitly set the `media` for the tracks, since it is expected by # `autotag.apply_metadata`, and set `medium_total`. for track in tracks: @@ -631,6 +630,14 @@ class DiscogsPlugin(MetadataSourcePlugin): return tracklist + def strip_disambiguation(self, text) -> str: + """Removes discogs specific disambiguations from a string. + Turns 'Label Name (5)' to 'Label Name' or 'Artist (1) & Another Artist (2)' + to 'Artist & Another Artist'. Does nothing if strip_disambiguation is False.""" + if not self.config["strip_disambiguation"]: + return text + return DISAMBIGUATION_RE.sub("", text) + def get_track_info(self, track, index, divisions): """Returns a TrackInfo object for a discogs track.""" title = track["title"] @@ -643,6 +650,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist( track.get("artists", []), join_key="join" ) + artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) return TrackInfo( title=title, diff --git a/docs/changelog.rst b/docs/changelog.rst index 0814686f8..ddafd975d 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -18,8 +18,12 @@ Bug fixes: - :doc:`plugins/spotify` Removed old and undocumented config options `artist_field`, `album_field` and `track` that were causing issues with track matching. :bug:`5189` -- :doc:`plugins/discogs` Added config option `strip_disambiguation` to allow choice of removing discogs numeric disambiguation :bug:`5366` -- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels :bug:`5366` +- :doc:`plugins/discogs` Added config option `strip_disambiguation` to allow + choice of removing discogs numeric disambiguation :bug:`5366` +- :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from + artists but not labels. :bug:`5366` +- :doc:`plugins/discogs` Wrote test coverage for removing disambiguation. + :bug:`5366` For packagers: diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 44c0c0e22..3dac558bd 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -109,6 +109,9 @@ Other configurations available under ``discogs:`` are: - **search_limit**: The maximum number of results to return from Discogs. This is useful if you want to limit the number of results returned to speed up searches. Default: ``5`` +- **strip_disambiguation**: Discogs uses strings like ``"(4)"`` to mark distinct + artists and labels with the same name. If you'd like to use the discogs + disambiguation in your tags, you can disable it. Default: ``True`` .. _discogs guidelines: https://support.discogs.com/hc/en-us/articles/360005055373-Database-Guidelines-12-Tracklisting#Index_Tracks_And_Headings diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index c279ff128..5fe73dcac 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -375,7 +375,7 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.style is None def test_strip_disambiguation_label_artist(self): - """Test removing discogs disambiguation""" + """Test removing discogs disambiguation from artist and label""" data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", @@ -399,21 +399,21 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.label == "LABEL NAME" def test_strip_disambiguation_off_label_artist(self): - """Test not removing discogs disambiguation""" + """Test not removing discogs disambiguation from artist and label""" + config["discogs"]["strip_disambiguation"] = False data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [self._make_track("A", "1", "01:01")], + "tracklist": [self._make_track("a", "1", "01:01")], "artists": [{"name": "ARTIST NAME (2)", "id": 321, "join": ""}], - "title": "TITLE", + "title": "title", "labels": [ { "name": "LABEL NAME (5)", - "catno": "CATALOG NUMBER", + "catno": "catalog number", } ], } - config["discogs"]["strip_disambiguation"] = False release = Bag( data=data, title=data["title"], @@ -423,6 +423,57 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.artist == "ARTIST NAME (2)" assert d.label == "LABEL NAME (5)" + def test_strip_disambiguation_multiple_artists(self): + """Test removing disambiguation if there are multiple artists on the release""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [self._make_track("a", "1", "01:01")], + "artists": [ + {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, + {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.artist == "ARTIST NAME & OTHER ARTIST" + + def test_strip_disambiguation_artist_tracks(self): + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + { + "name": "TEST ARTIST (5)", + "tracks": "", + "id": 11146, + } + ], + } + ], + "artists": [{"name": "OTHER ARTIST (5)", "id": 321, "join": ""}], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + d = DiscogsPlugin().get_album_info(release) + assert d.tracks[0].artist == "TEST ARTIST" + assert d.artist == "OTHER ARTIST" + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", From 8a21bacd1455d06d571fd1251cd4a0513067b0e8 Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 19 Sep 2025 21:00:37 -0700 Subject: [PATCH 09/53] Add type check --- beetsplug/discogs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0ffbb0e3e..f22ea2274 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -630,7 +630,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return tracklist - def strip_disambiguation(self, text) -> str: + def strip_disambiguation(self, text: str) -> str: """Removes discogs specific disambiguations from a string. Turns 'Label Name (5)' to 'Label Name' or 'Artist (1) & Another Artist (2)' to 'Artist & Another Artist'. Does nothing if strip_disambiguation is False.""" From 28aee0fde463f1e18dfdba1994e2bdb80833722f Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Mon, 15 Sep 2025 23:56:43 +0200 Subject: [PATCH 10/53] Moved arts.py file into beetsplug namespace as it is not used in core. --- {beets => beetsplug/_utils}/art.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {beets => beetsplug/_utils}/art.py (100%) diff --git a/beets/art.py b/beetsplug/_utils/art.py similarity index 100% rename from beets/art.py rename to beetsplug/_utils/art.py From a796d6d7999b6e579932b96e489cabc609d35638 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Mon, 15 Sep 2025 23:57:15 +0200 Subject: [PATCH 11/53] New import location for art.py --- beetsplug/_utils/__init__.py | 0 beetsplug/convert.py | 3 ++- beetsplug/embedart.py | 3 ++- test/plugins/test_embedart.py | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 beetsplug/_utils/__init__.py diff --git a/beetsplug/_utils/__init__.py b/beetsplug/_utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/beetsplug/convert.py b/beetsplug/convert.py index e9db3592e..102976dd7 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -25,12 +25,13 @@ from string import Template import mediafile from confuse import ConfigTypeError, Optional -from beets import art, config, plugins, ui, util +from beets import config, plugins, ui, util from beets.library import Item, parse_query_string from beets.plugins import BeetsPlugin from beets.util import par_map from beets.util.artresizer import ArtResizer from beets.util.m3u import M3UFile +from beetsplug._utils import art _fs_lock = threading.Lock() _temp_files = [] # Keep track of temporary transcoded files for deletion. diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 1a59e4f9c..cbf40f570 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -20,11 +20,12 @@ from mimetypes import guess_extension import requests -from beets import art, config, ui +from beets import config, ui from beets.plugins import BeetsPlugin from beets.ui import print_ from beets.util import bytestring_path, displayable_path, normpath, syspath from beets.util.artresizer import ArtResizer +from beetsplug._utils import art def _confirm(objs, album): diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 58d2a5f63..734183d3b 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -23,7 +23,7 @@ from unittest.mock import MagicMock, patch import pytest from mediafile import MediaFile -from beets import art, config, logging, ui +from beets import config, logging, ui from beets.test import _common from beets.test.helper import ( BeetsTestCase, @@ -33,6 +33,7 @@ from beets.test.helper import ( ) from beets.util import bytestring_path, displayable_path, syspath from beets.util.artresizer import ArtResizer +from beetsplug._utils import art from test.test_art_resize import DummyIMBackend From 4ab1bb4df463ef54a066e293661cd8db3c91995f Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 16 Sep 2025 00:03:16 +0200 Subject: [PATCH 12/53] Added changelog and git blame ignore rev --- .git-blame-ignore-revs | 4 +++- docs/changelog.rst | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index fbe32b497..ed86e3f8c 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -70,4 +70,6 @@ d93ddf8dd43e4f9ed072a03829e287c78d2570a2 # Moved dev docs 07549ed896d9649562d40b75cd30702e6fa6e975 # Moved plugin docs Further Reading chapter -33f1a5d0bef8ca08be79ee7a0d02a018d502680d \ No newline at end of file +33f1a5d0bef8ca08be79ee7a0d02a018d502680d +# Moved art.py utility module from beets into beetsplug +a7b69e50108eebef8ce92c015f18a42f8bf7276f \ No newline at end of file diff --git a/docs/changelog.rst b/docs/changelog.rst index 8a35569c9..674f3ff28 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,9 @@ Other changes: match :bug:`6020` - :doc:`guides/tagger`: Section on no matching release found, related to possibly disabled musicbrainz plugin :bug:`6020` +- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as it + is not used in the core beets codebase. It can now be found in + ``beetsplug._utils``. 2.4.0 (September 13, 2025) -------------------------- From 73dc8f2bc74c78fd74d7afc844535fc3b9e64da3 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 16 Sep 2025 00:18:04 +0200 Subject: [PATCH 13/53] fix test by changing patch --- .git-blame-ignore-revs | 2 +- docs/changelog.rst | 4 ++-- test/plugins/test_embedart.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index ed86e3f8c..2ee64a97d 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -72,4 +72,4 @@ d93ddf8dd43e4f9ed072a03829e287c78d2570a2 # Moved plugin docs Further Reading chapter 33f1a5d0bef8ca08be79ee7a0d02a018d502680d # Moved art.py utility module from beets into beetsplug -a7b69e50108eebef8ce92c015f18a42f8bf7276f \ No newline at end of file +28aee0fde463f1e18dfdba1994e2bdb80833722f \ No newline at end of file diff --git a/docs/changelog.rst b/docs/changelog.rst index 674f3ff28..ba6a357b7 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,8 +29,8 @@ Other changes: match :bug:`6020` - :doc:`guides/tagger`: Section on no matching release found, related to possibly disabled musicbrainz plugin :bug:`6020` -- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as it - is not used in the core beets codebase. It can now be found in +- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as + it is not used in the core beets codebase. It can now be found in ``beetsplug._utils``. 2.4.0 (September 13, 2025) diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 734183d3b..d40025374 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -284,7 +284,7 @@ class DummyArtResizer(ArtResizer): @patch("beets.util.artresizer.subprocess") -@patch("beets.art.extract") +@patch("beetsplug._utils.art.extract") class ArtSimilarityTest(unittest.TestCase): def setUp(self): self.item = _common.item() From a57ef2cb3b037db43277595aaff974f1120baa15 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 14 Sep 2025 15:36:42 -0400 Subject: [PATCH 14/53] Add --pretend option to lastgenre plugin for previewing genre changes --- beetsplug/lastgenre/__init__.py | 44 +++++++++++++++++++++++---------- docs/changelog.rst | 3 +++ docs/plugins/lastgenre.rst | 4 +++ 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 8c09eefea..bebe15047 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -461,6 +461,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): def commands(self): lastgenre_cmd = ui.Subcommand("lastgenre", help="fetch genres") + lastgenre_cmd.parser.add_option( + "-p", + "--pretend", + action="store_true", + help="show actions but do nothing", + ) lastgenre_cmd.parser.add_option( "-f", "--force", @@ -521,45 +527,57 @@ class LastGenrePlugin(plugins.BeetsPlugin): def lastgenre_func(lib, opts, args): write = ui.should_write() + pretend = getattr(opts, "pretend", False) self.config.set_args(opts) if opts.album: # Fetch genres for whole albums for album in lib.albums(args): - album.genre, src = self._get_genre(album) + album_genre, src = self._get_genre(album) self._log.info( - 'genre for album "{0.album}" ({1}): {0.genre}', + 'genre for album "{0.album}" ({1}): {}', album, src, + album_genre, ) - if "track" in self.sources: - album.store(inherit=False) - else: - album.store() + if not pretend: + album.genre = album_genre + if "track" in self.sources: + album.store(inherit=False) + else: + album.store() for item in album.items(): # If we're using track-level sources, also look up each # track on the album. if "track" in self.sources: - item.genre, src = self._get_genre(item) - item.store() + item_genre, src = self._get_genre(item) self._log.info( - 'genre for track "{0.title}" ({1}): {0.genre}', + 'genre for track "{0.title}" ({1}): {}', item, src, + item_genre, ) + if not pretend: + item.genre = item_genre + item.store() - if write: + if write and not pretend: item.try_write() else: # Just query singletons, i.e. items that are not part of # an album for item in lib.items(args): - item.genre, src = self._get_genre(item) - item.store() + item_genre, src = self._get_genre(item) self._log.info( - "genre for track {0.title} ({1}): {0.genre}", item, src + 'genre for track "{0.title}" ({1}): {}', + item, + src, + item_genre, ) + if not pretend: + item.genre = item_genre + item.store() lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] diff --git a/docs/changelog.rst b/docs/changelog.rst index ba6a357b7..63a8fe339 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,9 @@ Unreleased New features: +- :doc:`plugins/lastgenre`: Add a ``--pretend`` option to preview genre changes + without storing or writing them. + Bug fixes: - :doc:`plugins/spotify` Fixed an issue where track matching and lookups could diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index 5ebe2d721..f4f92f7d1 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -213,5 +213,9 @@ fetch genres for albums or items matching a certain query. By default, ``beet lastgenre`` matches albums. To match individual tracks or singletons, use the ``-A`` switch: ``beet lastgenre -A [QUERY]``. +To preview changes without modifying your library, use the ``-p`` +(``--pretend``) flag. This shows which genres would be set but does not write +or store any changes. + To disable automatic genre fetching on import, set the ``auto`` config option to false. From 84986dc42d315c52ca32b130862fb16d762c0237 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 14 Sep 2025 15:49:34 -0400 Subject: [PATCH 15/53] Enhance lastgenre plugin: add item.try_write() for write operations and improve documentation clarity --- beetsplug/lastgenre/__init__.py | 2 ++ docs/plugins/lastgenre.rst | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index bebe15047..cfecbf0f8 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -578,6 +578,8 @@ class LastGenrePlugin(plugins.BeetsPlugin): if not pretend: item.genre = item_genre item.store() + if write and not pretend: + item.try_write() lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index f4f92f7d1..a33a79230 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -124,7 +124,7 @@ tags** and will only **fetch new genres for empty tags**. When ``force`` is ``yes`` the setting of the ``whitelist`` option (as documented in Usage_) applies to any existing or newly fetched genres. -The follwing configurations are possible: +The following configurations are possible: **Setup 1** (default) @@ -213,9 +213,9 @@ fetch genres for albums or items matching a certain query. By default, ``beet lastgenre`` matches albums. To match individual tracks or singletons, use the ``-A`` switch: ``beet lastgenre -A [QUERY]``. -To preview changes without modifying your library, use the ``-p`` -(``--pretend``) flag. This shows which genres would be set but does not write -or store any changes. +- To preview the changes that would be made without applying them, use the + ``-p`` (``--pretend``) flag. This shows which genres would be set but does + not write or store any changes. To disable automatic genre fetching on import, set the ``auto`` config option to false. From 95b35ded4ab74234f8de470e57217b87d36b920b Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 14 Sep 2025 15:54:12 -0400 Subject: [PATCH 16/53] Lint --- docs/plugins/lastgenre.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index a33a79230..a932585ac 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -213,9 +213,9 @@ fetch genres for albums or items matching a certain query. By default, ``beet lastgenre`` matches albums. To match individual tracks or singletons, use the ``-A`` switch: ``beet lastgenre -A [QUERY]``. -- To preview the changes that would be made without applying them, use the - ``-p`` (``--pretend``) flag. This shows which genres would be set but does - not write or store any changes. +To preview the changes that would be made without applying them, use the +``-p`` or ``--pretend`` flag. This shows which genres would be set but does +not write or store any changes. To disable automatic genre fetching on import, set the ``auto`` config option to false. From 56e132f3527b5abca61dec676bf43e78b5a9a607 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Sun, 14 Sep 2025 16:00:01 -0400 Subject: [PATCH 17/53] more lint --- docs/plugins/lastgenre.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index a932585ac..230694b06 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -213,9 +213,9 @@ fetch genres for albums or items matching a certain query. By default, ``beet lastgenre`` matches albums. To match individual tracks or singletons, use the ``-A`` switch: ``beet lastgenre -A [QUERY]``. -To preview the changes that would be made without applying them, use the -``-p`` or ``--pretend`` flag. This shows which genres would be set but does -not write or store any changes. +To preview the changes that would be made without applying them, use the ``-p`` +or ``--pretend`` flag. This shows which genres would be set but does not write +or store any changes. To disable automatic genre fetching on import, set the ``auto`` config option to false. From 5e6dd674a965c89c4fa9c18f5d99b7d9df9c39b6 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 17 Sep 2025 07:47:04 -0400 Subject: [PATCH 18/53] Update beetsplug/lastgenre/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šarūnas Nejus --- beetsplug/lastgenre/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index cfecbf0f8..fc8510a0a 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -535,7 +535,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): for album in lib.albums(args): album_genre, src = self._get_genre(album) self._log.info( - 'genre for album "{0.album}" ({1}): {}', + 'genre for album "{.album}" ({}): {}', album, src, album_genre, From 0be4cecf820785633feac956ccadca4b8da3f5a8 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 17 Sep 2025 07:47:14 -0400 Subject: [PATCH 19/53] Update beetsplug/lastgenre/__init__.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šarūnas Nejus --- beetsplug/lastgenre/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index fc8510a0a..946b041c3 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -553,7 +553,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): if "track" in self.sources: item_genre, src = self._get_genre(item) self._log.info( - 'genre for track "{0.title}" ({1}): {}', + 'genre for track "{.title}" ({}): {}', item, src, item_genre, From c7ee0e326c3579e6a8207567621b781f2f60a530 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Thu, 18 Sep 2025 07:53:48 -0400 Subject: [PATCH 20/53] Add prefix to log messages for genre fetching in LastGenrePlugin --- beetsplug/lastgenre/__init__.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 946b041c3..1da5ecde4 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -534,8 +534,10 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Fetch genres for whole albums for album in lib.albums(args): album_genre, src = self._get_genre(album) + prefix = "Pretend: " if pretend else "" self._log.info( - 'genre for album "{.album}" ({}): {}', + '{}genre for album "{.album}" ({}): {}', + prefix, album, src, album_genre, @@ -553,7 +555,8 @@ class LastGenrePlugin(plugins.BeetsPlugin): if "track" in self.sources: item_genre, src = self._get_genre(item) self._log.info( - 'genre for track "{.title}" ({}): {}', + '{}genre for track "{.title}" ({}): {}', + prefix, item, src, item_genre, @@ -569,8 +572,10 @@ class LastGenrePlugin(plugins.BeetsPlugin): # an album for item in lib.items(args): item_genre, src = self._get_genre(item) + prefix = "Pretend: " if pretend else "" self._log.info( - 'genre for track "{0.title}" ({1}): {}', + '{}genre for track "{0.title}" ({1}): {}', + prefix, item, src, item_genre, From 9b1537f22625746019f102e7f057001e957e5689 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Thu, 18 Sep 2025 08:20:40 -0400 Subject: [PATCH 21/53] Add test for --pretend option in LastGenrePlugin to skip library updates --- test/plugins/test_lastgenre.py | 37 +++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 72b0d4f00..273be4b28 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -14,7 +14,7 @@ """Tests for the 'lastgenre' plugin.""" -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest @@ -131,6 +131,41 @@ class LastGenrePluginTest(BeetsTestCase): "math rock", ] + def test_pretend_option_skips_library_updates(self): + item = self.create_item( + album="Pretend Album", + albumartist="Pretend Artist", + artist="Pretend Artist", + title="Pretend Track", + genre="", + ) + album = self.lib.add_album([item]) + + command = self.plugin.commands()[0] + opts, args = command.parser.parse_args(["--pretend"]) + + with patch.object(lastgenre.ui, "should_write", return_value=True): + with patch.object( + self.plugin, + "_get_genre", + return_value=("Mock Genre", "mock stage"), + ) as mock_get_genre: + with patch.object(self.plugin._log, "info") as log_info: + command.func(self.lib, opts, args) + + mock_get_genre.assert_called_once() + + assert any( + call.args[1] == "Pretend: " for call in log_info.call_args_list + ) + + stored_album = self.lib.get_album(album.id) + assert stored_album.genre == "" + + items = list(stored_album.items()) + assert items + assert items[0].genre == "" + def test_no_duplicate(self): """Remove duplicated genres.""" self._setup_config(count=99) From 2e307b519af55b6e6a12eb9f9bcb72d13f8ac5cc Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sat, 20 Sep 2025 19:18:56 +0200 Subject: [PATCH 22/53] lastgenre: Also mock try_write in test_pretend.. and add and original genre instead empty string (clarify intention of test / readability). Remove not really necessary assert items checks. --- test/plugins/test_lastgenre.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 273be4b28..d6df42f97 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -137,7 +137,7 @@ class LastGenrePluginTest(BeetsTestCase): albumartist="Pretend Artist", artist="Pretend Artist", title="Pretend Track", - genre="", + genre="Original Genre", ) album = self.lib.add_album([item]) @@ -151,7 +151,9 @@ class LastGenrePluginTest(BeetsTestCase): return_value=("Mock Genre", "mock stage"), ) as mock_get_genre: with patch.object(self.plugin._log, "info") as log_info: - command.func(self.lib, opts, args) + # Mock try_write to verify it's never called in pretend mode + with patch.object(item, "try_write") as mock_try_write: + command.func(self.lib, opts, args) mock_get_genre.assert_called_once() @@ -159,12 +161,12 @@ class LastGenrePluginTest(BeetsTestCase): call.args[1] == "Pretend: " for call in log_info.call_args_list ) - stored_album = self.lib.get_album(album.id) - assert stored_album.genre == "" + # Verify that try_write was never called (file operations skipped) + mock_try_write.assert_not_called() - items = list(stored_album.items()) - assert items - assert items[0].genre == "" + stored_album = self.lib.get_album(album.id) + assert stored_album.genre == "Original Genre" + assert stored_album.items()[0].genre == "Original Genre" def test_no_duplicate(self): """Remove duplicated genres.""" From 3fd49a7de8948379278adf48b6231f66068a8d56 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Mon, 15 Sep 2025 23:56:43 +0200 Subject: [PATCH 23/53] Moved arts.py file into beetsplug namespace as it is not used in core. --- {beets => beetsplug/_utils}/art.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {beets => beetsplug/_utils}/art.py (100%) diff --git a/beets/art.py b/beetsplug/_utils/art.py similarity index 100% rename from beets/art.py rename to beetsplug/_utils/art.py From 34114fe9155d1e8cf9b3cee72617429274bab949 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Mon, 15 Sep 2025 23:57:15 +0200 Subject: [PATCH 24/53] New import location for art.py --- beetsplug/_utils/__init__.py | 0 beetsplug/convert.py | 3 ++- beetsplug/embedart.py | 3 ++- test/plugins/test_embedart.py | 3 ++- 4 files changed, 6 insertions(+), 3 deletions(-) create mode 100644 beetsplug/_utils/__init__.py diff --git a/beetsplug/_utils/__init__.py b/beetsplug/_utils/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/beetsplug/convert.py b/beetsplug/convert.py index e9db3592e..102976dd7 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -25,12 +25,13 @@ from string import Template import mediafile from confuse import ConfigTypeError, Optional -from beets import art, config, plugins, ui, util +from beets import config, plugins, ui, util from beets.library import Item, parse_query_string from beets.plugins import BeetsPlugin from beets.util import par_map from beets.util.artresizer import ArtResizer from beets.util.m3u import M3UFile +from beetsplug._utils import art _fs_lock = threading.Lock() _temp_files = [] # Keep track of temporary transcoded files for deletion. diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 1a59e4f9c..cbf40f570 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -20,11 +20,12 @@ from mimetypes import guess_extension import requests -from beets import art, config, ui +from beets import config, ui from beets.plugins import BeetsPlugin from beets.ui import print_ from beets.util import bytestring_path, displayable_path, normpath, syspath from beets.util.artresizer import ArtResizer +from beetsplug._utils import art def _confirm(objs, album): diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 58d2a5f63..734183d3b 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -23,7 +23,7 @@ from unittest.mock import MagicMock, patch import pytest from mediafile import MediaFile -from beets import art, config, logging, ui +from beets import config, logging, ui from beets.test import _common from beets.test.helper import ( BeetsTestCase, @@ -33,6 +33,7 @@ from beets.test.helper import ( ) from beets.util import bytestring_path, displayable_path, syspath from beets.util.artresizer import ArtResizer +from beetsplug._utils import art from test.test_art_resize import DummyIMBackend From f4691c85e947aa34c389fe6b2613cd5e89eb1763 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 16 Sep 2025 00:03:16 +0200 Subject: [PATCH 25/53] Added changelog and git blame ignore rev --- .git-blame-ignore-revs | 4 +++- docs/changelog.rst | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index fbe32b497..ed86e3f8c 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -70,4 +70,6 @@ d93ddf8dd43e4f9ed072a03829e287c78d2570a2 # Moved dev docs 07549ed896d9649562d40b75cd30702e6fa6e975 # Moved plugin docs Further Reading chapter -33f1a5d0bef8ca08be79ee7a0d02a018d502680d \ No newline at end of file +33f1a5d0bef8ca08be79ee7a0d02a018d502680d +# Moved art.py utility module from beets into beetsplug +a7b69e50108eebef8ce92c015f18a42f8bf7276f \ No newline at end of file diff --git a/docs/changelog.rst b/docs/changelog.rst index ddafd975d..94a90a1b5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -35,6 +35,9 @@ Other changes: match :bug:`6020` - :doc:`guides/tagger`: Section on no matching release found, related to possibly disabled musicbrainz plugin :bug:`6020` +- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as it + is not used in the core beets codebase. It can now be found in + ``beetsplug._utils``. - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping From c991b14e7d913d719e0ab3580336b1e386bbfb69 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Tue, 16 Sep 2025 00:18:04 +0200 Subject: [PATCH 26/53] fix test by changing patch --- .git-blame-ignore-revs | 2 +- docs/changelog.rst | 4 ++-- test/plugins/test_embedart.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index ed86e3f8c..2ee64a97d 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -72,4 +72,4 @@ d93ddf8dd43e4f9ed072a03829e287c78d2570a2 # Moved plugin docs Further Reading chapter 33f1a5d0bef8ca08be79ee7a0d02a018d502680d # Moved art.py utility module from beets into beetsplug -a7b69e50108eebef8ce92c015f18a42f8bf7276f \ No newline at end of file +28aee0fde463f1e18dfdba1994e2bdb80833722f \ No newline at end of file diff --git a/docs/changelog.rst b/docs/changelog.rst index 94a90a1b5..44449e811 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -35,8 +35,8 @@ Other changes: match :bug:`6020` - :doc:`guides/tagger`: Section on no matching release found, related to possibly disabled musicbrainz plugin :bug:`6020` -- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as it - is not used in the core beets codebase. It can now be found in +- Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as + it is not used in the core beets codebase. It can now be found in ``beetsplug._utils``. - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 734183d3b..d40025374 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -284,7 +284,7 @@ class DummyArtResizer(ArtResizer): @patch("beets.util.artresizer.subprocess") -@patch("beets.art.extract") +@patch("beetsplug._utils.art.extract") class ArtSimilarityTest(unittest.TestCase): def setUp(self): self.item = _common.item() From 92579b30d833ba897cd22dd3b6e176fea83a4d27 Mon Sep 17 00:00:00 2001 From: Henry Date: Sun, 21 Sep 2025 09:25:30 -0700 Subject: [PATCH 27/53] Reformat docs --- docs/changelog.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4e7b92840..5d7e3f803 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -37,9 +37,8 @@ Other changes: possibly disabled musicbrainz plugin :bug:`6020` - Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as it is not used in the core beets codebase. It can now be found in - ``beetsplug._utils``. - - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific - disambiguation stripping + ``beetsplug._utils``. - :class:`beets.metadata_plugin.MetadataSourcePlugin`: + Remove discogs specific disambiguation stripping 2.4.0 (September 13, 2025) -------------------------- From 76c049938c8253ebb48e783f295e2c5d2cb97ed8 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 17 Sep 2025 13:25:20 -0400 Subject: [PATCH 28/53] Update missing plugin configuration options and formatting details --- docs/plugins/missing.rst | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/plugins/missing.rst b/docs/plugins/missing.rst index 7764f5fe1..81d0b2d0e 100644 --- a/docs/plugins/missing.rst +++ b/docs/plugins/missing.rst @@ -39,21 +39,26 @@ Configuration To configure the plugin, make a ``missing:`` section in your configuration file. The available options are: -- **count**: Print a count of missing tracks per album, with ``format`` - defaulting to ``$albumartist - $album: $missing``. Default: ``no``. -- **format**: A specific format with which to print every track. This uses the - same template syntax as beets' :doc:`path formats `. - The usage is inspired by, and therefore similar to, the :ref:`list ` - command. Default: :ref:`format_item`. +- **count**: Print a count of missing tracks per album, with the global + ``format_album`` used for formatting. Default: ``no``. - **total**: Print a single count of missing tracks in all albums. Default: ``no``. +Formatting +~~~~~~~~~~ + +- This plugin uses global formatting options from the main configuration; see :ref:`format_item` and :ref:`format_album`: + +- :ref:`format_item`: Used when listing missing tracks (default item format). +- :ref:`format_album`: Used when showing counts (``-c``) or missing albums (``-a``). + Here's an example :: + format_album: $albumartist - $album + format_item: $artist - $album - $title missing: - format: $albumartist - $album - $title count: no total: no From de4494a5b19890b80ed141d459c6625d0a79cc52 Mon Sep 17 00:00:00 2001 From: Alok Saboo Date: Wed, 17 Sep 2025 13:29:00 -0400 Subject: [PATCH 29/53] lint --- docs/plugins/missing.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/plugins/missing.rst b/docs/plugins/missing.rst index 81d0b2d0e..10842933c 100644 --- a/docs/plugins/missing.rst +++ b/docs/plugins/missing.rst @@ -47,10 +47,11 @@ The available options are: Formatting ~~~~~~~~~~ -- This plugin uses global formatting options from the main configuration; see :ref:`format_item` and :ref:`format_album`: - +- This plugin uses global formatting options from the main configuration; see + :ref:`format_item` and :ref:`format_album`: - :ref:`format_item`: Used when listing missing tracks (default item format). -- :ref:`format_album`: Used when showing counts (``-c``) or missing albums (``-a``). +- :ref:`format_album`: Used when showing counts (``-c``) or missing albums + (``-a``). Here's an example From f0a6059685f32d171c967d06a1a2c3a9563376f8 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Sat, 23 Aug 2025 08:37:22 -0500 Subject: [PATCH 30/53] feat(FtInTitle): support tracks by artists != album artist --- beetsplug/ftintitle.py | 56 +++++++++++++++++-------------- docs/changelog.rst | 3 ++ test/plugins/test_ftintitle.py | 60 +++++++++++++++++++++++++++++++--- 3 files changed, 89 insertions(+), 30 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index f0f088099..e17d7bc1c 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -26,14 +26,16 @@ if TYPE_CHECKING: from beets.library import Item -def split_on_feat(artist: str) -> tuple[str, str | None]: +def split_on_feat( + artist: str, for_artist: bool = True +) -> tuple[str, str | None]: """Given an artist string, split the "main" artist from any artist on the right-hand side of a string like "feat". Return the main artist, which is always a string, and the featuring artist, which may be a string or None if none is present. """ # split on the first "feat". - regex = re.compile(plugins.feat_tokens(), re.IGNORECASE) + regex = re.compile(plugins.feat_tokens(for_artist), re.IGNORECASE) parts = tuple(s.strip() for s in regex.split(artist, 1)) if len(parts) == 1: return parts[0], None @@ -53,32 +55,35 @@ def contains_feat(title: str) -> bool: ) -def find_feat_part(artist: str, albumartist: str) -> str | None: +def find_feat_part(artist: str, albumartist: str | None) -> str | None: """Attempt to find featured artists in the item's artist fields and return the results. Returns None if no featured artist found. """ - # Look for the album artist in the artist field. If it's not - # present, give up. - albumartist_split = artist.split(albumartist, 1) - if len(albumartist_split) <= 1: - return None + # Handle a wider variety of extraction cases if the album artist is + # contained within the track artist. + if albumartist and albumartist in artist: + albumartist_split = artist.split(albumartist, 1) - # If the last element of the split (the right-hand side of the - # album artist) is nonempty, then it probably contains the - # featured artist. - elif albumartist_split[1] != "": - # Extract the featured artist from the right-hand side. - _, feat_part = split_on_feat(albumartist_split[1]) - return feat_part + # If the last element of the split (the right-hand side of the + # album artist) is nonempty, then it probably contains the + # featured artist. + if albumartist_split[1] != "": + # Extract the featured artist from the right-hand side. + _, feat_part = split_on_feat(albumartist_split[1]) + return feat_part - # Otherwise, if there's nothing on the right-hand side, look for a - # featuring artist on the left-hand side. - else: - lhs, rhs = split_on_feat(albumartist_split[0]) - if lhs: - return lhs + # Otherwise, if there's nothing on the right-hand side, + # look for a featuring artist on the left-hand side. + else: + lhs, _ = split_on_feat(albumartist_split[0]) + if lhs: + return lhs - return None + # Fall back to conservative handling of the track artist without relying + # on albumartist, which covers compilations using a 'Various Artists' + # albumartist and album tracks by a guest artist featuring a third artist. + _, feat_part = split_on_feat(artist, False) + return feat_part class FtInTitlePlugin(plugins.BeetsPlugin): @@ -153,8 +158,9 @@ class FtInTitlePlugin(plugins.BeetsPlugin): "artist: {.artist} (Not changing due to keep_in_artist)", item ) else: - self._log.info("artist: {0.artist} -> {0.albumartist}", item) - item.artist = item.albumartist + track_artist, _ = split_on_feat(item.artist) + self._log.info("artist: {0.artist} -> {1}", item, track_artist) + item.artist = track_artist if item.artist_sort: # Just strip the featured artist from the sort name. @@ -187,7 +193,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # Check whether there is a featured artist on this track and the # artist field does not exactly match the album artist field. In # that case, we attempt to move the featured artist to the title. - if not albumartist or albumartist == artist: + if albumartist and artist == albumartist: return False _, featured = split_on_feat(artist) diff --git a/docs/changelog.rst b/docs/changelog.rst index 63a8fe339..c2d6ea1cf 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -161,6 +161,9 @@ Other changes: Autogenerated API references are now located in the ``docs/api`` subdirectory. - :doc:`/plugins/substitute`: Fix rST formatting for example cases so that each case is shown on separate lines. +- :doc:`/plugins/ftintitle`: Process items whose albumartist is not contained in + the artist field, including compilations using Various Artists as an + albumartist and album tracks by guest artists featuring a third artist. - Refactored library.py file by splitting it into multiple modules within the beets/library directory. - Added a test to check that all plugins can be imported without errors. diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 572431b45..e049fe32a 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -40,6 +40,43 @@ class FtInTitlePluginFunctional(PluginTestCase): self.config["ftintitle"]["auto"] = auto self.config["ftintitle"]["keep_in_artist"] = keep_in_artist + def test_functional_no_featured_artist(self): + item = self._ft_add_item("/", "Alice", "Song 1", "Alice") + self.run_command("ftintitle") + item.load() + assert item["artist"] == "Alice" + assert item["title"] == "Song 1" + + def test_functional_no_albumartist(self): + self._ft_set_config("feat {0}") + item = self._ft_add_item("/", "Alice ft. Bob", "Song 1", None) + self.run_command("ftintitle") + item.load() + assert item["artist"] == "Alice" + assert item["title"] == "Song 1 feat Bob" + + def test_functional_no_albumartist_no_feature(self): + item = self._ft_add_item("/", "Alice", "Song 1", None) + self.run_command("ftintitle") + item.load() + assert item["artist"] == "Alice" + assert item["title"] == "Song 1" + + def test_functional_guest_artist(self): + self._ft_set_config("featuring {0}") + item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "George") + self.run_command("ftintitle") + item.load() + assert item["artist"] == "Alice" + assert item["title"] == "Song 1 featuring Bob" + + def test_functional_guest_artist_no_feature(self): + item = self._ft_add_item("/", "Alice", "Song 1", "George") + self.run_command("ftintitle") + item.load() + assert item["artist"] == "Alice" + assert item["title"] == "Song 1" + def test_functional_drop(self): item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") self.run_command("ftintitle", "-d") @@ -47,12 +84,25 @@ class FtInTitlePluginFunctional(PluginTestCase): assert item["artist"] == "Alice" assert item["title"] == "Song 1" - def test_functional_not_found(self): + def test_functional_drop_no_featured_artist(self): + item = self._ft_add_item("/", "Alice", "Song 1", "Alice") + self.run_command("ftintitle", "-d") + item.load() + assert item["artist"] == "Alice" + assert item["title"] == "Song 1" + + def test_functional_drop_guest_artist(self): item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "George") self.run_command("ftintitle", "-d") item.load() - # item should be unchanged - assert item["artist"] == "Alice ft Bob" + assert item["artist"] == "Alice" + assert item["title"] == "Song 1" + + def test_functional_drop_guest_artist_no_feature(self): + item = self._ft_add_item("/", "Alice", "Song 1", "George") + self.run_command("ftintitle", "-d") + item.load() + assert item["artist"] == "Alice" assert item["title"] == "Song 1" def test_functional_custom_format(self): @@ -147,7 +197,7 @@ class FtInTitlePluginTest(unittest.TestCase): { "artist": "Alice ft. Carol", "album_artist": "Bob", - "feat_part": None, + "feat_part": "Carol", }, ] @@ -155,7 +205,7 @@ class FtInTitlePluginTest(unittest.TestCase): feat_part = ftintitle.find_feat_part( test_case["artist"], test_case["album_artist"] ) - assert feat_part == test_case["feat_part"] + assert feat_part == test_case["feat_part"], f"failed: {test_case}" def test_split_on_feat(self): parts = ftintitle.split_on_feat("Alice ft. Bob") From 6ad7c5489c1d1c1d6ca7e86b32e0f3497796bd48 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Thu, 18 Sep 2025 23:00:30 -0500 Subject: [PATCH 31/53] test(ftintitle): parameterize tests --- test/plugins/test_ftintitle.py | 435 +++++++++++++++++---------------- 1 file changed, 225 insertions(+), 210 deletions(-) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index e049fe32a..2b560e01c 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -14,8 +14,11 @@ """Tests for the 'ftintitle' plugin.""" -import unittest +from typing import Dict, Optional +from _pytest.fixtures import SubRequest +import pytest +from beets.library.models import Item from beets.test.helper import PluginTestCase from beetsplug import ftintitle @@ -23,219 +26,231 @@ from beetsplug import ftintitle class FtInTitlePluginFunctional(PluginTestCase): plugin = "ftintitle" - def _ft_add_item(self, path, artist, title, aartist): - return self.add_item( - path=path, - artist=artist, - artist_sort=artist, - title=title, - albumartist=aartist, - ) - def _ft_set_config( - self, ftformat, drop=False, auto=True, keep_in_artist=False - ): - self.config["ftintitle"]["format"] = ftformat - self.config["ftintitle"]["drop"] = drop - self.config["ftintitle"]["auto"] = auto - self.config["ftintitle"]["keep_in_artist"] = keep_in_artist - - def test_functional_no_featured_artist(self): - item = self._ft_add_item("/", "Alice", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_no_albumartist(self): - self._ft_set_config("feat {0}") - item = self._ft_add_item("/", "Alice ft. Bob", "Song 1", None) - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 feat Bob" - - def test_functional_no_albumartist_no_feature(self): - item = self._ft_add_item("/", "Alice", "Song 1", None) - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_guest_artist(self): - self._ft_set_config("featuring {0}") - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "George") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 featuring Bob" - - def test_functional_guest_artist_no_feature(self): - item = self._ft_add_item("/", "Alice", "Song 1", "George") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_drop(self): - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_drop_no_featured_artist(self): - item = self._ft_add_item("/", "Alice", "Song 1", "Alice") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_drop_guest_artist(self): - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "George") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_drop_guest_artist_no_feature(self): - item = self._ft_add_item("/", "Alice", "Song 1", "George") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1" - - def test_functional_custom_format(self): - self._ft_set_config("feat. {}") - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 feat. Bob" - - self._ft_set_config("featuring {}") - item = self._ft_add_item("/", "Alice feat. Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 featuring Bob" - - self._ft_set_config("with {}") - item = self._ft_add_item("/", "Alice feat Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice" - assert item["title"] == "Song 1 with Bob" - - def test_functional_keep_in_artist(self): - self._ft_set_config("feat. {}", keep_in_artist=True) - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle") - item.load() - assert item["artist"] == "Alice ft Bob" - assert item["title"] == "Song 1 feat. Bob" - - item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice") - self.run_command("ftintitle", "-d") - item.load() - assert item["artist"] == "Alice ft Bob" - assert item["title"] == "Song 1" +@pytest.fixture +def env(request: SubRequest) -> FtInTitlePluginFunctional: + case = FtInTitlePluginFunctional(methodName="runTest") + case.setUp() + request.addfinalizer(case.tearDown) + return case -class FtInTitlePluginTest(unittest.TestCase): - def setUp(self): - """Set up configuration""" - ftintitle.FtInTitlePlugin() +def set_config( + env: FtInTitlePluginFunctional, cfg: Optional[Dict[str, str | bool]] +) -> None: + cfg = {} if cfg is None else cfg + defaults = { + "drop": False, + "auto": True, + "keep_in_artist": False, + } + env.config["ftintitle"].set(defaults) + env.config["ftintitle"].set(cfg) - def test_find_feat_part(self): - test_cases = [ - { - "artist": "Alice ft. Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice feat Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice featuring Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice & Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice and Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice With Bob", - "album_artist": "Alice", - "feat_part": "Bob", - }, - { - "artist": "Alice defeat Bob", - "album_artist": "Alice", - "feat_part": None, - }, - { - "artist": "Alice & Bob", - "album_artist": "Bob", - "feat_part": "Alice", - }, - { - "artist": "Alice ft. Bob", - "album_artist": "Bob", - "feat_part": "Alice", - }, - { - "artist": "Alice ft. Carol", - "album_artist": "Bob", - "feat_part": "Carol", - }, - ] - for test_case in test_cases: - feat_part = ftintitle.find_feat_part( - test_case["artist"], test_case["album_artist"] - ) - assert feat_part == test_case["feat_part"], f"failed: {test_case}" +def add_item( + env: FtInTitlePluginFunctional, + path: str, + artist: str, + title: str, + albumartist: str, +) -> Item: + return env.add_item( + path=path, + artist=artist, + artist_sort=artist, + title=title, + albumartist=albumartist, + ) - def test_split_on_feat(self): - parts = ftintitle.split_on_feat("Alice ft. Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice feat Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice feat. Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice featuring Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice & Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice and Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice With Bob") - assert parts == ("Alice", "Bob") - parts = ftintitle.split_on_feat("Alice defeat Bob") - assert parts == ("Alice defeat Bob", None) - def test_contains_feat(self): - assert ftintitle.contains_feat("Alice ft. Bob") - assert ftintitle.contains_feat("Alice feat. Bob") - assert ftintitle.contains_feat("Alice feat Bob") - assert ftintitle.contains_feat("Alice featuring Bob") - assert ftintitle.contains_feat("Alice (ft. Bob)") - assert ftintitle.contains_feat("Alice (feat. Bob)") - assert ftintitle.contains_feat("Alice [ft. Bob]") - assert ftintitle.contains_feat("Alice [feat. Bob]") - assert not ftintitle.contains_feat("Alice defeat Bob") - assert not ftintitle.contains_feat("Aliceft.Bob") - assert not ftintitle.contains_feat("Alice (defeat Bob)") - assert not ftintitle.contains_feat("Live and Let Go") - assert not ftintitle.contains_feat("Come With Me") +@pytest.mark.parametrize( + "cfg, cmd_args, input, expected", + [ + pytest.param( + None, + ("ftintitle",), + ("Alice", "Song 1", "Alice"), + ("Alice", "Song 1"), + id="no-featured-artist", + ), + pytest.param( + {"format": "feat {0}"}, + ("ftintitle",), + ("Alice ft. Bob", "Song 1", None), + ("Alice", "Song 1 feat Bob"), + id="no-albumartist-custom-format", + ), + pytest.param( + None, + ("ftintitle",), + ("Alice", "Song 1", None), + ("Alice", "Song 1"), + id="no-albumartist-no-feature", + ), + pytest.param( + {"format": "featuring {0}"}, + ("ftintitle",), + ("Alice ft Bob", "Song 1", "George"), + ("Alice", "Song 1 featuring Bob"), + id="guest-artist-custom-format", + ), + pytest.param( + None, + ("ftintitle",), + ("Alice", "Song 1", "George"), + ("Alice", "Song 1"), + id="guest-artist-no-feature", + ), + # ---- drop (-d) variants ---- + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice", "Song 1"), + id="drop-self-ft", + ), + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice", "Song 1", "Alice"), + ("Alice", "Song 1"), + id="drop-self-no-ft", + ), + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice ft Bob", "Song 1", "George"), + ("Alice", "Song 1"), + id="drop-guest-ft", + ), + pytest.param( + None, + ("ftintitle", "-d"), + ("Alice", "Song 1", "George"), + ("Alice", "Song 1"), + id="drop-guest-no-ft", + ), + # ---- custom format variants ---- + pytest.param( + {"format": "feat. {}"}, + ("ftintitle",), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice", "Song 1 feat. Bob"), + id="custom-format-feat-dot", + ), + pytest.param( + {"format": "featuring {}"}, + ("ftintitle",), + ("Alice feat. Bob", "Song 1", "Alice"), + ("Alice", "Song 1 featuring Bob"), + id="custom-format-featuring", + ), + pytest.param( + {"format": "with {}"}, + ("ftintitle",), + ("Alice feat Bob", "Song 1", "Alice"), + ("Alice", "Song 1 with Bob"), + id="custom-format-with", + ), + # ---- keep_in_artist variants ---- + pytest.param( + {"format": "feat. {}", "keep_in_artist": True}, + ("ftintitle",), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice ft Bob", "Song 1 feat. Bob"), + id="keep-in-artist-add-to-title", + ), + pytest.param( + {"format": "feat. {}", "keep_in_artist": True}, + ("ftintitle", "-d"), + ("Alice ft Bob", "Song 1", "Alice"), + ("Alice ft Bob", "Song 1"), + id="keep-in-artist-drop-from-title", + ), + ], +) +def test_ftintitle_functional( + env: FtInTitlePluginFunctional, + cfg: Optional[Dict[str, str | bool]], + cmd_args: tuple[str, ...], + input: tuple[str, str, str], + expected: tuple[str, str], +) -> None: + set_config(env, cfg) + ftintitle.FtInTitlePlugin() + + artist, title, albumartist = input + item = add_item(env, "/", artist, title, albumartist) + + env.run_command(*cmd_args) + item.load() + + expected_artist, expected_title = expected + assert item["artist"] == expected_artist + assert item["title"] == expected_title + + +@pytest.mark.parametrize( + "artist,albumartist,expected", + [ + ("Alice ft. Bob", "Alice", "Bob"), + ("Alice feat Bob", "Alice", "Bob"), + ("Alice featuring Bob", "Alice", "Bob"), + ("Alice & Bob", "Alice", "Bob"), + ("Alice and Bob", "Alice", "Bob"), + ("Alice With Bob", "Alice", "Bob"), + ("Alice defeat Bob", "Alice", None), + ("Alice & Bob", "Bob", "Alice"), + ("Alice ft. Bob", "Bob", "Alice"), + ("Alice ft. Carol", "Bob", "Carol"), + ], +) +def test_find_feat_part( + artist: str, + albumartist: str, + expected: Optional[str], +) -> None: + assert ftintitle.find_feat_part(artist, albumartist) == expected + + +@pytest.mark.parametrize( + "input,expected", + [ + ("Alice ft. Bob", ("Alice", "Bob")), + ("Alice feat Bob", ("Alice", "Bob")), + ("Alice feat. Bob", ("Alice", "Bob")), + ("Alice featuring Bob", ("Alice", "Bob")), + ("Alice & Bob", ("Alice", "Bob")), + ("Alice and Bob", ("Alice", "Bob")), + ("Alice With Bob", ("Alice", "Bob")), + ("Alice defeat Bob", ("Alice defeat Bob", None)), + ], +) +def test_split_on_feat( + input: str, + expected: tuple[str, Optional[str]], +) -> None: + assert ftintitle.split_on_feat(input) == expected + + +@pytest.mark.parametrize( + "input,expected", + [ + ("Alice ft. Bob", True), + ("Alice feat. Bob", True), + ("Alice feat Bob", True), + ("Alice featuring Bob", True), + ("Alice (ft. Bob)", True), + ("Alice (feat. Bob)", True), + ("Alice [ft. Bob]", True), + ("Alice [feat. Bob]", True), + ("Alice defeat Bob", False), + ("Aliceft.Bob", False), + ("Alice (defeat Bob)", False), + ("Live and Let Go", False), + ("Come With Me", False), + ], +) +def test_contains_feat(input: str, expected: bool) -> None: + assert ftintitle.contains_feat(input) is expected From 042b5d64ebceb22cbcf5dbdf1745b7d9f0125b80 Mon Sep 17 00:00:00 2001 From: Trey Turner Date: Thu, 18 Sep 2025 23:18:01 -0500 Subject: [PATCH 32/53] test(ftintitle): fix flake, massage mypy --- test/plugins/test_ftintitle.py | 42 ++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 2b560e01c..005318b11 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -14,8 +14,8 @@ """Tests for the 'ftintitle' plugin.""" -from typing import Dict, Optional -from _pytest.fixtures import SubRequest +from typing import Dict, Generator, Optional, Tuple, Union + import pytest from beets.library.models import Item @@ -28,15 +28,17 @@ class FtInTitlePluginFunctional(PluginTestCase): @pytest.fixture -def env(request: SubRequest) -> FtInTitlePluginFunctional: +def env() -> Generator[FtInTitlePluginFunctional, None, None]: case = FtInTitlePluginFunctional(methodName="runTest") case.setUp() - request.addfinalizer(case.tearDown) - return case + try: + yield case + finally: + case.tearDown() def set_config( - env: FtInTitlePluginFunctional, cfg: Optional[Dict[str, str | bool]] + env: FtInTitlePluginFunctional, cfg: Optional[Dict[str, Union[str, bool]]] ) -> None: cfg = {} if cfg is None else cfg defaults = { @@ -53,7 +55,7 @@ def add_item( path: str, artist: str, title: str, - albumartist: str, + albumartist: Optional[str], ) -> Item: return env.add_item( path=path, @@ -65,7 +67,7 @@ def add_item( @pytest.mark.parametrize( - "cfg, cmd_args, input, expected", + "cfg, cmd_args, given, expected", [ pytest.param( None, @@ -172,15 +174,15 @@ def add_item( ) def test_ftintitle_functional( env: FtInTitlePluginFunctional, - cfg: Optional[Dict[str, str | bool]], - cmd_args: tuple[str, ...], - input: tuple[str, str, str], - expected: tuple[str, str], + cfg: Optional[Dict[str, Union[str, bool]]], + cmd_args: Tuple[str, ...], + given: Tuple[str, str, Optional[str]], + expected: Tuple[str, str], ) -> None: set_config(env, cfg) ftintitle.FtInTitlePlugin() - artist, title, albumartist = input + artist, title, albumartist = given item = add_item(env, "/", artist, title, albumartist) env.run_command(*cmd_args) @@ -215,7 +217,7 @@ def test_find_feat_part( @pytest.mark.parametrize( - "input,expected", + "given,expected", [ ("Alice ft. Bob", ("Alice", "Bob")), ("Alice feat Bob", ("Alice", "Bob")), @@ -228,14 +230,14 @@ def test_find_feat_part( ], ) def test_split_on_feat( - input: str, - expected: tuple[str, Optional[str]], + given: str, + expected: Tuple[str, Optional[str]], ) -> None: - assert ftintitle.split_on_feat(input) == expected + assert ftintitle.split_on_feat(given) == expected @pytest.mark.parametrize( - "input,expected", + "given,expected", [ ("Alice ft. Bob", True), ("Alice feat. Bob", True), @@ -252,5 +254,5 @@ def test_split_on_feat( ("Come With Me", False), ], ) -def test_contains_feat(input: str, expected: bool) -> None: - assert ftintitle.contains_feat(input) is expected +def test_contains_feat(given: str, expected: bool) -> None: + assert ftintitle.contains_feat(given) is expected From 8e644157e87c970bca3140075b022d0ac3a377bb Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Mon, 22 Sep 2025 19:47:50 +0200 Subject: [PATCH 33/53] Refactor tests, adjust changelog, move config option to new features. --- docs/changelog.rst | 11 ++--- test/plugins/test_discogs.py | 89 +++++++++++++----------------------- 2 files changed, 38 insertions(+), 62 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b182a6cba..8a16399f9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,8 @@ New features: - :doc:`plugins/lastgenre`: Add a ``--pretend`` option to preview genre changes without storing or writing them. +- :doc:`plugins/discogs`: New config option `strip_disambiguation` to toggle + stripping discogs numeric disambiguation on artist and label fields. Bug fixes: @@ -21,12 +23,8 @@ Bug fixes: - :doc:`plugins/spotify` Removed old and undocumented config options `artist_field`, `album_field` and `track` that were causing issues with track matching. :bug:`5189` -- :doc:`plugins/discogs` Added config option `strip_disambiguation` to allow - choice of removing discogs numeric disambiguation :bug:`5366` - :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels. :bug:`5366` -- :doc:`plugins/discogs` Wrote test coverage for removing disambiguation. - :bug:`5366` For packagers: @@ -40,8 +38,9 @@ Other changes: possibly disabled musicbrainz plugin :bug:`6020` - Moved ``art.py`` utility module from ``beets`` into ``beetsplug`` namespace as it is not used in the core beets codebase. It can now be found in - ``beetsplug._utils``. - :class:`beets.metadata_plugin.MetadataSourcePlugin`: - Remove discogs specific disambiguation stripping + ``beetsplug._utils``. +- :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific + disambiguation stripping. 2.4.0 (September 13, 2025) -------------------------- diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 5fe73dcac..92301380e 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -374,38 +374,26 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.genre == "GENRE1, GENRE2" assert d.style is None - def test_strip_disambiguation_label_artist(self): - """Test removing discogs disambiguation from artist and label""" + def test_strip_disambiguation(self): + """Test removing disambiguation from all disambiguated fields.""" data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [self._make_track("A", "1", "01:01")], - "artists": [{"name": "ARTIST NAME (2)", "id": 321, "join": ""}], - "title": "TITLE", - "labels": [ + "tracklist": [ { - "name": "LABEL NAME (5)", - "catno": "CATALOG NUMBER", + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} + ], } ], - } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) - d = DiscogsPlugin().get_album_info(release) - assert d.artist == "ARTIST NAME" - assert d.label == "LABEL NAME" - - def test_strip_disambiguation_off_label_artist(self): - """Test not removing discogs disambiguation from artist and label""" - config["discogs"]["strip_disambiguation"] = False - data = { - "id": 123, - "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [self._make_track("a", "1", "01:01")], - "artists": [{"name": "ARTIST NAME (2)", "id": 321, "join": ""}], + "artists": [ + {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, + {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + ], "title": "title", "labels": [ { @@ -420,30 +408,13 @@ class DGAlbumInfoTest(BeetsTestCase): artists=[Bag(data=d) for d in data["artists"]], ) d = DiscogsPlugin().get_album_info(release) - assert d.artist == "ARTIST NAME (2)" - assert d.label == "LABEL NAME (5)" - - def test_strip_disambiguation_multiple_artists(self): - """Test removing disambiguation if there are multiple artists on the release""" - data = { - "id": 123, - "uri": "https://www.discogs.com/release/123456-something", - "tracklist": [self._make_track("a", "1", "01:01")], - "artists": [ - {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, - {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, - ], - "title": "title", - } - release = Bag( - data=data, - title=data["title"], - artists=[Bag(data=d) for d in data["artists"]], - ) - d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME & OTHER ARTIST" + assert d.tracks[0].artist == "TEST ARTIST" + assert d.label == "LABEL NAME" - def test_strip_disambiguation_artist_tracks(self): + def test_strip_disambiguation_false(self): + """Test disabling disambiguation removal from all disambiguated fields.""" + config["discogs"]["strip_disambiguation"] = False data = { "id": 123, "uri": "https://www.discogs.com/release/123456-something", @@ -454,16 +425,21 @@ class DGAlbumInfoTest(BeetsTestCase): "type_": "track", "duration": "5:44", "artists": [ - { - "name": "TEST ARTIST (5)", - "tracks": "", - "id": 11146, - } + {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} ], } ], - "artists": [{"name": "OTHER ARTIST (5)", "id": 321, "join": ""}], + "artists": [ + {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, + {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + ], "title": "title", + "labels": [ + { + "name": "LABEL NAME (5)", + "catno": "catalog number", + } + ], } release = Bag( data=data, @@ -471,8 +447,9 @@ class DGAlbumInfoTest(BeetsTestCase): artists=[Bag(data=d) for d in data["artists"]], ) d = DiscogsPlugin().get_album_info(release) - assert d.tracks[0].artist == "TEST ARTIST" - assert d.artist == "OTHER ARTIST" + assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" + assert d.tracks[0].artist == "TEST ARTIST (5)" + assert d.label == "LABEL NAME (5)" @pytest.mark.parametrize( From 84e52e1b4a4a73151f29d3a99da857dfa0c96583 Mon Sep 17 00:00:00 2001 From: Henry Date: Mon, 22 Sep 2025 20:49:53 -0700 Subject: [PATCH 34/53] Test written, beginning fix --- beetsplug/discogs.py | 1 + test/plugins/test_discogs.py | 57 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index f22ea2274..caefbe64a 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -652,6 +652,7 @@ class DiscogsPlugin(MetadataSourcePlugin): ) artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) + featured = map(lambda artist: artist["name"] if artist["role"].find("Featuring") else "", track.get("extraartists", [])) return TrackInfo( title=title, track_id=track_id, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 92301380e..84573d337 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -451,6 +451,63 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" +@pytest.mark.parametrize("track, expected_artist", + [({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [{ + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "ARTIST feat. MUSICIAN" + ), + ({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [{ + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, { + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "ARTIST feat. PERFORMER & MUSICIAN" + ), + ({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artist": "NEW ARTIST", + "extraartists": [{ + "name": "SOLIST", + "role": "Featuring", + }, { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, { + "name": "RANDOM", + "role": "Written-By", + }, { + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "NEW ARTIST feat. SOLIST, PERFORMER & MUSICIAN" + )]) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_parse_featured_artists(track, expected_artist): + """ Tests the plugins ability to parse a featured artist. + Initial check with one featured artist, two featured artists, + and three. Ignores artists that are not listed as featured.""" + t = DiscogsPlugin().get_track_info(track, 1, 1) + assert t.artist == expected_artist @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", From 2bf411e77d9d5feb077c2d86ad48c1bfc3daeade Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 23 Sep 2025 10:11:13 -0700 Subject: [PATCH 35/53] Featured artists extracted and appended, need to see if join needs to be variable --- beetsplug/discogs.py | 20 +++++++++++++++----- test/plugins/test_discogs.py | 13 ++++++++----- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index caefbe64a..43a501a22 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -339,7 +339,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # convenient `.tracklist` property, which will strip out useful artist # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. - tracks = self.get_tracks(result.data["tracklist"]) + tracks = self.get_tracks(result.data["tracklist"], artist, artist_id) # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -446,7 +446,7 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks(self, tracklist): + def get_tracks(self, tracklist, album_artist, album_artist_id): """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -471,7 +471,8 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions) + track_info = self.get_track_info(track, index, divisions, + album_artist, album_artist_id) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -638,7 +639,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions): + def get_track_info(self, track, index, divisions, album_artist, album_artist_id): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -650,9 +651,18 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist( track.get("artists", []), join_key="join" ) + # If no artist and artist is returned, set to match album artist + if not artist: + artist = album_artist + artist_id = album_artist_id artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) - featured = map(lambda artist: artist["name"] if artist["role"].find("Featuring") else "", track.get("extraartists", [])) + # Add featured artists + extraartists = track.get("extraartists", []) + featured = [ + artist["name"] for artist in extraartists if artist["role"].find("Featuring") != -1] + if featured: + artist = f"{artist} feat. {', '.join(featured)}" return TrackInfo( title=title, track_id=track_id, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 84573d337..19428b125 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -477,16 +477,19 @@ class DGAlbumInfoTest(BeetsTestCase): "role": "Featuring", }] }, - "ARTIST feat. PERFORMER & MUSICIAN" + "ARTIST feat. PERFORMER, MUSICIAN" ), ({ "type_": "track", "title": "track", "position": "1", "duration": "5:00", - "artist": "NEW ARTIST", + "artists": [{ + "name": "NEW ARTIST", + "tracks": "", + "id": 11146}], "extraartists": [{ - "name": "SOLIST", + "name": "SOLOIST", "role": "Featuring", }, { "name": "PERFORMER", @@ -499,14 +502,14 @@ class DGAlbumInfoTest(BeetsTestCase): "role": "Featuring", }] }, - "NEW ARTIST feat. SOLIST, PERFORMER & MUSICIAN" + "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN" )]) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_parse_featured_artists(track, expected_artist): """ Tests the plugins ability to parse a featured artist. Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" - t = DiscogsPlugin().get_track_info(track, 1, 1) + t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) assert t.artist == expected_artist @pytest.mark.parametrize( From 6aba11d4a0fa85163607218853549004187969ae Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 23 Sep 2025 11:05:48 -0700 Subject: [PATCH 36/53] testing, updated changelog --- beetsplug/discogs.py | 18 ++--- docs/changelog.rst | 2 + test/plugins/test_discogs.py | 126 ++++++++++++++++++++--------------- 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 43a501a22..c8ce43a1e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -386,10 +386,6 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in tracks: track.media = media track.medium_total = mediums.count(track.medium) - if not track.artist: # get_track_info often fails to find artist - track.artist = artist - if not track.artist_id: - track.artist_id = artist_id # Discogs does not have track IDs. Invent our own IDs as proposed # in #2336. track.track_id = f"{album_id}-{track.track_alt}" @@ -471,8 +467,9 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions, - album_artist, album_artist_id) + track_info = self.get_track_info( + track, index, divisions, album_artist, album_artist_id + ) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -639,7 +636,9 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions, album_artist, album_artist_id): + def get_track_info( + self, track, index, divisions, album_artist, album_artist_id + ): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -660,7 +659,10 @@ class DiscogsPlugin(MetadataSourcePlugin): # Add featured artists extraartists = track.get("extraartists", []) featured = [ - artist["name"] for artist in extraartists if artist["role"].find("Featuring") != -1] + artist["name"] + for artist in extraartists + if artist["role"].find("Featuring") != -1 + ] if featured: artist = f"{artist} feat. {', '.join(featured)}" return TrackInfo( diff --git a/docs/changelog.rst b/docs/changelog.rst index 6c81a0d08..a5393e032 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,8 @@ Bug fixes: matching. :bug:`5189` - :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels. :bug:`5366` +- :doc:`plugins/discogs` Fixed issue with ignoring featured artists in the + extraartists field. For packagers: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 19428b125..562bda88f 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -451,67 +451,83 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" -@pytest.mark.parametrize("track, expected_artist", - [({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [{ - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "ARTIST feat. MUSICIAN" - ), - ({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [{ - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, { - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "ARTIST feat. PERFORMER, MUSICIAN" - ), - ({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "artists": [{ - "name": "NEW ARTIST", - "tracks": "", - "id": 11146}], - "extraartists": [{ - "name": "SOLOIST", - "role": "Featuring", - }, { - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, { - "name": "RANDOM", - "role": "Written-By", - }, { - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN" - )]) + +@pytest.mark.parametrize( + "track, expected_artist", + [ + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [ + { + "name": "MUSICIAN", + "role": "Featuring", + } + ], + }, + "ARTIST feat. MUSICIAN", + ), + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [ + { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, + { + "name": "MUSICIAN", + "role": "Featuring", + }, + ], + }, + "ARTIST feat. PERFORMER, MUSICIAN", + ), + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artists": [{"name": "NEW ARTIST", "tracks": "", "id": 11146}], + "extraartists": [ + { + "name": "SOLOIST", + "role": "Featuring", + }, + { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, + { + "name": "RANDOM", + "role": "Written-By", + }, + { + "name": "MUSICIAN", + "role": "Featuring", + }, + ], + }, + "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN", + ), + ], +) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_parse_featured_artists(track, expected_artist): - """ Tests the plugins ability to parse a featured artist. - Initial check with one featured artist, two featured artists, + """Tests the plugins ability to parse a featured artist. + Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) assert t.artist == expected_artist + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", [ From 61b632f2b428e7e2602b468526272775c83b6690 Mon Sep 17 00:00:00 2001 From: Finn Date: Tue, 23 Sep 2025 09:53:46 -0400 Subject: [PATCH 37/53] Add option to not write metadata --- beetsplug/convert.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 102976dd7..e72f8c75a 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -122,6 +122,7 @@ class ConvertPlugin(BeetsPlugin): "threads": os.cpu_count(), "format": "mp3", "id3v23": "inherit", + "write_metadata": True, "formats": { "aac": { "command": ( @@ -446,8 +447,9 @@ class ConvertPlugin(BeetsPlugin): if id3v23 == "inherit": id3v23 = None - # Write tags from the database to the converted file. - item.try_write(path=converted, id3v23=id3v23) + # Write tags from the database to the file if requested + if self.config["write_metadata"].get(bool): + item.try_write(path=converted, id3v23=id3v23) if keep_new: # If we're keeping the transcoded file, read it again (after From 98170f6c04e05f85ccb76eb6e1e741b6f16f4470 Mon Sep 17 00:00:00 2001 From: Multipixelone Date: Tue, 23 Sep 2025 19:07:13 -0400 Subject: [PATCH 38/53] add documentation for write_metadata option --- docs/changelog.rst | 1 + docs/plugins/convert.rst | 2 ++ 2 files changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index c2d6ea1cf..cb10c5810 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,7 @@ New features: - :doc:`plugins/lastgenre`: Add a ``--pretend`` option to preview genre changes without storing or writing them. +- :doc:`plugins/convert`: Add a config option to disable writing metadata to converted files. Bug fixes: diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index 8917422c5..5cc2ace3f 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -97,6 +97,8 @@ The available options are: - **embed**: Embed album art in converted items. Default: ``yes``. - **id3v23**: Can be used to override the global ``id3v23`` option. Default: ``inherit``. +- **write_metadata**: Can be used to disable writing metadata to converted files. Default: + ``true``. - **max_bitrate**: By default, the plugin does not transcode files that are already in the destination format. This option instead also transcodes files with high bitrates, even if they are already in the same format as the output. From 699e0a12723f65dd6cefefe75dff52dadc1b90ab Mon Sep 17 00:00:00 2001 From: Multipixelone Date: Wed, 24 Sep 2025 22:11:47 -0400 Subject: [PATCH 39/53] fixup! add documentation for write_metadata option --- docs/changelog.rst | 3 ++- docs/plugins/convert.rst | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index fd3d41f3f..092a1a5a0 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,7 +11,8 @@ New features: - :doc:`plugins/lastgenre`: Add a ``--pretend`` option to preview genre changes without storing or writing them. -- :doc:`plugins/convert`: Add a config option to disable writing metadata to converted files. +- :doc:`plugins/convert`: Add a config option to disable writing metadata to + converted files. - :doc:`plugins/discogs`: New config option `strip_disambiguation` to toggle stripping discogs numeric disambiguation on artist and label fields. diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index 5cc2ace3f..ecf60a85b 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -97,8 +97,8 @@ The available options are: - **embed**: Embed album art in converted items. Default: ``yes``. - **id3v23**: Can be used to override the global ``id3v23`` option. Default: ``inherit``. -- **write_metadata**: Can be used to disable writing metadata to converted files. Default: - ``true``. +- **write_metadata**: Can be used to disable writing metadata to converted + files. Default: ``true``. - **max_bitrate**: By default, the plugin does not transcode files that are already in the destination format. This option instead also transcodes files with high bitrates, even if they are already in the same format as the output. From 5c036728743743f5abe5fcc893e22c9df5457232 Mon Sep 17 00:00:00 2001 From: Henry Date: Mon, 22 Sep 2025 20:49:53 -0700 Subject: [PATCH 40/53] Test written, beginning fix --- beetsplug/discogs.py | 1 + test/plugins/test_discogs.py | 57 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index f22ea2274..caefbe64a 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -652,6 +652,7 @@ class DiscogsPlugin(MetadataSourcePlugin): ) artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) + featured = map(lambda artist: artist["name"] if artist["role"].find("Featuring") else "", track.get("extraartists", [])) return TrackInfo( title=title, track_id=track_id, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 92301380e..84573d337 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -451,6 +451,63 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" +@pytest.mark.parametrize("track, expected_artist", + [({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [{ + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "ARTIST feat. MUSICIAN" + ), + ({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [{ + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, { + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "ARTIST feat. PERFORMER & MUSICIAN" + ), + ({ + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artist": "NEW ARTIST", + "extraartists": [{ + "name": "SOLIST", + "role": "Featuring", + }, { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, { + "name": "RANDOM", + "role": "Written-By", + }, { + "name": "MUSICIAN", + "role": "Featuring", + }] + }, + "NEW ARTIST feat. SOLIST, PERFORMER & MUSICIAN" + )]) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_parse_featured_artists(track, expected_artist): + """ Tests the plugins ability to parse a featured artist. + Initial check with one featured artist, two featured artists, + and three. Ignores artists that are not listed as featured.""" + t = DiscogsPlugin().get_track_info(track, 1, 1) + assert t.artist == expected_artist @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", From 876c57c8b3eee4241b3af9e191a4f518fa2ca9b5 Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 23 Sep 2025 10:11:13 -0700 Subject: [PATCH 41/53] Featured artists extracted and appended, need to see if join needs to be variable --- beetsplug/discogs.py | 20 +++++++++++++++----- test/plugins/test_discogs.py | 13 ++++++++----- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index caefbe64a..43a501a22 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -339,7 +339,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # convenient `.tracklist` property, which will strip out useful artist # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. - tracks = self.get_tracks(result.data["tracklist"]) + tracks = self.get_tracks(result.data["tracklist"], artist, artist_id) # Extract information for the optional AlbumInfo fields, if possible. va = result.data["artists"][0].get("name", "").lower() == "various" @@ -446,7 +446,7 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def get_tracks(self, tracklist): + def get_tracks(self, tracklist, album_artist, album_artist_id): """Returns a list of TrackInfo objects for a discogs tracklist.""" try: clean_tracklist = self.coalesce_tracks(tracklist) @@ -471,7 +471,8 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions) + track_info = self.get_track_info(track, index, divisions, + album_artist, album_artist_id) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -638,7 +639,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions): + def get_track_info(self, track, index, divisions, album_artist, album_artist_id): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -650,9 +651,18 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist( track.get("artists", []), join_key="join" ) + # If no artist and artist is returned, set to match album artist + if not artist: + artist = album_artist + artist_id = album_artist_id artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) - featured = map(lambda artist: artist["name"] if artist["role"].find("Featuring") else "", track.get("extraartists", [])) + # Add featured artists + extraartists = track.get("extraartists", []) + featured = [ + artist["name"] for artist in extraartists if artist["role"].find("Featuring") != -1] + if featured: + artist = f"{artist} feat. {', '.join(featured)}" return TrackInfo( title=title, track_id=track_id, diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 84573d337..19428b125 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -477,16 +477,19 @@ class DGAlbumInfoTest(BeetsTestCase): "role": "Featuring", }] }, - "ARTIST feat. PERFORMER & MUSICIAN" + "ARTIST feat. PERFORMER, MUSICIAN" ), ({ "type_": "track", "title": "track", "position": "1", "duration": "5:00", - "artist": "NEW ARTIST", + "artists": [{ + "name": "NEW ARTIST", + "tracks": "", + "id": 11146}], "extraartists": [{ - "name": "SOLIST", + "name": "SOLOIST", "role": "Featuring", }, { "name": "PERFORMER", @@ -499,14 +502,14 @@ class DGAlbumInfoTest(BeetsTestCase): "role": "Featuring", }] }, - "NEW ARTIST feat. SOLIST, PERFORMER & MUSICIAN" + "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN" )]) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_parse_featured_artists(track, expected_artist): """ Tests the plugins ability to parse a featured artist. Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" - t = DiscogsPlugin().get_track_info(track, 1, 1) + t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) assert t.artist == expected_artist @pytest.mark.parametrize( From 43f2d423fa84c5d82092d69a39cdbec17e1a28bb Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 23 Sep 2025 11:05:48 -0700 Subject: [PATCH 42/53] testing, updated changelog --- beetsplug/discogs.py | 18 ++--- docs/changelog.rst | 2 + test/plugins/test_discogs.py | 126 ++++++++++++++++++++--------------- 3 files changed, 83 insertions(+), 63 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 43a501a22..c8ce43a1e 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -386,10 +386,6 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in tracks: track.media = media track.medium_total = mediums.count(track.medium) - if not track.artist: # get_track_info often fails to find artist - track.artist = artist - if not track.artist_id: - track.artist_id = artist_id # Discogs does not have track IDs. Invent our own IDs as proposed # in #2336. track.track_id = f"{album_id}-{track.track_alt}" @@ -471,8 +467,9 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info(track, index, divisions, - album_artist, album_artist_id) + track_info = self.get_track_info( + track, index, divisions, album_artist, album_artist_id + ) track_info.track_alt = track["position"] tracks.append(track_info) else: @@ -639,7 +636,9 @@ class DiscogsPlugin(MetadataSourcePlugin): return text return DISAMBIGUATION_RE.sub("", text) - def get_track_info(self, track, index, divisions, album_artist, album_artist_id): + def get_track_info( + self, track, index, divisions, album_artist, album_artist_id + ): """Returns a TrackInfo object for a discogs track.""" title = track["title"] if self.config["index_tracks"]: @@ -660,7 +659,10 @@ class DiscogsPlugin(MetadataSourcePlugin): # Add featured artists extraartists = track.get("extraartists", []) featured = [ - artist["name"] for artist in extraartists if artist["role"].find("Featuring") != -1] + artist["name"] + for artist in extraartists + if artist["role"].find("Featuring") != -1 + ] if featured: artist = f"{artist} feat. {', '.join(featured)}" return TrackInfo( diff --git a/docs/changelog.rst b/docs/changelog.rst index 092a1a5a0..1ae13303e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -27,6 +27,8 @@ Bug fixes: matching. :bug:`5189` - :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels. :bug:`5366` +- :doc:`plugins/discogs` Fixed issue with ignoring featured artists in the + extraartists field. For packagers: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 19428b125..562bda88f 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -451,67 +451,83 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" -@pytest.mark.parametrize("track, expected_artist", - [({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [{ - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "ARTIST feat. MUSICIAN" - ), - ({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [{ - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, { - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "ARTIST feat. PERFORMER, MUSICIAN" - ), - ({ - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "artists": [{ - "name": "NEW ARTIST", - "tracks": "", - "id": 11146}], - "extraartists": [{ - "name": "SOLOIST", - "role": "Featuring", - }, { - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, { - "name": "RANDOM", - "role": "Written-By", - }, { - "name": "MUSICIAN", - "role": "Featuring", - }] - }, - "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN" - )]) + +@pytest.mark.parametrize( + "track, expected_artist", + [ + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [ + { + "name": "MUSICIAN", + "role": "Featuring", + } + ], + }, + "ARTIST feat. MUSICIAN", + ), + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "extraartists": [ + { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, + { + "name": "MUSICIAN", + "role": "Featuring", + }, + ], + }, + "ARTIST feat. PERFORMER, MUSICIAN", + ), + ( + { + "type_": "track", + "title": "track", + "position": "1", + "duration": "5:00", + "artists": [{"name": "NEW ARTIST", "tracks": "", "id": 11146}], + "extraartists": [ + { + "name": "SOLOIST", + "role": "Featuring", + }, + { + "name": "PERFORMER", + "role": "Other Role, Featuring", + }, + { + "name": "RANDOM", + "role": "Written-By", + }, + { + "name": "MUSICIAN", + "role": "Featuring", + }, + ], + }, + "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN", + ), + ], +) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_parse_featured_artists(track, expected_artist): - """ Tests the plugins ability to parse a featured artist. - Initial check with one featured artist, two featured artists, + """Tests the plugins ability to parse a featured artist. + Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" t = DiscogsPlugin().get_track_info(track, 1, 1, "ARTIST", 2) assert t.artist == expected_artist + @pytest.mark.parametrize( "formats, expected_media, expected_albumtype", [ From b61306ea0d3df2b8644e932c292d27924b432ff6 Mon Sep 17 00:00:00 2001 From: Henry Date: Thu, 25 Sep 2025 08:39:38 -0700 Subject: [PATCH 43/53] Fixes, test improvement, rebase to master --- beetsplug/discogs.py | 4 ++-- docs/changelog.rst | 3 +-- test/plugins/test_discogs.py | 44 ++++++------------------------------ 3 files changed, 10 insertions(+), 41 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index c8ce43a1e..3dc962464 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -654,17 +654,17 @@ class DiscogsPlugin(MetadataSourcePlugin): if not artist: artist = album_artist artist_id = album_artist_id - artist = self.strip_disambiguation(artist) length = self.get_track_length(track["duration"]) # Add featured artists extraartists = track.get("extraartists", []) featured = [ artist["name"] for artist in extraartists - if artist["role"].find("Featuring") != -1 + if "Featuring" in artist["role"] ] if featured: artist = f"{artist} feat. {', '.join(featured)}" + artist = self.strip_disambiguation(artist) return TrackInfo( title=title, track_id=track_id, diff --git a/docs/changelog.rst b/docs/changelog.rst index 1ae13303e..34ead44af 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -15,6 +15,7 @@ New features: converted files. - :doc:`plugins/discogs`: New config option `strip_disambiguation` to toggle stripping discogs numeric disambiguation on artist and label fields. +- :doc:`plugins/discogs` Added support for featured artists. Bug fixes: @@ -27,8 +28,6 @@ Bug fixes: matching. :bug:`5189` - :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels. :bug:`5366` -- :doc:`plugins/discogs` Fixed issue with ignoring featured artists in the - extraartists field. For packagers: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 562bda88f..3652c0a54 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -450,6 +450,7 @@ class DGAlbumInfoTest(BeetsTestCase): assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" assert d.tracks[0].artist == "TEST ARTIST (5)" assert d.label == "LABEL NAME (5)" + config["discogs"]["strip_disambiguation"] = True @pytest.mark.parametrize( @@ -461,48 +462,17 @@ class DGAlbumInfoTest(BeetsTestCase): "title": "track", "position": "1", "duration": "5:00", - "extraartists": [ - { - "name": "MUSICIAN", - "role": "Featuring", - } + "artists": [ + {"name": "NEW ARTIST", "tracks": "", "id": 11146}, + {"name": "VOCALIST", "tracks": "", "id": 344, "join": "&"}, ], - }, - "ARTIST feat. MUSICIAN", - ), - ( - { - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "extraartists": [ - { - "name": "PERFORMER", - "role": "Other Role, Featuring", - }, - { - "name": "MUSICIAN", - "role": "Featuring", - }, - ], - }, - "ARTIST feat. PERFORMER, MUSICIAN", - ), - ( - { - "type_": "track", - "title": "track", - "position": "1", - "duration": "5:00", - "artists": [{"name": "NEW ARTIST", "tracks": "", "id": 11146}], "extraartists": [ { "name": "SOLOIST", "role": "Featuring", }, { - "name": "PERFORMER", + "name": "PERFORMER (1)", "role": "Other Role, Featuring", }, { @@ -511,11 +481,11 @@ class DGAlbumInfoTest(BeetsTestCase): }, { "name": "MUSICIAN", - "role": "Featuring", + "role": "Featuring [Uncredited]", }, ], }, - "NEW ARTIST feat. SOLOIST, PERFORMER, MUSICIAN", + "NEW ARTIST, VOCALIST feat. SOLOIST, PERFORMER, MUSICIAN", ), ], ) From a0a0a094d3ead2c373669037159bfd93aab67272 Mon Sep 17 00:00:00 2001 From: pSpitzner Date: Sat, 27 Sep 2025 13:06:12 +0200 Subject: [PATCH 44/53] Changed query from double to single quotes. --- beets/metadata_plugins.py | 2 +- docs/changelog.rst | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/beets/metadata_plugins.py b/beets/metadata_plugins.py index 0b8d81c95..56bf8124f 100644 --- a/beets/metadata_plugins.py +++ b/beets/metadata_plugins.py @@ -412,7 +412,7 @@ class SearchApiMetadataSourcePlugin( :return: Query string to be provided to the search API. """ - components = [query_string, *(f'{k}:"{v}"' for k, v in filters.items())] + components = [query_string, *(f"{k}:'{v}'" for k, v in filters.items())] query = " ".join(filter(None, components)) if self.config["search_query_ascii"].get(): diff --git a/docs/changelog.rst b/docs/changelog.rst index 092a1a5a0..ef7eb9ff8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -27,6 +27,8 @@ Bug fixes: matching. :bug:`5189` - :doc:`plugins/discogs` Fixed inconsistency in stripping disambiguation from artists but not labels. :bug:`5366` +- :doc:`plugins/spotify` Fixed an issue where candidate lookup would not find + matches due to query escaping (single vs double quotes). For packagers: From cc0024e089112863e2a8661de6a6e2b533b16964 Mon Sep 17 00:00:00 2001 From: pSpitzner Date: Sat, 27 Sep 2025 13:22:41 +0200 Subject: [PATCH 45/53] Spotify tests are now consistent with quote change --- test/plugins/test_spotify.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/plugins/test_spotify.py b/test/plugins/test_spotify.py index 86b5651b9..bc55485c6 100644 --- a/test/plugins/test_spotify.py +++ b/test/plugins/test_spotify.py @@ -82,8 +82,8 @@ class SpotifyPluginTest(PluginTestCase): params = _params(responses.calls[0].request.url) query = params["q"][0] assert "duifhjslkef" in query - assert 'artist:"ujydfsuihse"' in query - assert 'album:"lkajsdflakjsd"' in query + assert "artist:'ujydfsuihse'" in query + assert "album:'lkajsdflakjsd'" in query assert params["type"] == ["track"] @responses.activate @@ -117,8 +117,8 @@ class SpotifyPluginTest(PluginTestCase): params = _params(responses.calls[0].request.url) query = params["q"][0] assert "Happy" in query - assert 'artist:"Pharrell Williams"' in query - assert 'album:"Despicable Me 2"' in query + assert "artist:'Pharrell Williams'" in query + assert "album:'Despicable Me 2'" in query assert params["type"] == ["track"] @responses.activate @@ -233,8 +233,8 @@ class SpotifyPluginTest(PluginTestCase): params = _params(responses.calls[0].request.url) query = params["q"][0] assert item.title in query - assert f'artist:"{item.albumartist}"' in query - assert f'album:"{item.album}"' in query + assert f"artist:'{item.albumartist}'" in query + assert f"album:'{item.album}'" in query assert not query.isascii() # Is not found in the library if ascii encoding is enabled From c34b2a00a4d25acf3947403777c8625646a78577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 23 Sep 2025 15:03:17 +0100 Subject: [PATCH 46/53] Fix plugin loading --- beets/plugins.py | 11 +++++++++-- docs/changelog.rst | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index d8d465183..c0dd12e5b 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -22,6 +22,7 @@ import re import sys from collections import defaultdict from functools import wraps +from importlib import import_module from pathlib import Path from types import GenericAlias from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar @@ -365,11 +366,11 @@ def _get_plugin(name: str) -> BeetsPlugin | None: """ try: try: - namespace = __import__(f"{PLUGIN_NAMESPACE}.{name}", None, None) + namespace = import_module(f"{PLUGIN_NAMESPACE}.{name}") except Exception as exc: raise PluginImportError(name) from exc - for obj in getattr(namespace, name).__dict__.values(): + for obj in namespace.__dict__.values(): if ( inspect.isclass(obj) and not isinstance( @@ -378,6 +379,12 @@ def _get_plugin(name: str) -> BeetsPlugin | None: and issubclass(obj, BeetsPlugin) and obj != BeetsPlugin and not inspect.isabstract(obj) + # Only consider this plugin's module or submodules to avoid + # conflicts when plugins import other BeetsPlugin classes + and ( + obj.__module__ == namespace.__name__ + or obj.__module__.startswith(f"{namespace.__name__}.") + ) ): return obj() diff --git a/docs/changelog.rst b/docs/changelog.rst index cbcfe73f0..52e935445 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,6 +32,8 @@ Bug fixes: extraartists field. - :doc:`plugins/spotify` Fixed an issue where candidate lookup would not find matches due to query escaping (single vs double quotes). +- :doc:`plugins/chroma` :doc:`plugins/bpsync` Fix plugin loading issue caused by + an import of another :class:`beets.plugins.BeetsPlugin` class. :bug:`6033` For packagers: From 7954671c737cbcd462f8b6dc22a062b52d260184 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 23 Sep 2025 15:45:24 +0100 Subject: [PATCH 47/53] Mock DummyPlugin properly --- test/test_logging.py | 65 ++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/test/test_logging.py b/test/test_logging.py index aee0bd61b..b751dd70e 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -5,9 +5,10 @@ import sys import threading import unittest from io import StringIO +from types import ModuleType +from unittest.mock import patch import beets.logging as blog -import beetsplug from beets import plugins, ui from beets.test import _common, helper from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin @@ -47,36 +48,46 @@ class LoggingTest(unittest.TestCase): assert stream.getvalue(), "foo oof baz" +class DummyModule(ModuleType): + class DummyPlugin(plugins.BeetsPlugin): + def __init__(self): + plugins.BeetsPlugin.__init__(self, "dummy") + self.import_stages = [self.import_stage] + self.register_listener("dummy_event", self.listener) + + def log_all(self, name): + self._log.debug("debug {}", name) + self._log.info("info {}", name) + self._log.warning("warning {}", name) + + def commands(self): + cmd = ui.Subcommand("dummy") + cmd.func = lambda _, __, ___: self.log_all("cmd") + return (cmd,) + + def import_stage(self, session, task): + self.log_all("import_stage") + + def listener(self): + self.log_all("listener") + + def __init__(self, *_, **__): + module_name = "beetsplug.dummy" + super().__init__(module_name) + self.DummyPlugin.__module__ = module_name + self.DummyPlugin = self.DummyPlugin + + class LoggingLevelTest(AsIsImporterMixin, PluginMixin, ImportTestCase): plugin = "dummy" - class DummyModule: - class DummyPlugin(plugins.BeetsPlugin): - def __init__(self): - plugins.BeetsPlugin.__init__(self, "dummy") - self.import_stages = [self.import_stage] - self.register_listener("dummy_event", self.listener) + @classmethod + def setUpClass(cls): + patcher = patch.dict(sys.modules, {"beetsplug.dummy": DummyModule()}) + patcher.start() + cls.addClassCleanup(patcher.stop) - def log_all(self, name): - self._log.debug("debug {}", name) - self._log.info("info {}", name) - self._log.warning("warning {}", name) - - def commands(self): - cmd = ui.Subcommand("dummy") - cmd.func = lambda _, __, ___: self.log_all("cmd") - return (cmd,) - - def import_stage(self, session, task): - self.log_all("import_stage") - - def listener(self): - self.log_all("listener") - - def setUp(self): - sys.modules["beetsplug.dummy"] = self.DummyModule - beetsplug.dummy = self.DummyModule - super().setUp() + super().setUpClass() def test_command_level0(self): self.config["verbose"] = 0 From 461bc049a058a233afe02c5ae3ba4a9571651e75 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Wed, 17 Sep 2025 11:42:07 +0200 Subject: [PATCH 48/53] Enhanced custom logger typing and logging tests --- beets/logging.py | 47 ++++++++++++++++++++++++++++++-------------- test/test_logging.py | 45 +++++++++++++++++++++++++++++------------- 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index fd8b1962f..becfff86f 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -20,6 +20,9 @@ use {}-style formatting and can interpolate keywords arguments to the logging calls (`debug`, `info`, etc). """ +from __future__ import annotations + +import logging import threading from copy import copy from logging import ( @@ -34,6 +37,9 @@ from logging import ( NullHandler, StreamHandler, ) +from typing import TYPE_CHECKING, Any, Mapping, TypeVar + +from typing_extensions import ParamSpec __all__ = [ "DEBUG", @@ -49,8 +55,14 @@ __all__ = [ "getLogger", ] +if TYPE_CHECKING: + T = TypeVar("T") -def logsafe(val): + +P = ParamSpec("P") + + +def _logsafe(val: T) -> str | T: """Coerce `bytes` to `str` to avoid crashes solely due to logging. This is particularly relevant for bytestring paths. Much of our code @@ -83,40 +95,45 @@ class StrFormatLogger(Logger): """ class _LogMessage: - def __init__(self, msg, args, kwargs): + def __init__( + self, + msg: str, + args: logging._ArgsType, + kwargs: dict[str, Any], + ): self.msg = msg self.args = args self.kwargs = kwargs def __str__(self): - args = [logsafe(a) for a in self.args] - kwargs = {k: logsafe(v) for (k, v) in self.kwargs.items()} + args = [_logsafe(a) for a in self.args] + kwargs = {k: _logsafe(v) for (k, v) in self.kwargs.items()} return self.msg.format(*args, **kwargs) def _log( self, - level, - msg, - args, - exc_info=None, - extra=None, - stack_info=False, + level: int, + msg: object, + args: logging._ArgsType = (), + exc_info: logging._ExcInfoType = None, + extra: Mapping[str, Any] | None = None, + stack_info: bool = False, + stacklevel: int = 1, **kwargs, ): """Log msg.format(*args, **kwargs)""" - m = self._LogMessage(msg, args, kwargs) - stacklevel = kwargs.pop("stacklevel", 1) - stacklevel = {"stacklevel": stacklevel} + if isinstance(msg, str): + msg = self._LogMessage(msg, args, kwargs) return super()._log( level, - m, + msg, (), exc_info=exc_info, extra=extra, stack_info=stack_info, - **stacklevel, + stacklevel=stacklevel, ) diff --git a/test/test_logging.py b/test/test_logging.py index b751dd70e..4a62e3a3b 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -3,10 +3,9 @@ import logging as log import sys import threading -import unittest -from io import StringIO from types import ModuleType -from unittest.mock import patch + +import pytest import beets.logging as blog from beets import plugins, ui @@ -14,8 +13,10 @@ from beets.test import _common, helper from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin -class LoggingTest(unittest.TestCase): - def test_logging_management(self): +class TestStrFormatLogger: + """Tests for the custom str-formatting logger.""" + + def test_logger_creation(self): l1 = log.getLogger("foo123") l2 = blog.getLogger("foo123") assert l1 == l2 @@ -35,17 +36,33 @@ class LoggingTest(unittest.TestCase): l6 = blog.getLogger() assert l1 != l6 - def test_str_format_logging(self): - logger = blog.getLogger("baz123") - stream = StringIO() - handler = log.StreamHandler(stream) + @pytest.mark.parametrize( + "level", [log.DEBUG, log.INFO, log.WARNING, log.ERROR] + ) + @pytest.mark.parametrize( + "msg, args, kwargs, expected", + [ + ("foo {} bar {}", ("oof", "baz"), {}, "foo oof bar baz"), + ( + "foo {bar} baz {foo}", + (), + {"foo": "oof", "bar": "baz"}, + "foo baz baz oof", + ), + ("no args", (), {}, "no args"), + ("foo {} bar {baz}", ("oof",), {"baz": "baz"}, "foo oof bar baz"), + ], + ) + def test_str_format_logging( + self, level, msg, args, kwargs, expected, caplog + ): + logger = blog.getLogger("test_logger") + logger.setLevel(level) - logger.addHandler(handler) - logger.propagate = False + with caplog.at_level(level, logger="test_logger"): + logger.log(level, msg, *args, **kwargs) - logger.warning("foo {} {bar}", "oof", bar="baz") - handler.flush() - assert stream.getvalue(), "foo oof baz" + assert str(caplog.records[0].msg) == expected class DummyModule(ModuleType): From f637e5efbb29c47a3d1800e5c9233ecdea74a644 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Fri, 19 Sep 2025 15:23:36 +0200 Subject: [PATCH 49/53] Added overload to getLogger function. Added changelog entry and added myself to codeowners file. --- .github/CODEOWNERS | 3 +++ beets/logging.py | 12 ++++++++---- docs/changelog.rst | 6 ++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 767509c9a..bb888d520 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,2 +1,5 @@ # assign the entire repo to the maintainers team * @beetbox/maintainers + +# Specific ownerships: +/beets/metadata_plugins.py @semohr \ No newline at end of file diff --git a/beets/logging.py b/beets/logging.py index becfff86f..29357c0f0 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -35,9 +35,10 @@ from logging import ( Handler, Logger, NullHandler, + RootLogger, StreamHandler, ) -from typing import TYPE_CHECKING, Any, Mapping, TypeVar +from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload from typing_extensions import ParamSpec @@ -173,9 +174,12 @@ my_manager = copy(Logger.manager) my_manager.loggerClass = BeetsLogger -# Override the `getLogger` to use our machinery. -def getLogger(name=None): # noqa +@overload +def getLogger(name: str) -> BeetsLogger: ... +@overload +def getLogger(name: None = ...) -> RootLogger: ... +def getLogger(name=None) -> BeetsLogger | RootLogger: # noqa: N802 if name: - return my_manager.getLogger(name) + return my_manager.getLogger(name) # type: ignore[return-value] else: return Logger.root diff --git a/docs/changelog.rst b/docs/changelog.rst index 52e935445..38037955e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -51,6 +51,12 @@ Other changes: - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific disambiguation stripping. +For developers and plugin authors: + +- Typing improvements in ``beets/logging.py``: ``getLogger`` now returns + ``BeetsLogger`` when called with a name, or ``RootLogger`` when called without + a name. + 2.4.0 (September 13, 2025) -------------------------- From b2fc007480f50278c44eb5e7c2ea13181822a0bf Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Fri, 19 Sep 2025 15:35:51 +0200 Subject: [PATCH 50/53] Fixed plugin typehints: use actual logger class. --- beetsplug/fetchart.py | 2 +- beetsplug/lyrics.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 54c065da4..37e7426f6 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -36,10 +36,10 @@ from beets.util.config import sanitize_pairs if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence - from logging import Logger from beets.importer import ImportSession, ImportTask from beets.library import Album, Library + from beets.logging import BeetsLogger as Logger try: from bs4 import BeautifulSoup, Tag diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index f492ab3cc..d245d6a14 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -42,10 +42,9 @@ from beets.autotag.distance import string_dist from beets.util.config import sanitize_choices if TYPE_CHECKING: - from logging import Logger - from beets.importer import ImportTask from beets.library import Item, Library + from beets.logging import BeetsLogger as Logger from ._typing import ( GeniusAPI, @@ -186,7 +185,7 @@ def slug(text: str) -> str: class RequestHandler: - _log: beets.logging.Logger + _log: Logger def debug(self, message: str, *args) -> None: """Log a debug message with the class name.""" From caebf185f11b24ab73ccbf0699e5078bccef6bcf Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Fri, 19 Sep 2025 15:39:43 +0200 Subject: [PATCH 51/53] Removed unused ParamSpec and added a consistency check in the tests. --- beets/logging.py | 5 ----- test/test_logging.py | 1 + 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index 29357c0f0..086a590a0 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -40,8 +40,6 @@ from logging import ( ) from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload -from typing_extensions import ParamSpec - __all__ = [ "DEBUG", "INFO", @@ -60,9 +58,6 @@ if TYPE_CHECKING: T = TypeVar("T") -P = ParamSpec("P") - - def _logsafe(val: T) -> str | T: """Coerce `bytes` to `str` to avoid crashes solely due to logging. diff --git a/test/test_logging.py b/test/test_logging.py index 4a62e3a3b..f4dda331e 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -62,6 +62,7 @@ class TestStrFormatLogger: with caplog.at_level(level, logger="test_logger"): logger.log(level, msg, *args, **kwargs) + assert caplog.records, "No log records were captured" assert str(caplog.records[0].msg) == expected From 837295e2502a220fea0798f8352756d85627ba07 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Sun, 21 Sep 2025 22:27:24 +0200 Subject: [PATCH 52/53] Added typehints from typeshed and removed default argument. --- beets/logging.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index 086a590a0..64d6c50b1 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -22,7 +22,6 @@ calls (`debug`, `info`, etc). from __future__ import annotations -import logging import threading from copy import copy from logging import ( @@ -40,6 +39,8 @@ from logging import ( ) from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload +from typing_extensions import TypeAlias + __all__ = [ "DEBUG", "INFO", @@ -56,6 +57,15 @@ __all__ = [ if TYPE_CHECKING: T = TypeVar("T") + from types import TracebackType + + # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi + _SysExcInfoType: TypeAlias = ( + tuple[type[BaseException], BaseException, TracebackType | None] + | tuple[None, None, None] + ) + _ExcInfoType: TypeAlias = None | bool | _SysExcInfoType | BaseException + _ArgsType: TypeAlias = tuple[object, ...] | Mapping[str, object] def _logsafe(val: T) -> str | T: @@ -94,7 +104,7 @@ class StrFormatLogger(Logger): def __init__( self, msg: str, - args: logging._ArgsType, + args: _ArgsType, kwargs: dict[str, Any], ): self.msg = msg @@ -110,8 +120,8 @@ class StrFormatLogger(Logger): self, level: int, msg: object, - args: logging._ArgsType = (), - exc_info: logging._ExcInfoType = None, + args: _ArgsType, + exc_info: _ExcInfoType = None, extra: Mapping[str, Any] | None = None, stack_info: bool = False, stacklevel: int = 1, From 89c2e10680b56873a07738b0a745205391df7425 Mon Sep 17 00:00:00 2001 From: Sebastian Mohr Date: Sun, 21 Sep 2025 22:29:55 +0200 Subject: [PATCH 53/53] Removed typealias, worked locally with mypy but does seem to cause issues with the ci. Also python 3.9 requires unions here... --- beets/logging.py | 16 +++++++--------- test/test_logging.py | 1 + 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/beets/logging.py b/beets/logging.py index 64d6c50b1..3ed5e5a84 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -37,9 +37,7 @@ from logging import ( RootLogger, StreamHandler, ) -from typing import TYPE_CHECKING, Any, Mapping, TypeVar, overload - -from typing_extensions import TypeAlias +from typing import TYPE_CHECKING, Any, Mapping, TypeVar, Union, overload __all__ = [ "DEBUG", @@ -60,12 +58,12 @@ if TYPE_CHECKING: from types import TracebackType # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi - _SysExcInfoType: TypeAlias = ( - tuple[type[BaseException], BaseException, TracebackType | None] - | tuple[None, None, None] - ) - _ExcInfoType: TypeAlias = None | bool | _SysExcInfoType | BaseException - _ArgsType: TypeAlias = tuple[object, ...] | Mapping[str, object] + _SysExcInfoType = Union[ + tuple[type[BaseException], BaseException, Union[TracebackType, None]], + tuple[None, None, None], + ] + _ExcInfoType = Union[None, bool, _SysExcInfoType, BaseException] + _ArgsType = Union[tuple[object, ...], Mapping[str, object]] def _logsafe(val: T) -> str | T: diff --git a/test/test_logging.py b/test/test_logging.py index f4dda331e..48f9cbfd8 100644 --- a/test/test_logging.py +++ b/test/test_logging.py @@ -4,6 +4,7 @@ import logging as log import sys import threading from types import ModuleType +from unittest.mock import patch import pytest