From a4249af70082a3dd4b5f8cd3a744024e4e470a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 15 Jan 2026 21:02:21 +0000 Subject: [PATCH 01/60] Patch sys.stdin and sys.stdout in tests --- beets/test/_common.py | 8 -------- beets/test/helper.py | 12 ++++++------ test/ui/commands/test_completion.py | 1 - 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/beets/test/_common.py b/beets/test/_common.py index 487f7c442..10083c8cb 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -190,14 +190,6 @@ class DummyIO: def readcount(self): return self.stdin.reads - def install(self): - sys.stdin = self.stdin - sys.stdout = self.stdout - - def restore(self): - sys.stdin = sys.__stdin__ - sys.stdout = sys.__stdout__ - # Utility. diff --git a/beets/test/helper.py b/beets/test/helper.py index 6eba85b1b..0f1037102 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -168,13 +168,14 @@ class IOMixin: def io(self) -> _common.DummyIO: return _common.DummyIO() - def setUp(self): + def setUp(self) -> None: super().setUp() - self.io.install() - def tearDown(self): - super().tearDown() - self.io.restore() + patcher = patch.multiple( + "sys", stdin=self.io.stdin, stdout=self.io.stdout + ) + patcher.start() + self.addCleanup(patcher.stop) class TestHelper(ConfigMixin): @@ -761,7 +762,6 @@ class TerminalImportMixin(IOMixin, ImportHelper): io: _common.DummyIO def _get_import_session(self, import_dir: bytes) -> importer.ImportSession: - self.io.install() return TerminalImportSessionFixture( self.lib, loghandler=None, diff --git a/test/ui/commands/test_completion.py b/test/ui/commands/test_completion.py index ee2881a0e..992ed58c8 100644 --- a/test/ui/commands/test_completion.py +++ b/test/ui/commands/test_completion.py @@ -49,7 +49,6 @@ class CompletionTest(IOMixin, TestPluginTestCase): # Load completion script. self.run_command("completion", lib=None) completion_script = self.io.getoutput().encode("utf-8") - self.io.restore() tester.stdin.writelines(completion_script.splitlines(True)) # Load test suite. From 1c87da2c059c1c48c4f935af8124a0750105b081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 15 Jan 2026 21:11:16 +0000 Subject: [PATCH 02/60] Fix test failures --- test/plugins/test_ftintitle.py | 4 +++- test/ui/test_ui.py | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index aff4dda18..560f44402 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -454,7 +454,9 @@ def test_custom_words( assert ftintitle.contains_feat(given, custom_words) is expected -def test_album_template_value(): +def test_album_template_value(config): + config["ftintitle"]["custom_words"] = [] + album = Album() album["albumartist"] = "Foo ft. Bar" assert ftintitle._album_artist_no_feat(album) == "Foo" diff --git a/test/ui/test_ui.py b/test/ui/test_ui.py index a0bf2e598..f96d2c76a 100644 --- a/test/ui/test_ui.py +++ b/test/ui/test_ui.py @@ -71,11 +71,11 @@ class TestPluginTestCase(PluginTestCase): plugin = "test" def setUp(self): + self.config["pluginpath"] = [_common.PLUGINPATH] super().setUp() - config["pluginpath"] = [_common.PLUGINPATH] -class ConfigTest(TestPluginTestCase): +class ConfigTest(IOMixin, TestPluginTestCase): def setUp(self): super().setUp() @@ -162,6 +162,7 @@ class ConfigTest(TestPluginTestCase): with self.write_config_file() as config: config.write("library: /xxx/yyy/not/a/real/path") + self.io.addinput("n") with pytest.raises(ui.UserError): self.run_command("test", lib=None) From 6a65f1377632bd4982a44d343621f138f30f5442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 16 Jan 2026 01:16:39 +0000 Subject: [PATCH 03/60] Replace custom stdio mocks with pytest io fixture Create a centralised pytest fixture to provide controllable stdin and captured stdout in all tests. Simplify DummyIO/DummyIn and remove the custom DummyOut implementation and make use of pytest builtin fixtures. Create a centralised pytest fixture to provide controllable stdin and captured stdout that can be applied to any tests, regardless whether they are based on pytest or unittest. * `io` fixture can be used as a fixture in pytest-based tests * `IOMixin` can be used to attach `io` attribute to any test class, including `unittest.TestCase` --- beets/test/_common.py | 88 ++++++++++++++------------------- beets/test/helper.py | 17 ++----- test/conftest.py | 22 +++++++++ test/ui/commands/test_fields.py | 2 +- 4 files changed, 62 insertions(+), 67 deletions(-) diff --git a/beets/test/_common.py b/beets/test/_common.py index 10083c8cb..4de47c337 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -14,10 +14,13 @@ """Some common functionality for beets' test cases.""" +from __future__ import annotations + import os import sys import unittest from contextlib import contextmanager +from typing import TYPE_CHECKING import beets import beets.library @@ -28,6 +31,9 @@ from beets import importer, logging, util from beets.ui import commands from beets.util import syspath +if TYPE_CHECKING: + import pytest + beetsplug.__path__ = [ os.path.abspath( os.path.join( @@ -118,77 +124,55 @@ def import_session(lib=None, loghandler=None, paths=[], query=[], cli=False): # Mock I/O. -class InputError(Exception): - def __init__(self, output=None): - self.output = output - - def __str__(self): - msg = "Attempt to read with no input provided." - if self.output is not None: - msg += f" Output: {self.output!r}" - return msg - - -class DummyOut: - encoding = "utf-8" - - def __init__(self): - self.buf = [] - - def write(self, s): - self.buf.append(s) - - def get(self): - return "".join(self.buf) - - def flush(self): - self.clear() - - def clear(self): - self.buf = [] +class InputError(IOError): + def __str__(self) -> str: + return "Attempt to read with no input provided." class DummyIn: encoding = "utf-8" - def __init__(self, out=None): - self.buf = [] - self.reads = 0 - self.out = out + def __init__(self) -> None: + self.buf: list[str] = [] - def add(self, s): + def add(self, s: str) -> None: self.buf.append(f"{s}\n") - def close(self): + def close(self) -> None: pass - def readline(self): + def readline(self) -> str: if not self.buf: - if self.out: - raise InputError(self.out.get()) - else: - raise InputError() - self.reads += 1 + raise InputError + return self.buf.pop(0) class DummyIO: - """Mocks input and output streams for testing UI code.""" + """Test helper that manages standard input and output.""" - def __init__(self): - self.stdout = DummyOut() - self.stdin = DummyIn(self.stdout) + def __init__( + self, + monkeypatch: pytest.MonkeyPatch, + capteesys: pytest.CaptureFixture[str], + ) -> None: + self._capteesys = capteesys + self.stdin = DummyIn() - def addinput(self, s): - self.stdin.add(s) + monkeypatch.setattr("sys.stdin", self.stdin) - def getoutput(self): - res = self.stdout.get() - self.stdout.clear() - return res + def addinput(self, text: str) -> None: + """Simulate user typing into stdin.""" + self.stdin.add(text) - def readcount(self): - return self.stdin.reads + def getoutput(self) -> str: + """Get the standard output captured so far. + + Note: it clears the internal buffer, so subsequent calls will only + return *new* output. + """ + # Using capteesys allows you to see output in the console if the test fails + return self._capteesys.readouterr().out # Utility. diff --git a/beets/test/helper.py b/beets/test/helper.py index 0f1037102..6454bd0b4 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -44,6 +44,7 @@ from tempfile import gettempdir, mkdtemp, mkstemp from typing import Any, ClassVar from unittest.mock import patch +import pytest import responses from mediafile import Image, MediaFile @@ -163,19 +164,9 @@ NEEDS_REFLINK = unittest.skipUnless( ) +@pytest.mark.usefixtures("io") class IOMixin: - @cached_property - def io(self) -> _common.DummyIO: - return _common.DummyIO() - - def setUp(self) -> None: - super().setUp() - - patcher = patch.multiple( - "sys", stdin=self.io.stdin, stdout=self.io.stdout - ) - patcher.start() - self.addCleanup(patcher.stop) + io: _common.DummyIO class TestHelper(ConfigMixin): @@ -759,8 +750,6 @@ class TerminalImportSessionFixture(TerminalImportSession): class TerminalImportMixin(IOMixin, ImportHelper): """Provides_a terminal importer for the import session.""" - io: _common.DummyIO - def _get_import_session(self, import_dir: bytes) -> importer.ImportSession: return TerminalImportSessionFixture( self.lib, diff --git a/test/conftest.py b/test/conftest.py index 059526d2f..b35083641 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -5,6 +5,7 @@ import pytest from beets.autotag.distance import Distance from beets.dbcore.query import Query +from beets.test._common import DummyIO from beets.test.helper import ConfigMixin from beets.util import cached_classproperty @@ -60,3 +61,24 @@ def clear_cached_classproperty(): def config(): """Provide a fresh beets configuration for a module, when requested.""" return ConfigMixin().config + + +@pytest.fixture +def io( + request: pytest.FixtureRequest, + monkeypatch: pytest.MonkeyPatch, + capteesys: pytest.CaptureFixture[str], +) -> DummyIO: + """Fixture for tests that need controllable stdin and captured stdout. + + This fixture builds a per-test ``DummyIO`` helper and exposes it to the + test. When used on a test class, it attaches the helper as ``self.io`` + attribute to make it available to all test methods, including + ``unittest.TestCase``-based ones. + """ + io = DummyIO(monkeypatch, capteesys) + + if request.instance: + request.instance.io = io + + return io diff --git a/test/ui/commands/test_fields.py b/test/ui/commands/test_fields.py index 0eaaa9ceb..98d4809c9 100644 --- a/test/ui/commands/test_fields.py +++ b/test/ui/commands/test_fields.py @@ -16,7 +16,7 @@ class FieldsTest(IOMixin, ItemInDBTestCase): items = library.Item.all_keys() albums = library.Album.all_keys() - output = self.io.stdout.get().split() + output = self.io.getoutput().split() self.remove_keys(items, output) self.remove_keys(albums, output) From fea789bb59ab4b88d09c464dd48656cde17644d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 16 Jan 2026 01:25:14 +0000 Subject: [PATCH 04/60] Replace control_stdin with io.addinput --- beets/test/helper.py | 20 +------- test/plugins/test_bpd.py | 2 +- test/plugins/test_convert.py | 76 +++++++++++++++---------------- test/plugins/test_edit.py | 21 +++++---- test/plugins/test_importsource.py | 22 ++++----- test/plugins/test_mbsubmit.py | 17 ++++--- test/plugins/test_play.py | 20 ++++---- test/plugins/test_zero.py | 47 ++++++++----------- test/test_plugins.py | 9 ++-- test/ui/commands/test_modify.py | 21 +++++---- test/ui/test_ui_init.py | 28 ++++++------ 11 files changed, 134 insertions(+), 149 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 6454bd0b4..879ff5f6f 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -15,8 +15,8 @@ """This module includes various helpers that provide fixtures, capture information or mock the environment. -- The `control_stdin` and `capture_stdout` context managers allow one to - interact with the user interface. +- `capture_stdout` context managers allow one to interact with the user + interface. - `has_program` checks the presence of a command on the system. @@ -84,22 +84,6 @@ def capture_log(logger="beets"): log.removeHandler(capture) -@contextmanager -def control_stdin(input=None): - """Sends ``input`` to stdin. - - >>> with control_stdin('yes'): - ... input() - 'yes' - """ - org = sys.stdin - sys.stdin = StringIO(input) - try: - yield sys.stdin - finally: - sys.stdin = org - - @contextmanager def capture_stdout(): """Save stdout in a StringIO. diff --git a/test/plugins/test_bpd.py b/test/plugins/test_bpd.py index 157569bbe..81e088067 100644 --- a/test/plugins/test_bpd.py +++ b/test/plugins/test_bpd.py @@ -32,7 +32,7 @@ import yaml from beets.test.helper import PluginTestCase from beets.util import bluelet -bpd = pytest.importorskip("beetsplug.bpd") +bpd = pytest.importorskip("beetsplug.bpd", exc_type=ImportError) class CommandParseTest(unittest.TestCase): diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 2a1a3b94d..de2650617 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -30,8 +30,8 @@ from beets.test.helper import ( AsIsImporterMixin, ImportHelper, PluginTestCase, + IOMixin, capture_log, - control_stdin, ) from beetsplug import convert @@ -66,7 +66,7 @@ class ConvertMixin: return path.read_bytes().endswith(tag.encode("utf-8")) -class ConvertTestCase(ConvertMixin, PluginTestCase): +class ConvertTestCase(IOMixin, ConvertMixin, PluginTestCase): db_on_disk = True plugin = "convert" @@ -157,8 +157,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): } def test_convert(self): - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert self.file_endswith(self.converted_mp3, "mp3") def test_convert_with_auto_confirmation(self): @@ -166,22 +166,22 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): assert self.file_endswith(self.converted_mp3, "mp3") def test_reject_confirmation(self): - with control_stdin("n"): - self.run_convert() + self.io.addinput("n") + self.run_convert() assert not self.converted_mp3.exists() def test_convert_keep_new(self): assert os.path.splitext(self.item.path)[1] == b".ogg" - with control_stdin("y"): - self.run_convert("--keep-new") + self.io.addinput("y") + self.run_convert("--keep-new") self.item.load() assert os.path.splitext(self.item.path)[1] == b".mp3" def test_format_option(self): - with control_stdin("y"): - self.run_convert("--format", "opus") + self.io.addinput("y") + self.run_convert("--format", "opus") assert self.file_endswith(self.convert_dest / "converted.ops", "opus") def test_embed_album_art(self): @@ -192,8 +192,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): with open(os.path.join(image_path), "rb") as f: image_data = f.read() - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() mediafile = MediaFile(self.converted_mp3) assert mediafile.images[0].data == image_data @@ -215,26 +215,26 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): def test_no_transcode_when_maxbr_set_high_and_different_formats(self): self.config["convert"]["max_bitrate"] = 5000 - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert self.file_endswith(self.converted_mp3, "mp3") def test_transcode_when_maxbr_set_low_and_different_formats(self): self.config["convert"]["max_bitrate"] = 5 - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert self.file_endswith(self.converted_mp3, "mp3") def test_transcode_when_maxbr_set_to_none_and_different_formats(self): - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert self.file_endswith(self.converted_mp3, "mp3") def test_no_transcode_when_maxbr_set_high_and_same_formats(self): self.config["convert"]["max_bitrate"] = 5000 self.config["convert"]["format"] = "ogg" - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert not self.file_endswith( self.convert_dest / "converted.ogg", "ogg" ) @@ -243,8 +243,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): self.config["convert"]["max_bitrate"] = 5000 self.config["convert"]["format"] = "ogg" - with control_stdin("y"): - self.run_convert("--force") + self.io.addinput("y") + self.run_convert("--force") converted = self.convert_dest / "converted.ogg" assert self.file_endswith(converted, "ogg") @@ -252,21 +252,21 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): def test_transcode_when_maxbr_set_low_and_same_formats(self): self.config["convert"]["max_bitrate"] = 5 self.config["convert"]["format"] = "ogg" - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert self.file_endswith(self.convert_dest / "converted.ogg", "ogg") def test_transcode_when_maxbr_set_to_none_and_same_formats(self): self.config["convert"]["format"] = "ogg" - with control_stdin("y"): - self.run_convert() + self.io.addinput("y") + self.run_convert() assert not self.file_endswith( self.convert_dest / "converted.ogg", "ogg" ) def test_playlist(self): - with control_stdin("y"): - self.run_convert("--playlist", "playlist.m3u8") + self.io.addinput("y") + self.run_convert("--playlist", "playlist.m3u8") assert (self.convert_dest / "playlist.m3u8").exists() def test_playlist_pretend(self): @@ -282,8 +282,8 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand): [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item, "--format", "opus", "--force") + self.io.addinput("y") + self.run_convert_path(item, "--format", "opus", "--force") converted = self.convert_dest / "converted.ops" assert self.file_endswith(converted, "opus") @@ -309,23 +309,23 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): def test_transcode_from_lossless(self): [item] = self.add_item_fixtures(ext="flac") - with control_stdin("y"): - self.run_convert_path(item) + self.io.addinput("y") + self.run_convert_path(item) converted = self.convert_dest / "converted.mp3" assert self.file_endswith(converted, "mp3") def test_transcode_from_lossy(self): self.config["convert"]["never_convert_lossy_files"] = False [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item) + self.io.addinput("y") + self.run_convert_path(item) converted = self.convert_dest / "converted.mp3" assert self.file_endswith(converted, "mp3") def test_transcode_from_lossy_prevented(self): [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item) + self.io.addinput("y") + self.run_convert_path(item) converted = self.convert_dest / "converted.ogg" assert not self.file_endswith(converted, "mp3") @@ -336,8 +336,8 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): } [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item, "--format", "opus", "--force") + self.io.addinput("y") + self.run_convert_path(item, "--format", "opus", "--force") converted = self.convert_dest / "converted.ops" assert self.file_endswith(converted, "opus") diff --git a/test/plugins/test_edit.py b/test/plugins/test_edit.py index 564b2ff1a..94ab34a03 100644 --- a/test/plugins/test_edit.py +++ b/test/plugins/test_edit.py @@ -17,15 +17,16 @@ from typing import ClassVar from unittest.mock import patch from beets.dbcore.query import TrueQuery +from beets.importer import Action from beets.library import Item from beets.test import _common from beets.test.helper import ( AutotagImportTestCase, AutotagStub, BeetsTestCase, + IOMixin, PluginMixin, TerminalImportMixin, - control_stdin, ) @@ -73,7 +74,7 @@ class ModifyFileMocker: f.write(contents) -class EditMixin(PluginMixin): +class EditMixin(IOMixin, PluginMixin): """Helper containing some common functionality used for the Edit tests.""" plugin = "edit" @@ -103,24 +104,26 @@ class EditMixin(PluginMixin): """ m = ModifyFileMocker(**modify_file_args) with patch("beetsplug.edit.edit", side_effect=m.action): - with control_stdin("\n".join(stdin)): - self.importer.run() + for char in stdin: + self.importer.add_choice(char) + self.importer.run() def run_mocked_command(self, modify_file_args={}, stdin=[], args=[]): """Run the edit command, with mocked stdin and yaml writing, and passing `args` to `run_command`.""" m = ModifyFileMocker(**modify_file_args) with patch("beetsplug.edit.edit", side_effect=m.action): - with control_stdin("\n".join(stdin)): - self.run_command("edit", *args) + for char in stdin: + self.io.addinput(char) + self.run_command("edit", *args) @_common.slow_test() @patch("beets.library.Item.write") class EditCommandTest(EditMixin, BeetsTestCase): """Black box tests for `beetsplug.edit`. Command line interaction is - simulated using `test.helper.control_stdin()`, and yaml editing via an - external editor is simulated using `ModifyFileMocker`. + simulated using mocked stdin, and yaml editing via an external editor is + simulated using `ModifyFileMocker`. """ ALBUM_COUNT = 1 @@ -412,7 +415,7 @@ class EditDuringImporterNonSingletonTest(EditDuringImporterTestCase): self.run_mocked_interpreter( {}, # 1, Apply changes. - ["1", "a"], + ["1", Action.APPLY], ) # Retag and edit track titles. On retag, the importer will reset items diff --git a/test/plugins/test_importsource.py b/test/plugins/test_importsource.py index a4f498181..7306558a1 100644 --- a/test/plugins/test_importsource.py +++ b/test/plugins/test_importsource.py @@ -19,7 +19,7 @@ import os import time from beets import importer, plugins -from beets.test.helper import AutotagImportTestCase, PluginMixin, control_stdin +from beets.test.helper import AutotagImportTestCase, IOMixin, PluginMixin from beets.util import syspath from beetsplug.importsource import ImportSourcePlugin @@ -34,7 +34,7 @@ def preserve_plugin_listeners(): ImportSourcePlugin.listeners = _listeners -class ImportSourceTest(PluginMixin, AutotagImportTestCase): +class ImportSourceTest(IOMixin, PluginMixin, AutotagImportTestCase): plugin = "importsource" preload_plugin = False @@ -50,31 +50,29 @@ class ImportSourceTest(PluginMixin, AutotagImportTestCase): self.all_items = self.lib.albums().get().items() self.item_to_remove = self.all_items[0] - def interact(self, stdin_input: str): - with control_stdin(stdin_input): - self.run_command( - "remove", - f"path:{syspath(self.item_to_remove.path)}", - ) + def interact(self, stdin: list[str]): + for char in stdin: + self.io.addinput(char) + self.run_command("remove", f"path:{syspath(self.item_to_remove.path)}") def test_do_nothing(self): - self.interact("N") + self.interact(["N"]) assert os.path.exists(self.item_to_remove.source_path) def test_remove_single(self): - self.interact("y\nD") + self.interact(["y", "D"]) assert not os.path.exists(self.item_to_remove.source_path) def test_remove_all_from_single(self): - self.interact("y\nR\ny") + self.interact(["y", "R", "y"]) for item in self.all_items: assert not os.path.exists(item.source_path) def test_stop_suggesting(self): - self.interact("y\nS") + self.interact(["y", "S"]) for item in self.all_items: assert os.path.exists(item.source_path) diff --git a/test/plugins/test_mbsubmit.py b/test/plugins/test_mbsubmit.py index 712c90866..fb275462a 100644 --- a/test/plugins/test_mbsubmit.py +++ b/test/plugins/test_mbsubmit.py @@ -18,7 +18,6 @@ from beets.test.helper import ( PluginMixin, TerminalImportMixin, capture_stdout, - control_stdin, ) @@ -35,9 +34,10 @@ class MBSubmitPluginTest( def test_print_tracks_output(self): """Test the output of the "print tracks" choice.""" with capture_stdout() as output: - with control_stdin("\n".join(["p", "s"])): - # Print tracks; Skip - self.importer.run() + self.io.addinput("p") + self.io.addinput("s") + # Print tracks; Skip + self.importer.run() # Manually build the string for comparing the output. tracklist = ( @@ -50,9 +50,12 @@ class MBSubmitPluginTest( def test_print_tracks_output_as_tracks(self): """Test the output of the "print tracks" choice, as singletons.""" with capture_stdout() as output: - with control_stdin("\n".join(["t", "s", "p", "s"])): - # as Tracks; Skip; Print tracks; Skip - self.importer.run() + self.io.addinput("t") + self.io.addinput("s") + self.io.addinput("p") + self.io.addinput("s") + # as Tracks; Skip; Print tracks; Skip + self.importer.run() # Manually build the string for comparing the output. tracklist = ( diff --git a/test/plugins/test_play.py b/test/plugins/test_play.py index b184db63f..e67c8cddf 100644 --- a/test/plugins/test_play.py +++ b/test/plugins/test_play.py @@ -21,14 +21,14 @@ from unittest.mock import ANY, patch import pytest -from beets.test.helper import CleanupModulesMixin, PluginTestCase, control_stdin +from beets.test.helper import CleanupModulesMixin, PluginTestCase, IOMixin from beets.ui import UserError from beets.util import open_anything from beetsplug.play import PlayPlugin @patch("beetsplug.play.util.interactive_open") -class PlayPluginTest(CleanupModulesMixin, PluginTestCase): +class PlayPluginTest(IOMixin, CleanupModulesMixin, PluginTestCase): modules = (PlayPlugin.__module__,) plugin = "play" @@ -127,8 +127,8 @@ class PlayPluginTest(CleanupModulesMixin, PluginTestCase): self.config["play"]["warning_threshold"] = 1 self.add_item(title="another NiceTitle") - with control_stdin("a"): - self.run_command("play", "nice") + self.io.addinput("a") + self.run_command("play", "nice") open_mock.assert_not_called() @@ -138,12 +138,12 @@ class PlayPluginTest(CleanupModulesMixin, PluginTestCase): expected_playlist = f"{self.item.filepath}\n{self.other_item.filepath}" - with control_stdin("a"): - self.run_and_assert( - open_mock, - ["-y", "NiceTitle"], - expected_playlist=expected_playlist, - ) + self.io.addinput("a") + self.run_and_assert( + open_mock, + ["-y", "NiceTitle"], + expected_playlist=expected_playlist, + ) def test_command_failed(self, open_mock): open_mock.side_effect = OSError("some reason") diff --git a/test/plugins/test_zero.py b/test/plugins/test_zero.py index b08bf0dca..23eb0e3cf 100644 --- a/test/plugins/test_zero.py +++ b/test/plugins/test_zero.py @@ -3,12 +3,12 @@ from mediafile import MediaFile from beets.library import Item -from beets.test.helper import PluginTestCase, control_stdin +from beets.test.helper import IOMixin, PluginTestCase from beets.util import syspath from beetsplug.zero import ZeroPlugin -class ZeroPluginTest(PluginTestCase): +class ZeroPluginTest(IOMixin, PluginTestCase): plugin = "zero" preload_plugin = False @@ -102,12 +102,10 @@ class ZeroPluginTest(PluginTestCase): item.write() item_id = item.id - with ( - self.configure_plugin( - {"fields": ["comments"], "update_database": True, "auto": False} - ), - control_stdin("y"), + with self.configure_plugin( + {"fields": ["comments"], "update_database": True, "auto": False} ): + self.io.addinput("y") self.run_command("zero") mf = MediaFile(syspath(item.path)) @@ -125,16 +123,14 @@ class ZeroPluginTest(PluginTestCase): item.write() item_id = item.id - with ( - self.configure_plugin( - { - "fields": ["comments"], - "update_database": False, - "auto": False, - } - ), - control_stdin("y"), + with self.configure_plugin( + { + "fields": ["comments"], + "update_database": False, + "auto": False, + } ): + self.io.addinput("y") self.run_command("zero") mf = MediaFile(syspath(item.path)) @@ -187,7 +183,8 @@ class ZeroPluginTest(PluginTestCase): item_id = item.id - with self.configure_plugin({"fields": []}), control_stdin("y"): + with self.configure_plugin({"fields": []}): + self.io.addinput("y") self.run_command("zero") item = self.lib.get_item(item_id) @@ -203,12 +200,10 @@ class ZeroPluginTest(PluginTestCase): item_id = item.id - with ( - self.configure_plugin( - {"fields": ["year"], "keep_fields": ["comments"]} - ), - control_stdin("y"), + with self.configure_plugin( + {"fields": ["year"], "keep_fields": ["comments"]} ): + self.io.addinput("y") self.run_command("zero") item = self.lib.get_item(item_id) @@ -303,12 +298,10 @@ class ZeroPluginTest(PluginTestCase): ) item.write() item_id = item.id - with ( - self.configure_plugin( - {"fields": ["comments"], "update_database": True, "auto": False} - ), - control_stdin("n"), + with self.configure_plugin( + {"fields": ["comments"], "update_database": True, "auto": False} ): + self.io.addinput("n") self.run_command("zero") mf = MediaFile(syspath(item.path)) diff --git a/test/test_plugins.py b/test/test_plugins.py index 53f24c13d..c23ea0c7a 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -429,8 +429,9 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase): # DummyPlugin.foo() should be called once with patch.object(DummyPlugin, "foo", autospec=True) as mock_foo: - with helper.control_stdin("\n".join(["f", "s"])): - self.importer.run() + self.io.addinput("f") + self.io.addinput("n") + self.importer.run() assert mock_foo.call_count == 1 # input_options should be called twice, as foo() returns None @@ -471,8 +472,8 @@ class PromptChoicesTest(TerminalImportMixin, PluginImportTestCase): ) # DummyPlugin.foo() should be called once - with helper.control_stdin("f\n"): - self.importer.run() + self.io.addinput("f") + self.importer.run() # input_options should be called once, as foo() returns SKIP self.mock_input_options.assert_called_once_with( diff --git a/test/ui/commands/test_modify.py b/test/ui/commands/test_modify.py index 77d378032..3e7a63d90 100644 --- a/test/ui/commands/test_modify.py +++ b/test/ui/commands/test_modify.py @@ -2,23 +2,24 @@ import unittest from mediafile import MediaFile -from beets.test.helper import BeetsTestCase, control_stdin +from beets.test.helper import BeetsTestCase, IOMixin from beets.ui.commands.modify import modify_parse_args from beets.util import syspath -class ModifyTest(BeetsTestCase): +class ModifyTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() self.album = self.add_album_fixture() [self.item] = self.album.items() - def modify_inp(self, inp, *args): - with control_stdin(inp): - self.run_command("modify", *args) + def modify_inp(self, inp: list[str], *args): + for chat in inp: + self.io.addinput(chat) + self.run_command("modify", *args) def modify(self, *args): - self.modify_inp("y", *args) + self.modify_inp(["y"], *args) # Item tests @@ -30,14 +31,14 @@ class ModifyTest(BeetsTestCase): def test_modify_item_abort(self): item = self.lib.items().get() title = item.title - self.modify_inp("n", "title=newTitle") + self.modify_inp(["n"], "title=newTitle") item = self.lib.items().get() assert item.title == title def test_modify_item_no_change(self): title = "Tracktitle" item = self.add_item_fixture(title=title) - self.modify_inp("y", "title", f"title={title}") + self.modify_inp(["y"], "title", f"title={title}") item = self.lib.items(title).get() assert item.title == title @@ -96,7 +97,9 @@ class ModifyTest(BeetsTestCase): title=f"{title}{i}", artist=original_artist, album=album ) self.modify_inp( - "s\ny\ny\ny\nn\nn\ny\ny\ny\ny\nn", title, f"artist={new_artist}" + ["s", "y", "y", "y", "n", "n", "y", "y", "y", "y", "n"], + title, + f"artist={new_artist}", ) original_items = self.lib.items(f"artist:{original_artist}") new_items = self.lib.items(f"artist:{new_artist}") diff --git a/test/ui/test_ui_init.py b/test/ui/test_ui_init.py index f6c9fe245..00e0a6fe5 100644 --- a/test/ui/test_ui_init.py +++ b/test/ui/test_ui_init.py @@ -22,7 +22,7 @@ from random import random from beets import config, ui from beets.test import _common -from beets.test.helper import BeetsTestCase, IOMixin, control_stdin +from beets.test.helper import BeetsTestCase, IOMixin class InputMethodsTest(IOMixin, unittest.TestCase): @@ -85,7 +85,7 @@ class InputMethodsTest(IOMixin, unittest.TestCase): assert items == ["1", "3"] -class ParentalDirCreation(BeetsTestCase): +class ParentalDirCreation(IOMixin, BeetsTestCase): def test_create_yes(self): non_exist_path = _common.os.fsdecode( os.path.join(self.temp_dir, b"nonexist", str(random()).encode()) @@ -94,8 +94,8 @@ class ParentalDirCreation(BeetsTestCase): # occur; wish I can use a golang defer here. test_config = deepcopy(config) test_config["library"] = non_exist_path - with control_stdin("y"): - lib = ui._open_library(test_config) + self.io.addinput("y") + lib = ui._open_library(test_config) lib._close() def test_create_no(self): @@ -108,14 +108,14 @@ class ParentalDirCreation(BeetsTestCase): test_config = deepcopy(config) test_config["library"] = non_exist_path - with control_stdin("n"): - try: - lib = ui._open_library(test_config) - except ui.UserError: - if os.path.exists(non_exist_path_parent): - shutil.rmtree(non_exist_path_parent) - raise OSError("Parent directories should not be created.") - else: - if lib: - lib._close() + self.io.addinput("n") + try: + lib = ui._open_library(test_config) + except ui.UserError: + if os.path.exists(non_exist_path_parent): + shutil.rmtree(non_exist_path_parent) raise OSError("Parent directories should not be created.") + else: + if lib: + lib._close() + raise OSError("Parent directories should not be created.") From d613981efea32bd9c58d28ab9549035b196e9fda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 16 Jan 2026 21:40:19 +0000 Subject: [PATCH 05/60] Replace capture_output with io.getoutput --- beets/test/helper.py | 65 ++++++++++-------------------- test/plugins/test_bareasc.py | 16 +++----- test/plugins/test_convert.py | 2 +- test/plugins/test_edit.py | 4 +- test/plugins/test_export.py | 4 +- test/plugins/test_fetchart.py | 4 +- test/plugins/test_info.py | 4 +- test/plugins/test_lastgenre.py | 4 +- test/plugins/test_limit.py | 4 +- test/plugins/test_mbsubmit.py | 27 ++++++------- test/plugins/test_missing.py | 19 ++++----- test/plugins/test_play.py | 2 +- test/plugins/test_smartplaylist.py | 4 +- test/plugins/test_types_plugin.py | 4 +- test/test_metasync.py | 4 +- test/test_plugins.py | 3 +- test/ui/commands/test_config.py | 4 +- test/ui/commands/test_list.py | 37 +++++++++-------- test/ui/commands/test_write.py | 4 +- test/ui/test_ui.py | 2 +- 20 files changed, 94 insertions(+), 123 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 879ff5f6f..7762ab866 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -15,9 +15,6 @@ """This module includes various helpers that provide fixtures, capture information or mock the environment. -- `capture_stdout` context managers allow one to interact with the user - interface. - - `has_program` checks the presence of a command on the system. - The `ImportSessionFixture` allows one to run importer code while @@ -38,7 +35,6 @@ from contextlib import contextmanager from dataclasses import dataclass from enum import Enum from functools import cached_property -from io import StringIO from pathlib import Path from tempfile import gettempdir, mkdtemp, mkstemp from typing import Any, ClassVar @@ -84,25 +80,6 @@ def capture_log(logger="beets"): log.removeHandler(capture) -@contextmanager -def capture_stdout(): - """Save stdout in a StringIO. - - >>> with capture_stdout() as output: - ... print('spam') - ... - >>> output.getvalue() - 'spam' - """ - org = sys.stdout - sys.stdout = capture = StringIO() - try: - yield sys.stdout - finally: - sys.stdout = org - print(capture.getvalue()) - - def has_program(cmd, args=["--version"]): """Returns `True` if `cmd` can be executed.""" full_cmd = [cmd, *args] @@ -148,12 +125,31 @@ NEEDS_REFLINK = unittest.skipUnless( ) +class RunMixin: + def run_command(self, *args, **kwargs): + """Run a beets command with an arbitrary amount of arguments. The + Library` defaults to `self.lib`, but can be overridden with + the keyword argument `lib`. + """ + sys.argv = ["beet"] # avoid leakage from test suite args + lib = None + if hasattr(self, "lib"): + lib = self.lib + lib = kwargs.get("lib", lib) + beets.ui._raw_main(list(args), lib) + + @pytest.mark.usefixtures("io") -class IOMixin: +class IOMixin(RunMixin): io: _common.DummyIO + def run_with_output(self, *args): + self.io.getoutput() + self.run_command(*args) + return self.io.getoutput() -class TestHelper(ConfigMixin): + +class TestHelper(RunMixin, ConfigMixin): """Helper mixin for high-level cli and plugin tests. This mixin provides methods to isolate beets' global state provide @@ -368,25 +364,6 @@ class TestHelper(ConfigMixin): return path - # Running beets commands - - def run_command(self, *args, **kwargs): - """Run a beets command with an arbitrary amount of arguments. The - Library` defaults to `self.lib`, but can be overridden with - the keyword argument `lib`. - """ - sys.argv = ["beet"] # avoid leakage from test suite args - lib = None - if hasattr(self, "lib"): - lib = self.lib - lib = kwargs.get("lib", lib) - beets.ui._raw_main(list(args), lib) - - def run_with_output(self, *args): - with capture_stdout() as out: - self.run_command(*args) - return out.getvalue() - # Safe file operations def create_temp_dir(self, **kwargs) -> str: diff --git a/test/plugins/test_bareasc.py b/test/plugins/test_bareasc.py index e699a3dcf..a661ae7aa 100644 --- a/test/plugins/test_bareasc.py +++ b/test/plugins/test_bareasc.py @@ -4,10 +4,10 @@ """Tests for the 'bareasc' plugin.""" from beets import logging -from beets.test.helper import PluginTestCase, capture_stdout +from beets.test.helper import IOMixin, PluginTestCase -class BareascPluginTest(PluginTestCase): +class BareascPluginTest(IOMixin, PluginTestCase): """Test bare ASCII query matching.""" plugin = "bareasc" @@ -65,16 +65,12 @@ class BareascPluginTest(PluginTestCase): def test_bareasc_list_output(self): """Bare-ASCII version of list command - check output.""" - with capture_stdout() as output: - self.run_command("bareasc", "with accents") + self.run_command("bareasc", "with accents") - assert "Antonin Dvorak" in output.getvalue() + assert "Antonin Dvorak" in self.io.getoutput() def test_bareasc_format_output(self): """Bare-ASCII version of list -f command - check output.""" - with capture_stdout() as output: - self.run_command( - "bareasc", "with accents", "-f", "$artist:: $title" - ) + self.run_command("bareasc", "with accents", "-f", "$artist:: $title") - assert "Antonin Dvorak:: with accents\n" == output.getvalue() + assert "Antonin Dvorak:: with accents\n" == self.io.getoutput() diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index de2650617..13dbea084 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -29,8 +29,8 @@ from beets.test import _common from beets.test.helper import ( AsIsImporterMixin, ImportHelper, - PluginTestCase, IOMixin, + PluginTestCase, capture_log, ) from beetsplug import convert diff --git a/test/plugins/test_edit.py b/test/plugins/test_edit.py index 94ab34a03..06c7cad74 100644 --- a/test/plugins/test_edit.py +++ b/test/plugins/test_edit.py @@ -74,7 +74,7 @@ class ModifyFileMocker: f.write(contents) -class EditMixin(IOMixin, PluginMixin): +class EditMixin(PluginMixin): """Helper containing some common functionality used for the Edit tests.""" plugin = "edit" @@ -120,7 +120,7 @@ class EditMixin(IOMixin, PluginMixin): @_common.slow_test() @patch("beets.library.Item.write") -class EditCommandTest(EditMixin, BeetsTestCase): +class EditCommandTest(IOMixin, EditMixin, BeetsTestCase): """Black box tests for `beetsplug.edit`. Command line interaction is simulated using mocked stdin, and yaml editing via an external editor is simulated using `ModifyFileMocker`. diff --git a/test/plugins/test_export.py b/test/plugins/test_export.py index f37a0d2a7..3c795b2dc 100644 --- a/test/plugins/test_export.py +++ b/test/plugins/test_export.py @@ -19,10 +19,10 @@ import re # used to test csv format from xml.etree import ElementTree from xml.etree.ElementTree import Element -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class ExportPluginTest(PluginTestCase): +class ExportPluginTest(IOMixin, PluginTestCase): plugin = "export" def setUp(self): diff --git a/test/plugins/test_fetchart.py b/test/plugins/test_fetchart.py index 96d882e9a..f347ed66a 100644 --- a/test/plugins/test_fetchart.py +++ b/test/plugins/test_fetchart.py @@ -18,10 +18,10 @@ import os import sys from beets import util -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class FetchartCliTest(PluginTestCase): +class FetchartCliTest(IOMixin, PluginTestCase): plugin = "fetchart" def setUp(self): diff --git a/test/plugins/test_info.py b/test/plugins/test_info.py index c1b3fc941..3ad4d0884 100644 --- a/test/plugins/test_info.py +++ b/test/plugins/test_info.py @@ -15,11 +15,11 @@ from mediafile import MediaFile -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase from beets.util import displayable_path -class InfoTest(PluginTestCase): +class InfoTest(IOMixin, PluginTestCase): plugin = "info" def test_path(self): diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 3de43d197..f499992c6 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -19,11 +19,11 @@ from unittest.mock import Mock, patch import pytest from beets.test import _common -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase from beetsplug import lastgenre -class LastGenrePluginTest(PluginTestCase): +class LastGenrePluginTest(IOMixin, PluginTestCase): plugin = "lastgenre" def setUp(self): diff --git a/test/plugins/test_limit.py b/test/plugins/test_limit.py index d77e47ca8..8f227d41e 100644 --- a/test/plugins/test_limit.py +++ b/test/plugins/test_limit.py @@ -13,10 +13,10 @@ """Tests for the 'limit' plugin.""" -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class LimitPluginTest(PluginTestCase): +class LimitPluginTest(IOMixin, PluginTestCase): """Unit tests for LimitPlugin Note: query prefix tests do not work correctly with `run_with_output`. diff --git a/test/plugins/test_mbsubmit.py b/test/plugins/test_mbsubmit.py index fb275462a..48426fd7d 100644 --- a/test/plugins/test_mbsubmit.py +++ b/test/plugins/test_mbsubmit.py @@ -17,7 +17,6 @@ from beets.test.helper import ( AutotagImportTestCase, PluginMixin, TerminalImportMixin, - capture_stdout, ) @@ -33,11 +32,10 @@ class MBSubmitPluginTest( def test_print_tracks_output(self): """Test the output of the "print tracks" choice.""" - with capture_stdout() as output: - self.io.addinput("p") - self.io.addinput("s") - # Print tracks; Skip - self.importer.run() + self.io.addinput("p") + self.io.addinput("s") + # Print tracks; Skip + self.importer.run() # Manually build the string for comparing the output. tracklist = ( @@ -45,20 +43,19 @@ class MBSubmitPluginTest( "01. Tag Track 1 - Tag Artist (0:01)\n" "02. Tag Track 2 - Tag Artist (0:01)" ) - assert tracklist in output.getvalue() + assert tracklist in self.io.getoutput() def test_print_tracks_output_as_tracks(self): """Test the output of the "print tracks" choice, as singletons.""" - with capture_stdout() as output: - self.io.addinput("t") - self.io.addinput("s") - self.io.addinput("p") - self.io.addinput("s") - # as Tracks; Skip; Print tracks; Skip - self.importer.run() + self.io.addinput("t") + self.io.addinput("s") + self.io.addinput("p") + self.io.addinput("s") + # as Tracks; Skip; Print tracks; Skip + self.importer.run() # Manually build the string for comparing the output. tracklist = ( "Open files with Picard? 02. Tag Track 2 - Tag Artist (0:01)" ) - assert tracklist in output.getvalue() + assert tracklist in self.io.getoutput() diff --git a/test/plugins/test_missing.py b/test/plugins/test_missing.py index d12f2b4cf..812ed5fa3 100644 --- a/test/plugins/test_missing.py +++ b/test/plugins/test_missing.py @@ -3,20 +3,23 @@ import uuid import pytest from beets.library import Album -from beets.test.helper import PluginMixin, TestHelper +from beets.test.helper import IOMixin, PluginMixin, TestHelper @pytest.fixture -def helper(): +def helper(request): helper = TestHelper() helper.setup_beets() - yield helper + request.instance.lib = helper.lib + + yield helper.teardown_beets() -class TestMissingAlbums(PluginMixin): +@pytest.mark.usefixtures("helper") +class TestMissingAlbums(IOMixin, PluginMixin): plugin = "missing" album_in_lib = Album( album="Album", @@ -47,15 +50,13 @@ class TestMissingAlbums(PluginMixin): ], ) def test_missing_artist_albums( - self, requests_mock, helper, release_from_mb, expected_output + self, requests_mock, release_from_mb, expected_output ): - helper.lib.add(self.album_in_lib) + self.lib.add(self.album_in_lib) requests_mock.get( f"/ws/2/release-group?artist={self.album_in_lib.mb_albumartistid}", json={"release-groups": [release_from_mb]}, ) with self.configure_plugin({}): - assert ( - helper.run_with_output("missing", "--album") == expected_output - ) + assert self.run_with_output("missing", "--album") == expected_output diff --git a/test/plugins/test_play.py b/test/plugins/test_play.py index e67c8cddf..53de21233 100644 --- a/test/plugins/test_play.py +++ b/test/plugins/test_play.py @@ -21,7 +21,7 @@ from unittest.mock import ANY, patch import pytest -from beets.test.helper import CleanupModulesMixin, PluginTestCase, IOMixin +from beets.test.helper import CleanupModulesMixin, IOMixin, PluginTestCase from beets.ui import UserError from beets.util import open_anything from beetsplug.play import PlayPlugin diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index 8ec2c74ce..7cc712330 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -24,7 +24,7 @@ import pytest from beets import config from beets.dbcore.query import FixedFieldSort, MultipleSort, NullSort from beets.library import Album, Item, parse_query_string -from beets.test.helper import BeetsTestCase, PluginTestCase +from beets.test.helper import BeetsTestCase, IOMixin, PluginTestCase from beets.ui import UserError from beets.util import CHAR_REPLACE, syspath from beetsplug.smartplaylist import SmartPlaylistPlugin @@ -458,7 +458,7 @@ class SmartPlaylistTest(BeetsTestCase): assert content.count(b"/item2.mp3") == 1 -class SmartPlaylistCLITest(PluginTestCase): +class SmartPlaylistCLITest(IOMixin, PluginTestCase): plugin = "smartplaylist" def setUp(self): diff --git a/test/plugins/test_types_plugin.py b/test/plugins/test_types_plugin.py index 41807b80d..24fb577f7 100644 --- a/test/plugins/test_types_plugin.py +++ b/test/plugins/test_types_plugin.py @@ -19,10 +19,10 @@ from datetime import datetime import pytest from confuse import ConfigValueError -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase -class TypesPluginTest(PluginTestCase): +class TypesPluginTest(IOMixin, PluginTestCase): plugin = "types" def test_integer_modify_and_query(self): diff --git a/test/test_metasync.py b/test/test_metasync.py index 13c003a1c..aeb38545b 100644 --- a/test/test_metasync.py +++ b/test/test_metasync.py @@ -20,7 +20,7 @@ from datetime import datetime from beets.library import Item from beets.test import _common -from beets.test.helper import PluginTestCase +from beets.test.helper import IOMixin, PluginTestCase def _parsetime(s): @@ -31,7 +31,7 @@ def _is_windows(): return platform.system() == "Windows" -class MetaSyncTest(PluginTestCase): +class MetaSyncTest(IOMixin, PluginTestCase): plugin = "metasync" itunes_library_unix = os.path.join(_common.RSRC, b"itunes_library_unix.xml") itunes_library_windows = os.path.join( diff --git a/test/test_plugins.py b/test/test_plugins.py index c23ea0c7a..4786b12b4 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -38,6 +38,7 @@ from beets.test import helper from beets.test.helper import ( AutotagStub, ImportHelper, + IOMixin, PluginMixin, PluginTestCase, TerminalImportMixin, @@ -45,7 +46,7 @@ from beets.test.helper import ( from beets.util import PromptChoice, displayable_path, syspath -class TestPluginRegistration(PluginTestCase): +class TestPluginRegistration(IOMixin, PluginTestCase): class RatingPlugin(plugins.BeetsPlugin): item_types: ClassVar[dict[str, types.Type]] = { "rating": types.Float(), diff --git a/test/ui/commands/test_config.py b/test/ui/commands/test_config.py index c1215ef43..cd83b919f 100644 --- a/test/ui/commands/test_config.py +++ b/test/ui/commands/test_config.py @@ -5,10 +5,10 @@ import pytest import yaml from beets import config, ui -from beets.test.helper import BeetsTestCase +from beets.test.helper import BeetsTestCase, IOMixin -class ConfigCommandTest(BeetsTestCase): +class ConfigCommandTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() for k in ("VISUAL", "EDITOR"): diff --git a/test/ui/commands/test_list.py b/test/ui/commands/test_list.py index a63a56ad1..372d75410 100644 --- a/test/ui/commands/test_list.py +++ b/test/ui/commands/test_list.py @@ -1,9 +1,9 @@ from beets.test import _common -from beets.test.helper import BeetsTestCase, capture_stdout +from beets.test.helper import BeetsTestCase, IOMixin from beets.ui.commands.list import list_items -class ListTest(BeetsTestCase): +class ListTest(IOMixin, BeetsTestCase): def setUp(self): super().setUp() self.item = _common.item() @@ -12,13 +12,12 @@ class ListTest(BeetsTestCase): self.lib.add_album([self.item]) def _run_list(self, query="", album=False, path=False, fmt=""): - with capture_stdout() as stdout: - list_items(self.lib, query, album, fmt) - return stdout + list_items(self.lib, query, album, fmt) + return self.io.getoutput() def test_list_outputs_item(self): stdout = self._run_list() - assert "the title" in stdout.getvalue() + assert "the title" in stdout def test_list_unicode_query(self): self.item.title = "na\xefve" @@ -26,44 +25,44 @@ class ListTest(BeetsTestCase): self.lib._connection().commit() stdout = self._run_list(["na\xefve"]) - out = stdout.getvalue() + out = stdout assert "na\xefve" in out def test_list_item_path(self): stdout = self._run_list(fmt="$path") - assert stdout.getvalue().strip() == "xxx/yyy" + assert stdout.strip() == "xxx/yyy" def test_list_album_outputs_something(self): stdout = self._run_list(album=True) - assert len(stdout.getvalue()) > 0 + assert len(stdout) > 0 def test_list_album_path(self): stdout = self._run_list(album=True, fmt="$path") - assert stdout.getvalue().strip() == "xxx" + assert stdout.strip() == "xxx" def test_list_album_omits_title(self): stdout = self._run_list(album=True) - assert "the title" not in stdout.getvalue() + assert "the title" not in stdout def test_list_uses_track_artist(self): stdout = self._run_list() - assert "the artist" in stdout.getvalue() - assert "the album artist" not in stdout.getvalue() + assert "the artist" in stdout + assert "the album artist" not in stdout def test_list_album_uses_album_artist(self): stdout = self._run_list(album=True) - assert "the artist" not in stdout.getvalue() - assert "the album artist" in stdout.getvalue() + assert "the artist" not in stdout + assert "the album artist" in stdout def test_list_item_format_artist(self): stdout = self._run_list(fmt="$artist") - assert "the artist" in stdout.getvalue() + assert "the artist" in stdout def test_list_item_format_multiple(self): stdout = self._run_list(fmt="$artist - $album - $year") - assert "the artist - the album - 0001" == stdout.getvalue().strip() + assert "the artist - the album - 0001" == stdout.strip() def test_list_album_format(self): stdout = self._run_list(album=True, fmt="$genre") - assert "the genre" in stdout.getvalue() - assert "the album" not in stdout.getvalue() + assert "the genre" in stdout + assert "the album" not in stdout diff --git a/test/ui/commands/test_write.py b/test/ui/commands/test_write.py index 312b51dd2..7197cbded 100644 --- a/test/ui/commands/test_write.py +++ b/test/ui/commands/test_write.py @@ -1,7 +1,7 @@ -from beets.test.helper import BeetsTestCase +from beets.test.helper import BeetsTestCase, IOMixin -class WriteTest(BeetsTestCase): +class WriteTest(IOMixin, BeetsTestCase): def write_cmd(self, *args): return self.run_with_output("write", *args) diff --git a/test/ui/test_ui.py b/test/ui/test_ui.py index f96d2c76a..577954a85 100644 --- a/test/ui/test_ui.py +++ b/test/ui/test_ui.py @@ -398,7 +398,7 @@ class PluginTest(TestPluginTestCase): self.run_command("test", lib=None) -class CommonOptionsParserCliTest(BeetsTestCase): +class CommonOptionsParserCliTest(IOMixin, BeetsTestCase): """Test CommonOptionsParser and formatting LibModel formatting on 'list' command. """ From b3c1da3c4f1ac2ab25002ebf9c514e32b3c59d5b Mon Sep 17 00:00:00 2001 From: Meth Munindradasa Date: Wed, 11 Feb 2026 21:43:55 +1100 Subject: [PATCH 06/60] Clarify Azure Translator resource region requirement Added note about setting the translator resource region to Global to avoid 401 errors. --- docs/plugins/lyrics.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/plugins/lyrics.rst b/docs/plugins/lyrics.rst index 9cc63a8b7..566fa808e 100644 --- a/docs/plugins/lyrics.rst +++ b/docs/plugins/lyrics.rst @@ -174,6 +174,9 @@ We use Azure to optionally translate your lyrics. To set up the integration, follow these steps: 1. `Create a Translator resource`_ on Azure. + Make sure the region of the translator resource is set to Global. You + will get 401 unauthorized errors if not. The region of the resource group + does not matter. 2. `Obtain its API key`_. 3. Add the API key to your configuration as ``translate.api_key``. 4. Configure your target language using the ``translate.to_language`` option. From f41f1839be7c6183c58d328e1efff70c1e73d267 Mon Sep 17 00:00:00 2001 From: Meth Munindradasa Date: Thu, 12 Feb 2026 07:51:04 +1100 Subject: [PATCH 07/60] Format docs to pass lint checker --- docs/plugins/lyrics.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/plugins/lyrics.rst b/docs/plugins/lyrics.rst index 566fa808e..33aa9b61e 100644 --- a/docs/plugins/lyrics.rst +++ b/docs/plugins/lyrics.rst @@ -174,9 +174,9 @@ We use Azure to optionally translate your lyrics. To set up the integration, follow these steps: 1. `Create a Translator resource`_ on Azure. - Make sure the region of the translator resource is set to Global. You - will get 401 unauthorized errors if not. The region of the resource group - does not matter. + Make sure the region of the translator resource is set to Global. You + will get 401 unauthorized errors if not. The region of the resource group + does not matter. 2. `Obtain its API key`_. 3. Add the API key to your configuration as ``translate.api_key``. 4. Configure your target language using the ``translate.to_language`` option. From 37e18fbb46df8e2b9c5ebc49dd8dc2ee77d8e94e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 25 Jan 2026 06:45:07 +0000 Subject: [PATCH 08/60] Adapt code to fully typed confuse library --- beets/__init__.py | 4 ++-- beets/plugins.py | 4 ++-- beets/ui/__init__.py | 25 +++++++++++++------------ beets/ui/commands/import_/session.py | 3 ++- beetsplug/discogs/__init__.py | 7 +++---- beetsplug/fetchart.py | 5 +++-- beetsplug/lastgenre/__init__.py | 22 ++++++++++------------ beetsplug/lyrics.py | 2 +- beetsplug/playlist.py | 4 ++-- beetsplug/smartplaylist.py | 5 +++-- beetsplug/titlecase.py | 2 +- 11 files changed, 42 insertions(+), 41 deletions(-) diff --git a/beets/__init__.py b/beets/__init__.py index 4bde53504..750efb3b3 100644 --- a/beets/__init__.py +++ b/beets/__init__.py @@ -37,11 +37,11 @@ class IncludeLazyConfig(confuse.LazyConfig): YAML files specified in an `include` setting. """ - def read(self, user=True, defaults=True): + def read(self, user: bool = True, defaults: bool = True) -> None: super().read(user, defaults) try: - for view in self["include"]: + for view in self["include"].sequence(): self.set_file(view.as_filename()) except confuse.NotFoundError: pass diff --git a/beets/plugins.py b/beets/plugins.py index 01d9d3327..b1650d5b0 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -37,7 +37,7 @@ from beets.util.deprecation import deprecate_for_maintainers, deprecate_for_user if TYPE_CHECKING: from collections.abc import Callable, Iterable, Iterator, Sequence - from confuse import ConfigView + from confuse import Subview from beets.dbcore import Query from beets.dbcore.db import FieldQueryType @@ -162,7 +162,7 @@ class BeetsPlugin(metaclass=BeetsPluginMeta): album_template_fields: TFuncMap[Album] name: str - config: ConfigView + config: Subview early_import_stages: list[ImportStageFunc] import_stages: list[ImportStageFunc] diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 8db4dd79f..4c93d66d8 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -30,7 +30,6 @@ import textwrap import traceback from difflib import SequenceMatcher from functools import cache -from itertools import chain from typing import TYPE_CHECKING, Any, Literal import confuse @@ -551,18 +550,20 @@ def get_color_config() -> dict[ColorName, str]: legacy single-color format. Validates all color names against known codes and raises an error for any invalid entries. """ - colors_by_color_name: dict[ColorName, list[str]] = { - k: (v if isinstance(v, list) else LEGACY_COLORS.get(v, [v])) - for k, v in config["ui"]["colors"].flatten().items() - } - - if invalid_colors := ( - set(chain.from_iterable(colors_by_color_name.values())) - - CODE_BY_COLOR.keys() - ): - raise UserError( - f"Invalid color(s) in configuration: {', '.join(invalid_colors)}" + template_dict: dict[ColorName, confuse.OneOf[str | list[str]]] = { + n: confuse.OneOf( + [ + confuse.Choice(sorted(LEGACY_COLORS)), + confuse.Sequence(confuse.Choice(sorted(CODE_BY_COLOR))), + ] ) + for n in ColorName.__args__ # type: ignore[attr-defined] + } + template = confuse.MappingTemplate(template_dict) + colors_by_color_name = { + k: (v if isinstance(v, list) else LEGACY_COLORS.get(v, [v])) + for k, v in config["ui"]["colors"].get(template).items() + } return { n: ";".join(str(CODE_BY_COLOR[c]) for c in colors) diff --git a/beets/ui/commands/import_/session.py b/beets/ui/commands/import_/session.py index 42a809634..1848e4192 100644 --- a/beets/ui/commands/import_/session.py +++ b/beets/ui/commands/import_/session.py @@ -327,7 +327,7 @@ def summarize_items(items, singleton): return ", ".join(summary_parts) -def _summary_judgment(rec): +def _summary_judgment(rec: Recommendation) -> importer.Action | None: """Determines whether a decision should be made without even asking the user. This occurs in quiet mode and when an action is chosen for NONE recommendations. Return None if the user should be queried. @@ -335,6 +335,7 @@ def _summary_judgment(rec): summary judgment is made. """ + action: importer.Action | None if config["import"]["quiet"]: if rec == Recommendation.strong: return importer.Action.APPLY diff --git a/beetsplug/discogs/__init__.py b/beetsplug/discogs/__init__.py index dc88e0f14..bdbeb8fc0 100644 --- a/beetsplug/discogs/__init__.py +++ b/beetsplug/discogs/__init__.py @@ -355,10 +355,9 @@ class DiscogsPlugin(MetadataSourcePlugin): style = self.format(result.data.get("styles")) base_genre = self.format(result.data.get("genres")) - if self.config["append_style_genre"] and style: - genre = self.config["separator"].as_str().join([base_genre, style]) - else: - genre = base_genre + genre = base_genre + if self.config["append_style_genre"] and genre is not None and style: + genre += f"{self.config['separator'].as_str()}{style}" discogs_albumid = self._extract_id(result.data.get("uri")) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index ef311cbbd..e4de9181b 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -288,7 +288,8 @@ class Candidate: elif check == ImageAction.REFORMAT: self.path = ArtResizer.shared.reformat( self.path, - plugin.cover_format, + # TODO: fix this gnarly logic to remove the need for type ignore + plugin.cover_format, # type: ignore[arg-type] deinterlaced=plugin.deinterlace, ) @@ -1367,7 +1368,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): # allow both pixel and percentage-based margin specifications self.enforce_ratio = self.config["enforce_ratio"].get( - confuse.OneOf( + confuse.OneOf[bool | str]( [ bool, confuse.String(pattern=self.PAT_PX), diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 1c91688a6..f7aef0261 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -41,6 +41,7 @@ if TYPE_CHECKING: import optparse from collections.abc import Callable + from beets.importer import ImportSession, ImportTask from beets.library import LibModel LASTFM = pylast.LastFMNetwork(api_key=plugins.LASTFM_KEY) @@ -178,14 +179,13 @@ class LastGenrePlugin(plugins.BeetsPlugin): """A tuple of allowed genre sources. May contain 'track', 'album', or 'artist.' """ - source = self.config["source"].as_choice(("track", "album", "artist")) - if source == "track": - return "track", "album", "artist" - if source == "album": - return "album", "artist" - if source == "artist": - return ("artist",) - return tuple() + return self.config["source"].as_choice( + { + "track": ("track", "album", "artist"), + "album": ("album", "artist"), + "artist": ("artist",), + } + ) # More canonicalization and general helpers. @@ -603,10 +603,8 @@ class LastGenrePlugin(plugins.BeetsPlugin): lastgenre_cmd.func = lastgenre_func return [lastgenre_cmd] - def imported( - self, session: library.Session, task: library.ImportTask - ) -> None: - self._process(task.album if task.is_album else task.item, write=False) + def imported(self, _: ImportSession, task: ImportTask) -> None: + self._process(task.album if task.is_album else task.item, write=False) # type: ignore[attr-defined] def _tags_for( self, diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 3b626a50b..72df907db 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -358,7 +358,7 @@ class LRCLib(Backend): for group in self.fetch_candidates(artist, title, album, length): candidates = [evaluate_item(item) for item in group] if item := self.pick_best_match(candidates): - lyrics = item.get_text(self.config["synced"]) + lyrics = item.get_text(self.config["synced"].get(bool)) return lyrics, f"{self.GET_URL}/{item.id}" return None diff --git a/beetsplug/playlist.py b/beetsplug/playlist.py index a1f9fff39..54a03646f 100644 --- a/beetsplug/playlist.py +++ b/beetsplug/playlist.py @@ -69,7 +69,7 @@ class PlaylistQuery(InQuery[bytes]): relative_to = os.path.dirname(playlist_path) else: relative_to = config["relative_to"].as_filename() - relative_to = beets.util.bytestring_path(relative_to) + relative_to_bytes = beets.util.bytestring_path(relative_to) for line in f: if line[0] == "#": @@ -78,7 +78,7 @@ class PlaylistQuery(InQuery[bytes]): paths.append( beets.util.normpath( - os.path.join(relative_to, line.rstrip()) + os.path.join(relative_to_bytes, line.rstrip()) ) ) f.close() diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index e22a65787..a5cc8e362 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -262,8 +262,9 @@ class SmartPlaylistPlugin(BeetsPlugin): "Updating {} smart playlists...", len(self._matched_playlists) ) - playlist_dir = self.config["playlist_dir"].as_filename() - playlist_dir = bytestring_path(playlist_dir) + playlist_dir = bytestring_path( + self.config["playlist_dir"].as_filename() + ) tpl = self.config["uri_format"].get() prefix = bytestring_path(self.config["prefix"].as_str()) relative_to = self.config["relative_to"].get() diff --git a/beetsplug/titlecase.py b/beetsplug/titlecase.py index d722d4d16..634f5fe4d 100644 --- a/beetsplug/titlecase.py +++ b/beetsplug/titlecase.py @@ -104,7 +104,7 @@ class TitlecasePlugin(BeetsPlugin): @cached_property def replace(self) -> list[tuple[str, str]]: - return self.config["replace"].as_pairs() + return self.config["replace"].as_pairs(default_value="") @cached_property def the_artist(self) -> bool: From 15755c1ff9ce4d270e7ec81d96867b6fed6a1c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Wed, 28 Jan 2026 10:46:37 +0000 Subject: [PATCH 09/60] Update confuse --- poetry.lock | 13 ++++++++----- pyproject.toml | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/poetry.lock b/poetry.lock index 1a0515819..023ea7718 100644 --- a/poetry.lock +++ b/poetry.lock @@ -722,18 +722,21 @@ files = [ [[package]] name = "confuse" -version = "2.1.0" +version = "2.2.0" description = "Painless YAML config files" optional = false -python-versions = ">=3.9" +python-versions = ">=3.10" files = [ - {file = "confuse-2.1.0-py3-none-any.whl", hash = "sha256:502be1299aa6bf7c48f7719f56795720c073fb28550c0c7a37394366c9d30316"}, - {file = "confuse-2.1.0.tar.gz", hash = "sha256:abb9674a99c7a6efaef84e2fc84403ecd2dd304503073ff76ea18ed4176e218d"}, + {file = "confuse-2.2.0-py3-none-any.whl", hash = "sha256:470c6aa1a5008c8d740267f2ad574e3a715b6dd873c1e5f8778b7f7abb954722"}, + {file = "confuse-2.2.0.tar.gz", hash = "sha256:35c1b53e81be125f441bee535130559c935917b26aeaa61289010cd1f55c2b9e"}, ] [package.dependencies] pyyaml = "*" +[package.extras] +docs = ["sphinx (>=7.4.7)", "sphinx-rtd-theme (>=3.0.2)"] + [[package]] name = "coverage" version = "7.11.0" @@ -4558,4 +4561,4 @@ web = ["flask", "flask-cors"] [metadata] lock-version = "2.0" python-versions = ">=3.10,<4" -content-hash = "eefe427d3b3b9b871ca6bcd8405e3578a16d660afd7925c14793514f03c96ac6" +content-hash = "9cff39f63616b2654fbf44b006f7eedcae6c1846fbb9f04af82483891b7d77b9" diff --git a/pyproject.toml b/pyproject.toml index 2a0a1904d..57a12fa75 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,7 +44,7 @@ Changelog = "https://github.com/beetbox/beets/blob/master/docs/changelog.rst" python = ">=3.10,<4" colorama = { version = "*", markers = "sys_platform == 'win32'" } -confuse = ">=2.1.0" +confuse = ">=2.2.0" jellyfish = "*" lap = ">=0.5.12" mediafile = ">=0.12.0" From 34fe39d0668704dc9c96b858168b8253d4c61b36 Mon Sep 17 00:00:00 2001 From: "Andrey M." Date: Mon, 16 Feb 2026 19:23:57 +0200 Subject: [PATCH 10/60] Add beets-fillmissing to the list of plugins --- docs/plugins/index.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 0a461b857..bb89dde3f 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -481,6 +481,10 @@ beets-filetote_ Helps bring non-music extra files, attachments, and artifacts during imports and CLI file manipulation actions (``beet move``, etc.). +beets-fillmissing_ + Interactively prompts you to fill in missing or incomplete metadata fields + for music tracks. + beets-follow_ Lets you check for new albums from artists you like. @@ -596,6 +600,8 @@ beets-youtube_ .. _beets-filetote: https://github.com/gtronset/beets-filetote +.. _beets-fillmissing: https://github.com/amiv1/beets-fillmissing + .. _beets-follow: https://github.com/nolsto/beets-follow .. _beets-goingrunning: https://pypi.org/project/beets-goingrunning From 743a59be3b3d5f83b6f0cbb51dcb76ffb3fbbf4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 9 Feb 2026 09:43:41 +0000 Subject: [PATCH 11/60] Fix replaygain tests --- test/plugins/test_replaygain.py | 63 +++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 094349b25..054b70a76 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -14,12 +14,11 @@ import unittest -from typing import ClassVar +from typing import Any, ClassVar import pytest from mediafile import MediaFile -from beets import config from beets.test.helper import ( AsIsImporterMixin, ImportTestCase, @@ -39,10 +38,15 @@ try: except (ImportError, ValueError): GST_AVAILABLE = False -if any(has_program(cmd, ["-v"]) for cmd in ["mp3gain", "aacgain"]): - GAIN_PROG_AVAILABLE = True -else: - GAIN_PROG_AVAILABLE = False + +GAIN_PROG = next( + ( + cmd + for cmd in ["mp3gain", "mp3rgain", "aacgain"] + if has_program(cmd, ["-v"]) + ), + None, +) FFMPEG_AVAILABLE = has_program("ffmpeg", ["-version"]) @@ -63,14 +67,18 @@ class ReplayGainTestCase(PluginMixin, ImportTestCase): plugin = "replaygain" preload_plugin = False - backend: ClassVar[str] + plugin_config: ClassVar[dict[str, Any]] + + @property + def backend(self): + return self.plugin_config["backend"] def setUp(self): # Implemented by Mixins, see above. This may decide to skip the test. self.test_backend() super().setUp() - self.config["replaygain"]["backend"] = self.backend + self.config["replaygain"].set(self.plugin_config) self.load_plugins() @@ -81,8 +89,16 @@ class ThreadedImportMixin: self.config["threaded"] = True -class GstBackendMixin: - backend = "gstreamer" +class BackendMixin: + plugin_config: ClassVar[dict[str, Any]] + has_r128_support: bool + + def test_backend(self): + """Check whether the backend actually has all required functionality.""" + + +class GstBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "gstreamer"} has_r128_support = True def test_backend(self): @@ -90,30 +106,25 @@ class GstBackendMixin: try: # Check if required plugins can be loaded by instantiating a # GStreamerBackend (via its .__init__). - config["replaygain"]["targetlevel"] = 89 - GStreamerBackend(config["replaygain"], None) + self.config["replaygain"]["targetlevel"] = 89 + GStreamerBackend(self.config["replaygain"], None) except FatalGstreamerPluginReplayGainError as e: # Skip the test if plugins could not be loaded. self.skipTest(str(e)) -class CmdBackendMixin: - backend = "command" +class CmdBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = { + "backend": "command", + "command": GAIN_PROG, + } has_r128_support = False - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - -class FfmpegBackendMixin: - backend = "ffmpeg" +class FfmpegBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "ffmpeg"} has_r128_support = True - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - class ReplayGainCliTest: FNAME: str @@ -332,7 +343,7 @@ class ReplayGainGstCliTest( FNAME = "full" # file contains only silence -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdCliTest( ReplayGainCliTest, ReplayGainTestCase, CmdBackendMixin ): @@ -369,7 +380,7 @@ class ReplayGainGstImportTest(ImportTest, ReplayGainTestCase, GstBackendMixin): pass -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdImportTest(ImportTest, ReplayGainTestCase, CmdBackendMixin): pass From 45aa1d550742447e4d57cc6190fc2ad2de6bf7fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 16 Feb 2026 22:05:28 +0000 Subject: [PATCH 12/60] Ensure mp3gain is installed in CI --- .github/workflows/ci.yaml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bfd05c718..0736a0a2f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,7 +35,12 @@ jobs: python-version: ${{ matrix.python-version }} cache: poetry - - name: Install PyGobject and release script dependencies on Ubuntu + - name: Install system dependencies on Windows + if: matrix.platform == 'windows-latest' + run: | + choco install mp3gain -y + + - name: Install system dependencies on Ubuntu if: matrix.platform == 'ubuntu-latest' run: | sudo apt update @@ -43,11 +48,12 @@ jobs: ffmpeg \ gobject-introspection \ gstreamer1.0-plugins-base \ - python3-gst-1.0 \ + imagemagick \ libcairo2-dev \ libgirepository-2.0-dev \ + mp3gain \ pandoc \ - imagemagick + python3-gst-1.0 - name: Get changed lyrics files id: lyrics-update From c220d1f96029d1e85df1174e9557cc201bb4bb32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 17 Feb 2026 01:38:32 +0000 Subject: [PATCH 13/60] Fix mp3gain/aacgain paths on Windows Fixes #2946 --- beetsplug/replaygain.py | 26 ++++++++++++++------------ docs/changelog.rst | 2 ++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e83345059..e69f6b2ee 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -34,7 +34,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui from beets.plugins import BeetsPlugin -from beets.util import command_output, displayable_path, syspath +from beets.util import command_output, syspath if TYPE_CHECKING: import optparse @@ -425,7 +425,7 @@ class FfmpegBackend(Backend): # call ffmpeg self._log.debug("analyzing {}", item) cmd = self._construct_cmd(item, peak_method) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stderr.splitlines() # parse output @@ -643,18 +643,20 @@ class CommandBackend(Backend): # tag-writing; this turns the mp3gain/aacgain tool into a gain # calculator rather than a tag manipulator because we take care # of changing tags ourselves. - cmd: list[str] = [self.command, "-o", "-s", "s"] - if self.noclip: - # Adjust to avoid clipping. - cmd = [*cmd, "-k"] - else: - # Disable clipping warning. - cmd = [*cmd, "-c"] - cmd = [*cmd, "-d", str(int(target_level - 89))] - cmd = cmd + [syspath(i.path) for i in items] + cmd = [ + self.command, + "-o", + "-s", + "s", + # Avoid clipping or disable clipping warning + "-k" if self.noclip else "-c", + "-d", + str(int(target_level - 89)), + *[str(i.filepath) for i in items], + ] self._log.debug("analyzing {} files", len(items)) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stdout self._log.debug("analysis finished") return self.parse_tool_output( diff --git a/docs/changelog.rst b/docs/changelog.rst index 3938d26a4..d3dfa104a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,8 @@ Bug fixes - :doc:`plugins/musicbrainz`: Fix fetching very large releases that have more than 500 tracks. :bug:`6355` - :doc:`plugins/badfiles`: Fix number of found errors in log message +- :doc:`plugins/replaygain`: Avoid magic Windows prefix in calls to command + backends, such as ``mp3gain``. :bug:`2946` .. For plugin developers From 1930400ab80656673a38e456aff7e90f8987c3ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 22 Feb 2026 02:09:25 +0000 Subject: [PATCH 14/60] Set default release/recording includes in MusicBrainzAPI --- beetsplug/_utils/musicbrainz.py | 29 +++++++++++++++++++++++++ beetsplug/musicbrainz.py | 38 +++------------------------------ docs/changelog.rst | 2 ++ 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/beetsplug/_utils/musicbrainz.py b/beetsplug/_utils/musicbrainz.py index 57201e909..95327e75a 100644 --- a/beetsplug/_utils/musicbrainz.py +++ b/beetsplug/_utils/musicbrainz.py @@ -36,6 +36,33 @@ log = logging.getLogger("beets") LUCENE_SPECIAL_CHAR_PAT = re.compile(r'([-+&|!(){}[\]^"~*?:\\/])') +RELEASE_INCLUDES = [ + "artists", + "media", + "recordings", + "release-groups", + "labels", + "artist-credits", + "aliases", + "recording-level-rels", + "work-rels", + "work-level-rels", + "artist-rels", + "isrcs", + "url-rels", + "release-rels", + "genres", + "tags", +] + +RECORDING_INCLUDES = [ + "artists", + "aliases", + "isrcs", + "work-level-rels", + "artist-rels", +] + class LimiterTimeoutSession(LimiterMixin, TimeoutAndRetrySession): """HTTP session that enforces rate limits.""" @@ -223,12 +250,14 @@ class MusicBrainzAPI(RequestHandler): def get_release(self, id_: str, **kwargs: Unpack[LookupKwargs]) -> JSONDict: """Retrieve a release by its MusicBrainz ID.""" + kwargs.setdefault("includes", RELEASE_INCLUDES) return self._lookup("release", id_, **kwargs) def get_recording( self, id_: str, **kwargs: Unpack[LookupKwargs] ) -> JSONDict: """Retrieve a recording by its MusicBrainz ID.""" + kwargs.setdefault("includes", RECORDING_INCLUDES) return self._lookup("recording", id_, **kwargs) def get_work(self, id_: str, **kwargs: Unpack[LookupKwargs]) -> JSONDict: diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 9f4581004..ffef366ae 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -60,34 +60,6 @@ FIELDS_TO_MB_KEYS = { "alias": "alias", } - -RELEASE_INCLUDES = [ - "artists", - "media", - "recordings", - "release-groups", - "labels", - "artist-credits", - "aliases", - "recording-level-rels", - "work-rels", - "work-level-rels", - "artist-rels", - "isrcs", - "url-rels", - "release-rels", - "genres", - "tags", -] - -TRACK_INCLUDES = [ - "artists", - "aliases", - "isrcs", - "work-level-rels", - "artist-rels", -] - BROWSE_INCLUDES = [ "artist-credits", "work-rels", @@ -797,7 +769,7 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): # A 404 error here is fine. e.g. re-importing a release that has # been deleted on MusicBrainz. try: - res = self.mb_api.get_release(albumid, includes=RELEASE_INCLUDES) + res = self.mb_api.get_release(albumid) except HTTPNotFoundError: self._log.debug("Release {} not found on MusicBrainz.", albumid) return None @@ -813,9 +785,7 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): rel["type"] == "transl-tracklisting" and rel["direction"] == "backward" ): - actual_res = self.mb_api.get_release( - rel["release"]["id"], includes=RELEASE_INCLUDES - ) + actual_res = self.mb_api.get_release(rel["release"]["id"]) # release is potentially a pseudo release release = self.album_info(res) @@ -838,8 +808,6 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): return None with suppress(HTTPNotFoundError): - return self.track_info( - self.mb_api.get_recording(trackid, includes=TRACK_INCLUDES) - ) + return self.track_info(self.mb_api.get_recording(trackid)) return None diff --git a/docs/changelog.rst b/docs/changelog.rst index d3dfa104a..b15eb2944 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,6 +26,8 @@ Bug fixes - :doc:`plugins/badfiles`: Fix number of found errors in log message - :doc:`plugins/replaygain`: Avoid magic Windows prefix in calls to command backends, such as ``mp3gain``. :bug:`2946` +- :doc:`plugins/mbpseudo`: Fix crash due to missing ``artist_credit`` field in + the MusicBrainz API response. :bug:`6339` .. For plugin developers From 13e978ca0e91d68b002257e45b06904976f82936 Mon Sep 17 00:00:00 2001 From: snejus Date: Sun, 22 Feb 2026 16:04:40 +0000 Subject: [PATCH 15/60] Increment version to 2.6.2 --- beets/__init__.py | 2 +- docs/changelog.rst | 19 +++++++++++++++++++ docs/conf.py | 2 +- pyproject.toml | 2 +- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/beets/__init__.py b/beets/__init__.py index 750efb3b3..cfa5be2c4 100644 --- a/beets/__init__.py +++ b/beets/__init__.py @@ -19,7 +19,7 @@ import confuse from .util.deprecation import deprecate_imports -__version__ = "2.6.1" +__version__ = "2.6.2" __author__ = "Adrian Sampson " diff --git a/docs/changelog.rst b/docs/changelog.rst index b15eb2944..8abed1367 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,25 @@ below! Unreleased ---------- +.. + New features + ~~~~~~~~~~~~ + +.. + Bug fixes + ~~~~~~~~~ + +.. + For plugin developers + ~~~~~~~~~~~~~~~~~~~~~ + +.. + Other changes + ~~~~~~~~~~~~~ + +2.6.2 (February 22, 2026) +------------------------- + .. New features ~~~~~~~~~~~~ diff --git a/docs/conf.py b/docs/conf.py index 15ba699cd..f66e91645 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -19,7 +19,7 @@ copyright = "2016, Adrian Sampson" master_doc = "index" language = "en" version = "2.6" -release = "2.6.1" +release = "2.6.2" # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration diff --git a/pyproject.toml b/pyproject.toml index 57a12fa75..0ce774d9a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "beets" -version = "2.6.1" +version = "2.6.2" description = "music tagger and library organizer" authors = ["Adrian Sampson "] maintainers = ["Serene-Arc"] From 9d237d10fc3149e1b851cb1aa5d521bff20e2f7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 22 Feb 2026 00:21:09 +0000 Subject: [PATCH 16/60] Fix multi-value delimiter handling in templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use '\␀' as the DB delimiter while formatting lists with '; ' for templates. - Update DelimitedString parsing to accept both separators: * '\␀' for the values from the DB * '; ' for the rest of parsed values (for example `beet modify genres="eletronic; jazz"`) - Refresh %first docs and tests to reflect multi-value field behavior. --- beets/dbcore/types.py | 28 ++++++++++++++++++++-------- beets/library/models.py | 4 ++-- docs/changelog.rst | 10 +++++++--- docs/conf.py | 3 ++- docs/reference/cli.rst | 3 +++ docs/reference/pathformat.rst | 33 ++++++++++++++++++++++++++++----- test/test_library.py | 12 ++++++------ test/test_plugins.py | 3 +-- 8 files changed, 69 insertions(+), 27 deletions(-) diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 61336d9ce..3f94afd05 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -288,25 +288,37 @@ class String(BaseString[str, Any]): class DelimitedString(BaseString[list[str], list[str]]): - """A list of Unicode strings, represented in-database by a single string + r"""A list of Unicode strings, represented in-database by a single string containing delimiter-separated values. + + In template evaluation the list is formatted by joining the values with + a fixed '; ' delimiter regardless of the database delimiter. That is because + the '\␀' character used for multi-value fields is mishandled on Windows + as it contains a backslash character. """ model_type = list[str] + fmt_delimiter = "; " - def __init__(self, delimiter: str): - self.delimiter = delimiter + def __init__(self, db_delimiter: str): + self.db_delimiter = db_delimiter def format(self, value: list[str]): - return self.delimiter.join(value) + return self.fmt_delimiter.join(value) def parse(self, string: str): if not string: return [] - return string.split(self.delimiter) + + delimiter = ( + self.db_delimiter + if self.db_delimiter in string + else self.fmt_delimiter + ) + return string.split(delimiter) def to_sql(self, model_value: list[str]): - return self.delimiter.join(model_value) + return self.db_delimiter.join(model_value) class Boolean(Type): @@ -464,7 +476,7 @@ NULL_FLOAT = NullFloat() STRING = String() BOOLEAN = Boolean() DATE = DateType() -SEMICOLON_SPACE_DSV = DelimitedString(delimiter="; ") +SEMICOLON_SPACE_DSV = DelimitedString("; ") # Will set the proper null char in mediafile -MULTI_VALUE_DSV = DelimitedString(delimiter="\\␀") +MULTI_VALUE_DSV = DelimitedString("\\␀") diff --git a/beets/library/models.py b/beets/library/models.py index e26f2ced3..373c07ee3 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -1546,8 +1546,8 @@ class DefaultTemplateFunctions: s: the string count: The number of items included skip: The number of items skipped - sep: the separator. Usually is '; ' (default) or '/ ' - join_str: the string which will join the items, default '; '. + sep: the separator + join_str: the string which will join the items """ skip = int(skip) count = skip + int(count) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8abed1367..eff2e2cc8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,9 +21,13 @@ Unreleased For plugin developers ~~~~~~~~~~~~~~~~~~~~~ -.. - Other changes - ~~~~~~~~~~~~~ +Other changes +~~~~~~~~~~~~~ + +- :ref:`modify-cmd`: Use the following separator to delimite multiple field + values: |semicolon_space|. For example ``beet modify albumtypes="album; ep"``. + Previously, ``\␀`` was used as a separator. This applies to fields such as + ``artists``, ``albumtypes`` etc. 2.6.2 (February 22, 2026) ------------------------- diff --git a/docs/conf.py b/docs/conf.py index f66e91645..8a812d159 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -98,7 +98,7 @@ man_pages = [ ] # Global substitutions that can be used anywhere in the documentation. -rst_epilog = """ +rst_epilog = r""" .. |Album| replace:: :class:`~beets.library.models.Album` .. |AlbumInfo| replace:: :class:`beets.autotag.hooks.AlbumInfo` .. |BeetsPlugin| replace:: :class:`beets.plugins.BeetsPlugin` @@ -108,6 +108,7 @@ rst_epilog = """ .. |Library| replace:: :class:`~beets.library.library.Library` .. |Model| replace:: :class:`~beets.dbcore.db.Model` .. |TrackInfo| replace:: :class:`beets.autotag.hooks.TrackInfo` +.. |semicolon_space| replace:: :literal:`; \ ` """ # -- Options for HTML output ------------------------------------------------- diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index c0274553a..15024022b 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -267,6 +267,9 @@ Values can also be *templates*, using the same syntax as :doc:`path formats artist sort name into the artist field for all your tracks, and ``beet modify title='$track $title'`` will add track numbers to their title metadata. +To adjust a multi-valued field, such as ``genres``, separate the values with +|semicolon_space|. For example, ``beet modify genres="rock; pop"``. + The ``-a`` option changes to querying album fields instead of track fields and also enables to operate on albums in addition to the individual tracks. Without this flag, the command will only change *track-level* data, even if all the diff --git a/docs/reference/pathformat.rst b/docs/reference/pathformat.rst index 10dd3ae05..aff48a7c6 100644 --- a/docs/reference/pathformat.rst +++ b/docs/reference/pathformat.rst @@ -75,11 +75,34 @@ These functions are built in to beets: - ``%time{date_time,format}``: Return the date and time in any format accepted by strftime_. For example, to get the year some music was added to your library, use ``%time{$added,%Y}``. -- ``%first{text}``: Returns the first item, separated by ``;`` (a semicolon - followed by a space). You can use ``%first{text,count,skip}``, where ``count`` - is the number of items (default 1) and ``skip`` is number to skip (default 0). - You can also use ``%first{text,count,skip,sep,join}`` where ``sep`` is the - separator, like ``;`` or ``/`` and join is the text to concatenate the items. +- ``%first{text,count,skip,sep,join}``: Extract a subset of items from a + delimited string. Splits ``text`` by ``sep``, skips the first ``skip`` items, + then returns the next ``count`` items joined by ``join``. + + This is especially useful for multi-valued fields like ``artists`` or + ``genres`` where you may only want the first artist or a limited number of + genres in a path. + + Defaults: + + .. + Comically, you need to follow |semicolon_space| with some punctuation to + make sure it gets rendered correctly as '; ' in the docs. + + - **count**: 1, + - **skip**: 0, + - **sep**: |semicolon_space|, + - **join**: |semicolon_space|. + + Examples: + + :: + + %first{$genres} → returns the first genre + %first{$genres,2} → returns the first two genres, joined by "; " + %first{$genres,2,1} → skips the first genre, returns the next two + %first{$genres,2,0, , -> } → splits by space, joins with " -> " + - ``%ifdef{field}``, ``%ifdef{field,truetext}`` or ``%ifdef{field,truetext,falsetext}``: Checks if an flexible attribute ``field`` is defined. If it exists, then return ``truetext`` or ``field`` diff --git a/test/test_library.py b/test/test_library.py index 4acf34746..4df4e4b58 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -688,14 +688,14 @@ class DestinationFunctionTest(BeetsTestCase, PathFormattingMixin): self._assert_dest(b"/base/not_played") def test_first(self): - self.i.genres = "Pop; Rock; Classical Crossover" - self._setf("%first{$genres}") - self._assert_dest(b"/base/Pop") + self.i.albumtypes = ["album", "compilation"] + self._setf("%first{$albumtypes}") + self._assert_dest(b"/base/album") def test_first_skip(self): - self.i.genres = "Pop; Rock; Classical Crossover" - self._setf("%first{$genres,1,2}") - self._assert_dest(b"/base/Classical Crossover") + self.i.albumtype = "album; ep; compilation" + self._setf("%first{$albumtype,1,2}") + self._assert_dest(b"/base/compilation") def test_first_different_sep(self): self._setf("%first{Alice / Bob / Eve,2,0, / , & }") diff --git a/test/test_plugins.py b/test/test_plugins.py index 4786b12b4..9622fb8db 100644 --- a/test/test_plugins.py +++ b/test/test_plugins.py @@ -96,8 +96,7 @@ class TestPluginRegistration(IOMixin, PluginTestCase): item.add(self.lib) out = self.run_with_output("ls", "-f", "$multi_value") - delimiter = types.MULTI_VALUE_DSV.delimiter - assert out == f"one{delimiter}two{delimiter}three\n" + assert out == "one; two; three\n" class PluginImportTestCase(ImportHelper, PluginTestCase): From 31f79f14a310937a26fb325bfd5fe2c54a93cc25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 22 Feb 2026 01:12:04 +0000 Subject: [PATCH 17/60] Colorize multi-valued field changes distinctly --- beets/ui/__init__.py | 20 ++++++++++++++++++-- docs/changelog.rst | 1 + test/ui/test_field_diff.py | 15 +++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 4c93d66d8..ddd5ee98b 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1048,6 +1048,18 @@ def print_newline_layout( FLOAT_EPSILON = 0.01 +def _multi_value_diff(field: str, oldset: set[str], newset: set[str]) -> str: + added = newset - oldset + removed = oldset - newset + + parts = [ + f"{field}:", + *(colorize("text_diff_removed", f" - {i}") for i in sorted(removed)), + *(colorize("text_diff_added", f" + {i}") for i in sorted(added)), + ] + return "\n".join(parts) + + def _field_diff( field: str, old: FormattedMapping, new: FormattedMapping ) -> str | None: @@ -1063,6 +1075,11 @@ def _field_diff( ): return None + if isinstance(oldval, list): + if (oldset := set(oldval)) != (newset := set(newval)): + return _multi_value_diff(field, oldset, newset) + return None + # Get formatted values for output. oldstr, newstr = old.get(field, ""), new.get(field, "") if field not in new: @@ -1071,8 +1088,7 @@ def _field_diff( if field not in old: return colorize("text_diff_added", f"{field}: {newstr}") - # For strings, highlight changes. For others, colorize the whole - # thing. + # For strings, highlight changes. For others, colorize the whole thing. if isinstance(oldval, str): oldstr, newstr = colordiff(oldstr, newstr) else: diff --git a/docs/changelog.rst b/docs/changelog.rst index eff2e2cc8..58ad3d58f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,7 @@ Other changes values: |semicolon_space|. For example ``beet modify albumtypes="album; ep"``. Previously, ``\␀`` was used as a separator. This applies to fields such as ``artists``, ``albumtypes`` etc. +- Improve highlighting of multi-valued fields changes. 2.6.2 (February 22, 2026) ------------------------- diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py index 35f3c6ca7..8480d7d7c 100644 --- a/test/ui/test_field_diff.py +++ b/test/ui/test_field_diff.py @@ -1,3 +1,5 @@ +from textwrap import dedent + import pytest from beets.library import Item @@ -38,6 +40,19 @@ class TestFieldDiff: p({"mb_trackid": None}, {"mb_trackid": "1234"}, "mb_trackid", "mb_trackid: -> [text_diff_added]1234[/]", id="none_to_value"), p({}, {"new_flex": "foo"}, "new_flex", "[text_diff_added]new_flex: foo[/]", id="flex_field_added"), p({"old_flex": "foo"}, {}, "old_flex", "[text_diff_removed]old_flex: foo[/]", id="flex_field_removed"), + p({"albumtypes": ["album", "ep"]}, {"albumtypes": ["ep", "album"]}, "albumtypes", None, id="multi_value_unchanged"), + p( + {"albumtypes": ["ep"]}, + {"albumtypes": ["album", "compilation"]}, + "albumtypes", + dedent(""" + albumtypes: + [text_diff_removed] - ep[/] + [text_diff_added] + album[/] + [text_diff_added] + compilation[/] + """).strip(), + id="multi_value_changed" + ), ], ) # fmt: skip @pytest.mark.parametrize("color", [True], ids=["color_enabled"]) From 4699958f253eb27263f2bc3eef385e9ce7793673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 22 Feb 2026 01:36:02 +0000 Subject: [PATCH 18/60] Remove redundant coloring logic --- beets/ui/__init__.py | 51 ++++++---------------------- beets/ui/commands/import_/display.py | 2 +- test/ui/test_field_diff.py | 2 +- 3 files changed, 13 insertions(+), 42 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index ddd5ee98b..ad14fe1f8 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -571,15 +571,16 @@ def get_color_config() -> dict[ColorName, str]: } -def colorize(color_name: ColorName, text: str) -> str: - """Apply ANSI color formatting to text based on configuration settings. +def _colorize(color_name: ColorName, text: str) -> str: + """Apply ANSI color formatting to text based on configuration settings.""" + color_code = get_color_config()[color_name] + return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}" - Returns colored text when color output is enabled and NO_COLOR environment - variable is not set, otherwise returns plain text unchanged. - """ + +def colorize(color_name: ColorName, text: str) -> str: + """Colorize text when color output is enabled.""" if config["ui"]["color"] and "NO_COLOR" not in os.environ: - color_code = get_color_config()[color_name] - return f"{COLOR_ESCAPE}[{color_code}m{text}{RESET_COLOR}" + return _colorize(color_name, text) return text @@ -643,32 +644,12 @@ def color_len(colored_text): return len(uncolorize(colored_text)) -def _colordiff(a: Any, b: Any) -> tuple[str, str]: - """Given two values, return the same pair of strings except with - their differences highlighted in the specified color. Strings are - highlighted intelligently to show differences; other values are - stringified and highlighted in their entirety. - """ - # First, convert paths to readable format - if isinstance(a, bytes) or isinstance(b, bytes): - # A path field. - a = util.displayable_path(a) - b = util.displayable_path(b) - - if not isinstance(a, str) or not isinstance(b, str): - # Non-strings: use ordinary equality. - if a == b: - return str(a), str(b) - else: - return ( - colorize("text_diff_removed", str(a)), - colorize("text_diff_added", str(b)), - ) - +def colordiff(a: str, b: str) -> tuple[str, str]: + """Intelligently highlight the differences between two strings.""" before = "" after = "" - matcher = SequenceMatcher(lambda x: False, a, b) + matcher = SequenceMatcher(lambda _: False, a, b) for op, a_start, a_end, b_start, b_end in matcher.get_opcodes(): before_part, after_part = a[a_start:a_end], b[b_start:b_end] if op in {"delete", "replace"}: @@ -682,16 +663,6 @@ def _colordiff(a: Any, b: Any) -> tuple[str, str]: return before, after -def colordiff(a, b): - """Colorize differences between two values if color is enabled. - (Like _colordiff but conditional.) - """ - if config["ui"]["color"]: - return _colordiff(a, b) - else: - return str(a), str(b) - - def get_path_formats(subview=None): """Get the configuration's path formats as a list of query/template pairs. diff --git a/beets/ui/commands/import_/display.py b/beets/ui/commands/import_/display.py index bdc44d51f..764dafd39 100644 --- a/beets/ui/commands/import_/display.py +++ b/beets/ui/commands/import_/display.py @@ -127,7 +127,7 @@ class ChangeRepresentation: and artist name. """ # Artist. - artist_l, artist_r = self.cur_artist or "", self.match.info.artist + artist_l, artist_r = self.cur_artist or "", self.match.info.artist or "" if artist_r == VARIOUS_ARTISTS: # Hide artists for VA releases. artist_l, artist_r = "", "" diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py index 8480d7d7c..d42e55a93 100644 --- a/test/ui/test_field_diff.py +++ b/test/ui/test_field_diff.py @@ -17,7 +17,7 @@ class TestFieldDiff: def patch_colorize(self, monkeypatch): """Patch to return a deterministic string format instead of ANSI codes.""" monkeypatch.setattr( - "beets.ui.colorize", + "beets.ui._colorize", lambda color_name, text: f"[{color_name}]{text}[/]", ) From edfe00516f24d77b625a2e5b1c6557bc710438dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 23 Feb 2026 00:33:24 +0000 Subject: [PATCH 19/60] Handle DelimitedString fields as native lists in edit plugin Treat DelimitedString as a safe YAML-editable type in the edit plugin, allowing multi-valued fields to be edited as native lists. --- beets/dbcore/types.py | 6 ++++-- beetsplug/edit.py | 7 ++++++- docs/changelog.rst | 3 +++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 3f94afd05..8907584a4 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -66,6 +66,8 @@ class Type(ABC, Generic[T, N]): """The `Query` subclass to be used when querying the field. """ + # For sequence-like types, keep ``model_type`` unsubscripted as it's used + # for ``isinstance`` checks. Use ``list`` instead of ``list[str]`` model_type: type[T] """The Python type that is used to represent the value in the model. @@ -287,7 +289,7 @@ class String(BaseString[str, Any]): model_type = str -class DelimitedString(BaseString[list[str], list[str]]): +class DelimitedString(BaseString[list, list]): # type: ignore[type-arg] r"""A list of Unicode strings, represented in-database by a single string containing delimiter-separated values. @@ -297,7 +299,7 @@ class DelimitedString(BaseString[list[str], list[str]]): as it contains a backslash character. """ - model_type = list[str] + model_type = list fmt_delimiter = "; " def __init__(self, db_delimiter: str): diff --git a/beetsplug/edit.py b/beetsplug/edit.py index 46e756122..0f358c9a1 100644 --- a/beetsplug/edit.py +++ b/beetsplug/edit.py @@ -30,7 +30,12 @@ from beets.util import PromptChoice # These "safe" types can avoid the format/parse cycle that most fields go # through: they are safe to edit with native YAML types. -SAFE_TYPES = (types.BaseFloat, types.BaseInteger, types.Boolean) +SAFE_TYPES = ( + types.BaseFloat, + types.BaseInteger, + types.Boolean, + types.DelimitedString, +) class ParseError(Exception): diff --git a/docs/changelog.rst b/docs/changelog.rst index 58ad3d58f..9f73a5725 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -29,6 +29,9 @@ Other changes Previously, ``\␀`` was used as a separator. This applies to fields such as ``artists``, ``albumtypes`` etc. - Improve highlighting of multi-valued fields changes. +- :doc:`plugins/edit`: Editing multi-valued fields now behaves more naturally, + with list values handled directly to make metadata edits smoother and more + predictable. 2.6.2 (February 22, 2026) ------------------------- From 6fe7a9a7d387f525e8626b8fc5563db94ca5afd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 23 Feb 2026 03:53:56 +0000 Subject: [PATCH 20/60] Add @snejus to team --- docs/team.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/team.rst b/docs/team.rst index b23b125ce..7ea12867a 100644 --- a/docs/team.rst +++ b/docs/team.rst @@ -74,6 +74,17 @@ of what you can expect from these *knowledge owners*. - Code quality - Typing +@snejus +------- + +- Grug-minded approach: simple, obvious solutions over clever complexity +- MusicBrainz/autotagger internals and source-plugin behavior +- Query/path handling and SQL-backed lookup behavior +- Typing and tooling modernization (mypy, Ruff, annotations) +- Test architecture, CI reliability, and coverage improvements +- Release and packaging workflows (Poetry/pyproject, dependencies, changelog) +- Cross-plugin refactors (especially metadata and lyrics-related internals) + @wisp3rwind ----------- From dcef1f4ea40d72e6b33a028ad5479003b66af362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 23 Feb 2026 05:09:03 +0000 Subject: [PATCH 21/60] Create .instructions.md for Copilot Let's test this --- .github/copilot-instructions.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/copilot-instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..9776da711 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,22 @@ +## PR Review Voice + +When reviewing pull requests, respond entirely in the voice of the Grug Brained Developer. +Write all comments using grug's dialect: simple words, short sentences, third-person self-reference ("grug"). + +Core grug beliefs to apply when reviewing: + +- Complexity very, very bad. Flag any complexity demon spirit entering codebase. + Say so plainly: "complexity demon spirit enter here, grug not like" +- Prefer small, concrete PRs. Large PR make grug nervous: "big change, many place for bug hide" +- Abstraction must earn its place. Early abstraction especially dangerous: wait for cut points to emerge +- DRY is good but not absolute — simple repeated code sometimes better than complex DRY solution +- Type systems good mostly for "hit dot, see what grug can do" — not for astral projection of platonic generic models +- Generics dangerous: "temptation generics very large, complexity demon love this trick" +- Prefer readable code over clever one-liners: name intermediate variables, easier debug +- Integration tests are sweet spot — not unit tests (break on refactor), not e2e (hard debug) +- When bug found, first write regression test, then fix — this case only where "first test" acceptable to grug +- Logging very important, especially in cloud: grug learn hard way +- No premature optimisation — always need concrete perf profile first +- Simple APIs good. Layered APIs ok. Java streams make grug reach for club +- SPA frameworks increase complexity demon surface area — be suspicious +- Saying "this too complex for grug" is senior developer superpower — remove Fear Of Looking Dumb (FOLD) From 70bf57baf6cb8de837df0483b1723896b5aa8c05 Mon Sep 17 00:00:00 2001 From: Johann Fot Date: Sun, 7 Dec 2025 00:22:26 +0100 Subject: [PATCH 22/60] Add native support for multiple genres per album/track Simplify multi-genre implementation based on maintainer feedback (PR #6169). Changes: - Remove multi_value_genres and genre_separator config options - Replace complex sync_genre_fields() with ensure_first_value('genre', 'genres') - Update all plugins (Beatport, MusicBrainz, LastGenre) to always write genres as lists - Add automatic migration for comma/semicolon/slash-separated genre strings - Add 'beet migrate genres' command for explicit batch migration with --pretend flag - Update all tests to reflect simplified approach (44 tests passing) - Update documentation Implementation aligns with maintainer vision of always using multi-value genres internally with automatic backward-compatible sync to the genre field via ensure_first_value(), eliminating configuration complexity. Migration strategy avoids problems from #5540: - Automatic lazy migration on item access (no reimport/mbsync needed) - Optional batch migration command for user control - No endless rewrite loops due to proper field synchronization --- beets/autotag/__init__.py | 31 +++++++++ beets/autotag/hooks.py | 2 + beets/library/models.py | 3 + beets/ui/commands/__init__.py | 10 +-- beets/ui/commands/migrate.py | 98 +++++++++++++++++++++++++++ beetsplug/beatport.py | 17 +++-- beetsplug/lastgenre/__init__.py | 55 +++++++-------- beetsplug/musicbrainz.py | 5 +- docs/changelog.rst | 21 +++++- test/plugins/test_beatport.py | 9 ++- test/plugins/test_lastgenre.py | 98 ++++++++++++++++++--------- test/plugins/test_musicbrainz.py | 4 +- test/test_autotag.py | 113 +++++++++++++++++++++++++++++++ test/test_library.py | 28 ++++++++ 14 files changed, 416 insertions(+), 78 deletions(-) create mode 100644 beets/ui/commands/migrate.py diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index feeefbf28..eabc41833 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -166,7 +166,38 @@ def correct_list_fields(m: LibModel) -> None: elif list_val: setattr(m, single_field, list_val[0]) + def migrate_legacy_genres() -> None: + """Migrate comma-separated genre strings to genres list. + + For users upgrading from previous versions, their genre field may + contain comma-separated values (e.g., "Rock, Alternative, Indie"). + This migration splits those values into the genres list on first access, + avoiding the need to reimport the entire library. + """ + genre_val = getattr(m, "genre", "") + genres_val = getattr(m, "genres", []) + + # Only migrate if genres list is empty and genre contains separators + if not genres_val and genre_val: + # Try common separators used by lastgenre and other tools + for separator in [", ", "; ", " / "]: + if separator in genre_val: + # Split and clean the genre string + split_genres = [ + g.strip() + for g in genre_val.split(separator) + if g.strip() + ] + if len(split_genres) > 1: + # Found a valid split - populate genres list + setattr(m, "genres", split_genres) + # Clear genre so ensure_first_value sets it correctly + setattr(m, "genre", "") + break + ensure_first_value("albumtype", "albumtypes") + migrate_legacy_genres() + ensure_first_value("genre", "genres") if hasattr(m, "mb_artistids"): ensure_first_value("mb_artistid", "mb_artistids") diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 82e685b7a..7818d5f21 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -76,6 +76,7 @@ class Info(AttrDict[Any]): data_source: str | None = None, data_url: str | None = None, genre: str | None = None, + genres: list[str] | None = None, media: str | None = None, **kwargs, ) -> None: @@ -91,6 +92,7 @@ class Info(AttrDict[Any]): self.data_source = data_source self.data_url = data_url self.genre = genre + self.genres = genres or [] self.media = media self.update(kwargs) diff --git a/beets/library/models.py b/beets/library/models.py index 373c07ee3..71445c203 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -242,6 +242,7 @@ class Album(LibModel): "albumartists_credit": types.MULTI_VALUE_DSV, "album": types.STRING, "genre": types.STRING, + "genres": types.MULTI_VALUE_DSV, "style": types.STRING, "discogs_albumid": types.INTEGER, "discogs_artistid": types.INTEGER, @@ -298,6 +299,7 @@ class Album(LibModel): "albumartists_credit", "album", "genre", + "genres", "style", "discogs_albumid", "discogs_artistid", @@ -651,6 +653,7 @@ class Item(LibModel): "albumartist_credit": types.STRING, "albumartists_credit": types.MULTI_VALUE_DSV, "genre": types.STRING, + "genres": types.MULTI_VALUE_DSV, "style": types.STRING, "discogs_albumid": types.INTEGER, "discogs_artistid": types.INTEGER, diff --git a/beets/ui/commands/__init__.py b/beets/ui/commands/__init__.py index e1d0389a3..b4eebb53f 100644 --- a/beets/ui/commands/__init__.py +++ b/beets/ui/commands/__init__.py @@ -24,6 +24,7 @@ from .fields import fields_cmd from .help import HelpCommand from .import_ import import_cmd from .list import list_cmd +from .migrate import migrate_cmd from .modify import modify_cmd from .move import move_cmd from .remove import remove_cmd @@ -52,12 +53,13 @@ default_commands = [ HelpCommand(), import_cmd, list_cmd, - update_cmd, - remove_cmd, - stats_cmd, - version_cmd, + migrate_cmd, modify_cmd, move_cmd, + remove_cmd, + stats_cmd, + update_cmd, + version_cmd, write_cmd, config_cmd, completion_cmd, diff --git a/beets/ui/commands/migrate.py b/beets/ui/commands/migrate.py new file mode 100644 index 000000000..2cb7e0d59 --- /dev/null +++ b/beets/ui/commands/migrate.py @@ -0,0 +1,98 @@ +"""The 'migrate' command: migrate library data for format changes.""" + +from beets import logging, ui +from beets.autotag import correct_list_fields + +# Global logger. +log = logging.getLogger("beets") + + +def migrate_genres(lib, pretend=False): + """Migrate comma-separated genre strings to genres list. + + For users upgrading from previous versions, their genre field may + contain comma-separated values (e.g., "Rock, Alternative, Indie"). + This command splits those values into the genres list, avoiding + the need to reimport the entire library. + """ + items = lib.items() + migrated_count = 0 + total_items = 0 + + ui.print_("Scanning library for items with comma-separated genres...") + + for item in items: + total_items += 1 + genre_val = item.genre or "" + genres_val = item.genres or [] + + # Check if migration is needed + needs_migration = False + if not genres_val and genre_val: + for separator in [", ", "; ", " / "]: + if separator in genre_val: + split_genres = [ + g.strip() + for g in genre_val.split(separator) + if g.strip() + ] + if len(split_genres) > 1: + needs_migration = True + break + + if needs_migration: + migrated_count += 1 + old_genre = item.genre + old_genres = item.genres or [] + + if pretend: + # Just show what would change + ui.print_( + f" Would migrate: {item.artist} - {item.title}\n" + f" genre: {old_genre!r} -> {split_genres[0]!r}\n" + f" genres: {old_genres!r} -> {split_genres!r}" + ) + else: + # Actually migrate + correct_list_fields(item) + item.store() + log.debug( + "migrated: {} - {} ({} -> {})", + item.artist, + item.title, + old_genre, + item.genres, + ) + + # Show summary + if pretend: + ui.print_( + f"\nWould migrate {migrated_count} of {total_items} items " + f"(run without --pretend to apply changes)" + ) + else: + ui.print_( + f"\nMigrated {migrated_count} of {total_items} items with " + f"comma-separated genres" + ) + + +def migrate_func(lib, opts, args): + """Handle the migrate command.""" + if not args or args[0] == "genres": + migrate_genres(lib, pretend=opts.pretend) + else: + raise ui.UserError(f"unknown migration target: {args[0]}") + + +migrate_cmd = ui.Subcommand( + "migrate", help="migrate library data for format changes" +) +migrate_cmd.parser.add_option( + "-p", + "--pretend", + action="store_true", + help="show what would be changed without applying", +) +migrate_cmd.parser.usage = "%prog migrate genres [options]" +migrate_cmd.func = migrate_func diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 718e0730e..8918a10cb 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -234,7 +234,9 @@ class BeatportObject: if "artists" in data: self.artists = [(x["id"], str(x["name"])) for x in data["artists"]] if "genres" in data: - self.genres = [str(x["name"]) for x in data["genres"]] + genre_list = [str(x["name"]) for x in data["genres"]] + # Remove duplicates while preserving order + self.genres = list(dict.fromkeys(genre_list)) def artists_str(self) -> str | None: if self.artists is not None: @@ -306,11 +308,16 @@ class BeatportTrack(BeatportObject): self.bpm = data.get("bpm") self.initial_key = str((data.get("key") or {}).get("shortName")) - # Use 'subgenre' and if not present, 'genre' as a fallback. + # Extract genres list from subGenres or genres if data.get("subGenres"): - self.genre = str(data["subGenres"][0].get("name")) + genre_list = [str(x.get("name")) for x in data["subGenres"]] elif data.get("genres"): - self.genre = str(data["genres"][0].get("name")) + genre_list = [str(x.get("name")) for x in data["genres"]] + else: + genre_list = [] + + # Remove duplicates while preserving order + self.genres = list(dict.fromkeys(genre_list)) class BeatportPlugin(MetadataSourcePlugin): @@ -484,6 +491,7 @@ class BeatportPlugin(MetadataSourcePlugin): data_source=self.data_source, data_url=release.url, genre=release.genre, + genres=release.genres, year=release_date.year if release_date else None, month=release_date.month if release_date else None, day=release_date.day if release_date else None, @@ -509,6 +517,7 @@ class BeatportPlugin(MetadataSourcePlugin): bpm=track.bpm, initial_key=track.initial_key, genre=track.genre, + genres=track.genres, ) def _get_artist(self, artists): diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index f7aef0261..a75af0aec 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -329,26 +329,23 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Main processing: _get_genre() and helpers. - def _format_and_stringify(self, tags: list[str]) -> str: - """Format to title_case if configured and return as delimited string.""" + def _format_genres(self, tags: list[str]) -> list[str]: + """Format to title_case if configured and return as list.""" if self.config["title_case"]: - formatted = [tag.title() for tag in tags] + return [tag.title() for tag in tags] else: - formatted = tags - - return self.config["separator"].as_str().join(formatted) + return tags def _get_existing_genres(self, obj: LibModel) -> list[str]: """Return a list of genres for this Item or Album. Empty string genres are removed.""" - separator = self.config["separator"].get() if isinstance(obj, library.Item): - item_genre = obj.get("genre", with_album=False).split(separator) + genres_list = obj.get("genres", with_album=False) else: - item_genre = obj.get("genre").split(separator) + genres_list = obj.get("genres") # Filter out empty strings - return [g for g in item_genre if g] + return [g for g in genres_list if g] if genres_list else [] def _combine_resolve_and_log( self, old: list[str], new: list[str] @@ -359,8 +356,8 @@ class LastGenrePlugin(plugins.BeetsPlugin): combined = old + new return self._resolve_genres(combined) - def _get_genre(self, obj: LibModel) -> tuple[str | None, ...]: - """Get the final genre string for an Album or Item object. + def _get_genre(self, obj: LibModel) -> tuple[list[str], str]: + """Get the final genre list for an Album or Item object. `self.sources` specifies allowed genre sources. Starting with the first source in this tuple, the following stages run through until a genre is @@ -370,9 +367,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): - artist, albumartist or "most popular track genre" (for VA-albums) - original fallback - configured fallback - - None + - empty list - A `(genre, label)` pair is returned, where `label` is a string used for + A `(genres, label)` pair is returned, where `label` is a string used for logging. For example, "keep + artist, whitelist" indicates that existing genres were combined with new last.fm genres and whitelist filtering was applied, while "artist, any" means only new last.fm genres are included @@ -391,7 +388,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): label = f"{stage_label}, {suffix}" if keep_genres: label = f"keep + {label}" - return self._format_and_stringify(resolved_genres), label + return self._format_genres(resolved_genres), label return None keep_genres = [] @@ -400,10 +397,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): if genres and not self.config["force"]: # Without force pre-populated tags are returned as-is. - label = "keep any, no-force" - if isinstance(obj, library.Item): - return obj.get("genre", with_album=False), label - return obj.get("genre"), label + return genres, "keep any, no-force" if self.config["force"]: # Force doesn't keep any unless keep_existing is set. @@ -480,8 +474,14 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Nothing found, leave original if configured and valid. if obj.genre and self.config["keep_existing"]: - if not self.whitelist or self._is_valid(obj.genre.lower()): - return obj.genre, "original fallback" + # Check if at least one genre is valid + valid_genres = [ + g + for g in genres + if not self.whitelist or self._is_valid(g.lower()) + ] + if valid_genres: + return valid_genres, "original fallback" else: # If the original genre doesn't match a whitelisted genre, check # if we can canonicalize it to find a matching, whitelisted genre! @@ -490,22 +490,23 @@ class LastGenrePlugin(plugins.BeetsPlugin): ): return result - # Return fallback string. + # Return fallback as a list. if fallback := self.config["fallback"].get(): - return fallback, "fallback" + return [fallback], "fallback" # No fallback configured. - return None, "fallback unconfigured" + return [], "fallback unconfigured" # Beets plugin hooks and CLI. def _fetch_and_log_genre(self, obj: LibModel) -> None: """Fetch genre and log it.""" self._log.info(str(obj)) - obj.genre, label = self._get_genre(obj) - self._log.debug("Resolved ({}): {}", label, obj.genre) + genres_list, label = self._get_genre(obj) + obj.genres = genres_list + self._log.debug("Resolved ({}): {}", label, genres_list) - ui.show_model_changes(obj, fields=["genre"], print_obj=False) + ui.show_model_changes(obj, fields=["genres"], print_obj=False) @singledispatchmethod def _process(self, obj: LibModel, write: bool) -> None: diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index ffef366ae..695ce8790 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -644,10 +644,11 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): for source in sources: for genreitem in source: genres[genreitem["name"]] += int(genreitem["count"]) - info.genre = "; ".join( + genre_list = [ genre for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) - ) + ] + info.genres = genre_list # We might find links to external sources (Discogs, Bandcamp, ...) external_ids = self.config["external_ids"].get() diff --git a/docs/changelog.rst b/docs/changelog.rst index 9f73a5725..2a4caf573 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,9 +9,24 @@ below! Unreleased ---------- -.. - New features - ~~~~~~~~~~~~ +New features +~~~~~~~~~~~~ + +- Add native support for multiple genres per album/track. The ``genres`` field + now stores genres as a list and is written to files as multiple individual + genre tags (e.g., separate GENRE tags for FLAC/MP3). The single ``genre`` + field is automatically synchronized to contain the first genre from the list + for backward compatibility. The :doc:`plugins/musicbrainz`, + :doc:`plugins/beatport`, and :doc:`plugins/lastgenre` plugins have been + updated to populate the ``genres`` field as a list. + + **Migration**: Existing libraries with comma-separated, semicolon-separated, + or slash-separated genre strings (e.g., ``"Rock, Alternative, Indie"``) will + be automatically migrated to the ``genres`` list when items are accessed. No + manual reimport or ``mbsync`` is required. For users who prefer explicit + control, a new ``beet migrate genres`` command is available to migrate the + entire library at once. Use ``beet migrate genres --pretend`` to preview + changes before applying them. .. Bug fixes diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index b92a3bf15..dbfb89c9c 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -583,7 +583,8 @@ class BeatportTest(BeetsTestCase): def test_genre_applied(self): for track, test_track in zip(self.tracks, self.test_tracks): - assert track.genre == test_track.genre + # BeatportTrack now has genres as a list + assert track.genres == [test_track.genre] class BeatportResponseEmptyTest(unittest.TestCase): @@ -634,7 +635,8 @@ class BeatportResponseEmptyTest(unittest.TestCase): self.test_tracks[0]["subGenres"] = [] - assert tracks[0].genre == self.test_tracks[0]["genres"][0]["name"] + # BeatportTrack now has genres as a list + assert tracks[0].genres == [self.test_tracks[0]["genres"][0]["name"]] def test_genre_empty(self): """No 'genre' is provided. Test if 'sub_genre' is applied.""" @@ -643,4 +645,5 @@ class BeatportResponseEmptyTest(unittest.TestCase): self.test_tracks[0]["genres"] = [] - assert tracks[0].genre == self.test_tracks[0]["subGenres"][0]["name"] + # BeatportTrack now has genres as a list + assert tracks[0].genres == [self.test_tracks[0]["subGenres"][0]["name"]] diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index f499992c6..9b510958e 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -80,13 +80,15 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): self._setup_config(canonical="", whitelist={"rock"}) assert self.plugin._resolve_genres(["delta blues"]) == [] - def test_format_and_stringify(self): - """Format genres list and return them as a separator-delimited string.""" + def test_format_genres(self): + """Format genres list with title case if configured.""" self._setup_config(count=2) - assert ( - self.plugin._format_and_stringify(["jazz", "pop", "rock", "blues"]) - == "Jazz, Pop, Rock, Blues" - ) + assert self.plugin._format_genres(["jazz", "pop", "rock", "blues"]) == [ + "Jazz", + "Pop", + "Rock", + "Blues", + ] def test_count_c14n(self): """Keep the n first genres, after having applied c14n when necessary""" @@ -144,7 +146,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): albumartist="Pretend Artist", artist="Pretend Artist", title="Pretend Track", - genre="Original Genre", + genres=["Original Genre"], ) album = self.lib.add_album([item]) @@ -155,10 +157,10 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): with patch("beetsplug.lastgenre.Item.store", unexpected_store): output = self.run_with_output("lastgenre", "--pretend") - assert "Mock Genre" in output + assert "genres:" in output album.load() - assert album.genre == "Original Genre" - assert album.items()[0].genre == "Original Genre" + assert album.genres == ["Original Genre"] + assert album.items()[0].genres == ["Original Genre"] def test_no_duplicate(self): """Remove duplicated genres.""" @@ -219,7 +221,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), # 1 - force and keep whitelisted, unknown original ( @@ -235,7 +237,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazz"], }, - ("Blues, Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), # 2 - force and keep whitelisted on empty tag ( @@ -251,7 +253,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazz"], }, - ("Jazz", "album, whitelist"), + (["Jazz"], "album, whitelist"), ), # 3 force and keep, artist configured ( @@ -268,7 +270,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": ["Jazz"], "artist": ["Pop"], }, - ("Blues, Pop", "keep + artist, whitelist"), + (["Blues", "Pop"], "keep + artist, whitelist"), ), # 4 - don't force, disabled whitelist ( @@ -284,7 +286,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazz"], }, - ("any genre", "keep any, no-force"), + (["any genre"], "keep any, no-force"), ), # 5 - don't force and empty is regular last.fm fetch; no whitelist too ( @@ -300,7 +302,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazzin"], }, - ("Jazzin", "album, any"), + (["Jazzin"], "album, any"), ), # 6 - fallback to next stages until found ( @@ -318,7 +320,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": None, "artist": ["Jazz"], }, - ("Unknown Genre, Jazz", "keep + artist, any"), + (["Unknown Genre", "Jazz"], "keep + artist, any"), ), # 7 - Keep the original genre when force and keep_existing are on, and # whitelist is disabled @@ -338,7 +340,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": None, "artist": None, }, - ("any existing", "original fallback"), + (["any existing"], "original fallback"), ), # 7.1 - Keep the original genre when force and keep_existing are on, and # whitelist is enabled, and genre is valid. @@ -358,7 +360,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": None, "artist": None, }, - ("Jazz", "original fallback"), + (["Jazz"], "original fallback"), ), # 7.2 - Return the configured fallback when force is on but # keep_existing is not. @@ -378,7 +380,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": None, "artist": None, }, - ("fallback genre", "fallback"), + (["fallback genre"], "fallback"), ), # 8 - fallback to fallback if no original ( @@ -397,7 +399,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": None, "artist": None, }, - ("fallback genre", "fallback"), + (["fallback genre"], "fallback"), ), # 9 - null charachter as separator ( @@ -409,12 +411,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "separator": "\u0000", "canonical": False, "prefer_specific": False, + "count": 10, }, "Blues", { "album": ["Jazz"], }, - ("Blues\u0000Jazz", "keep + album, whitelist"), + (["Blues", "Jazz"], "keep + album, whitelist"), ), # 10 - limit a lot of results ( @@ -432,7 +435,10 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazz", "Bebop", "Hardbop"], }, - ("Blues, Rock, Metal, Jazz, Bebop", "keep + album, whitelist"), + ( + ["Blues", "Rock", "Metal", "Jazz", "Bebop"], + "keep + album, whitelist", + ), ), # 11 - force off does not rely on configured separator ( @@ -448,7 +454,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["Jazz", "Bebop"], }, - ("not ; configured | separator", "keep any, no-force"), + (["not ; configured | separator"], "keep any, no-force"), ), # 12 - fallback to next stage (artist) if no allowed original present # and no album genre were fetched. @@ -468,7 +474,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": None, "artist": ["Jazz"], }, - ("Jazz", "keep + artist, whitelist"), + (["Jazz"], "keep + artist, whitelist"), ), # 13 - canonicalization transforms non-whitelisted genres to canonical forms # @@ -488,7 +494,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): { "album": ["acid techno"], }, - ("Techno, Electronic", "album, whitelist"), + (["Techno", "Electronic"], "album, whitelist"), ), # 14 - canonicalization transforms whitelisted genres to canonical forms and # includes originals @@ -512,7 +518,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": ["acid house"], }, ( - "Detroit Techno, Techno, Electronic, Acid House, House", + [ + "Detroit Techno", + "Techno", + "Electronic", + "Acid House", + "House", + ], "keep + album, whitelist", ), ), @@ -537,7 +549,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "album": ["Detroit Techno"], }, ( - "Disco, Electronic, Detroit Techno, Techno", + ["Disco", "Electronic", "Detroit Techno", "Techno"], "keep + album, whitelist", ), ), @@ -556,13 +568,13 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "prefer_specific": False, "count": 10, }, - "Cosmic Disco", + ["Cosmic Disco"], { "album": [], "artist": [], }, ( - "Disco, Electronic", + ["Disco", "Electronic"], "keep + original fallback, whitelist", ), ), @@ -592,9 +604,29 @@ def test_get_genre(config_values, item_genre, mock_genres, expected_result): # Configure plugin.config.set(config_values) plugin.setup() # Loads default whitelist and canonicalization tree + item = _common.item() - item.genre = item_genre + # Set genres as a list - if item_genre is a string, convert it to list + if item_genre: + # For compatibility with old separator-based tests, split if needed + if ( + "separator" in config_values + and config_values["separator"] in item_genre + ): + sep = config_values["separator"] + item.genres = [ + g.strip() for g in item_genre.split(sep) if g.strip() + ] + else: + # Assume comma-separated if no specific separator + if ", " in item_genre: + item.genres = [ + g.strip() for g in item_genre.split(", ") if g.strip() + ] + else: + item.genres = [item_genre] + else: + item.genres = [] # Run - res = plugin._get_genre(item) - assert res == expected_result + assert plugin._get_genre(item) == expected_result diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 8d7c5a2f8..4ebce1b01 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -526,14 +526,14 @@ class MBAlbumInfoTest(MusicBrainzTestCase): config["musicbrainz"]["genres_tag"] = "genre" release = self._make_release() d = self.mb.album_info(release) - assert d.genre == "GENRE" + assert d.genres == ["GENRE"] def test_tags(self): config["musicbrainz"]["genres"] = True config["musicbrainz"]["genres_tag"] = "tag" release = self._make_release() d = self.mb.album_info(release) - assert d.genre == "TAG" + assert d.genres == ["TAG"] def test_no_genres(self): config["musicbrainz"]["genres"] = False diff --git a/test/test_autotag.py b/test/test_autotag.py index 119ca15e8..e094d0ddc 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -475,3 +475,116 @@ def test_correct_list_fields( single_val, list_val = item[single_field], item[list_field] assert (not single_val and not list_val) or single_val == list_val[0] + + +# Tests for multi-value genres functionality +class TestGenreSync: + """Test the genre/genres field synchronization.""" + + def test_genres_list_to_genre_first(self): + """Genres list sets genre to first item.""" + item = Item(genres=["Rock", "Alternative", "Indie"]) + correct_list_fields(item) + + assert item.genre == "Rock" + assert item.genres == ["Rock", "Alternative", "Indie"] + + def test_genre_string_to_genres_list(self): + """Genre string becomes first item in genres list.""" + item = Item(genre="Rock") + correct_list_fields(item) + + assert item.genre == "Rock" + assert item.genres == ["Rock"] + + def test_genre_and_genres_both_present(self): + """When both genre and genres exist, genre becomes first in list.""" + item = Item(genre="Jazz", genres=["Rock", "Alternative"]) + correct_list_fields(item) + + # genre should be prepended to genres list (deduplicated) + assert item.genre == "Jazz" + assert item.genres == ["Jazz", "Rock", "Alternative"] + + def test_empty_genre(self): + """Empty genre field.""" + item = Item(genre="") + correct_list_fields(item) + + assert item.genre == "" + assert item.genres == [] + + def test_empty_genres(self): + """Empty genres list.""" + item = Item(genres=[]) + correct_list_fields(item) + + assert item.genre == "" + assert item.genres == [] + + def test_none_values(self): + """Handle None values in genre/genres fields without errors.""" + # Test with None genre + item = Item(genre=None, genres=["Rock"]) + correct_list_fields(item) + assert item.genres == ["Rock"] + assert item.genre == "Rock" + + # Test with None genres + item = Item(genre="Jazz", genres=None) + correct_list_fields(item) + assert item.genre == "Jazz" + assert item.genres == ["Jazz"] + + def test_none_both(self): + """Handle None in both genre and genres.""" + item = Item(genre=None, genres=None) + correct_list_fields(item) + + assert item.genres == [] + assert item.genre == "" + + def test_migrate_comma_separated_genres(self): + """Migrate legacy comma-separated genre strings.""" + item = Item(genre="Rock, Alternative, Indie", genres=[]) + correct_list_fields(item) + + # Should split into genres list + assert item.genres == ["Rock", "Alternative", "Indie"] + # Genre becomes first item after migration + assert item.genre == "Rock" + + def test_migrate_semicolon_separated_genres(self): + """Migrate legacy semicolon-separated genre strings.""" + item = Item(genre="Rock; Alternative; Indie", genres=[]) + correct_list_fields(item) + + assert item.genres == ["Rock", "Alternative", "Indie"] + assert item.genre == "Rock" + + def test_migrate_slash_separated_genres(self): + """Migrate legacy slash-separated genre strings.""" + item = Item(genre="Rock / Alternative / Indie", genres=[]) + correct_list_fields(item) + + assert item.genres == ["Rock", "Alternative", "Indie"] + assert item.genre == "Rock" + + def test_no_migration_when_genres_exists(self): + """Don't migrate if genres list already populated.""" + item = Item(genre="Jazz, Blues", genres=["Rock", "Pop"]) + correct_list_fields(item) + + # Existing genres list should be preserved + # The full genre string is prepended (migration doesn't run when genres exists) + assert item.genres == ["Jazz, Blues", "Rock", "Pop"] + assert item.genre == "Jazz, Blues" + + def test_no_migration_single_genre(self): + """Don't split single genres without separators.""" + item = Item(genre="Rock", genres=[]) + correct_list_fields(item) + + # Single genre (no separator) should not trigger migration + assert item.genres == ["Rock"] + assert item.genre == "Rock" diff --git a/test/test_library.py b/test/test_library.py index 4df4e4b58..bf3508e88 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -701,6 +701,34 @@ class DestinationFunctionTest(BeetsTestCase, PathFormattingMixin): self._setf("%first{Alice / Bob / Eve,2,0, / , & }") self._assert_dest(b"/base/Alice & Bob") + def test_first_genres_list(self): + # Test that setting genres as a list syncs to genre field properly + # and works with %first template function + from beets.autotag import correct_list_fields + + # Clear the default genre first + self.i.genre = "" + self.i.genres = ["Pop", "Rock", "Classical Crossover"] + correct_list_fields(self.i) + # genre field should now be synced to first item + assert self.i.genre == "Pop" + # %first should work on the genre field + self._setf("%first{$genre}") + self._assert_dest(b"/base/Pop") + + def test_first_genres_list_skip(self): + # Test that the genres list is accessible as a multi-value field + from beets.autotag import correct_list_fields + + # Clear the default genre first + self.i.genre = "" + self.i.genres = ["Pop", "Rock", "Classical Crossover"] + correct_list_fields(self.i) + # Access the second genre directly using index (genres is a list) + # The genres field should be available as a multi-value field + assert self.i.genres[1] == "Rock" + assert len(self.i.genres) == 3 + class DisambiguationTest(BeetsTestCase, PathFormattingMixin): def setUp(self): From 1c0ebcf348aa8356806ce2e7024d1eaa509bbb84 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 14:59:23 +0100 Subject: [PATCH 23/60] remove noisy comment from test/plugins/test_beatport.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šarūnas Nejus --- test/plugins/test_beatport.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index dbfb89c9c..b79e4dcc7 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -583,7 +583,6 @@ class BeatportTest(BeetsTestCase): def test_genre_applied(self): for track, test_track in zip(self.tracks, self.test_tracks): - # BeatportTrack now has genres as a list assert track.genres == [test_track.genre] From c55b6d103c05d249b22af7cd1c41dc933ea2870b Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 15:00:33 +0100 Subject: [PATCH 24/60] shorte test description in test/plugins/test_lastgenre.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šarūnas Nejus --- test/plugins/test_lastgenre.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 9b510958e..e218ae307 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -81,7 +81,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): assert self.plugin._resolve_genres(["delta blues"]) == [] def test_format_genres(self): - """Format genres list with title case if configured.""" + """Format genres list.""" self._setup_config(count=2) assert self.plugin._format_genres(["jazz", "pop", "rock", "blues"]) == [ "Jazz", From 4e30c181c604a959b5d38e66b0afe89c0234204e Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 15:01:45 +0100 Subject: [PATCH 25/60] better function description in 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 a75af0aec..1011eabd6 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -330,7 +330,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Main processing: _get_genre() and helpers. def _format_genres(self, tags: list[str]) -> list[str]: - """Format to title_case if configured and return as list.""" + """Format to title case if configured.""" if self.config["title_case"]: return [tag.title() for tag in tags] else: From 9922f8fb995774632593785b7d28132a687fd93c Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 15:02:40 +0100 Subject: [PATCH 26/60] simplify return logic in 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 1011eabd6..5333871cc 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -345,7 +345,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): genres_list = obj.get("genres") # Filter out empty strings - return [g for g in genres_list if g] if genres_list else [] + return genres_list def _combine_resolve_and_log( self, old: list[str], new: list[str] From 07a3cba262b60a86f0f84ca44f5f02f9793e4e04 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 15:05:47 +0100 Subject: [PATCH 27/60] simplify genre unpacking in 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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 5333871cc..67145b18b 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -502,9 +502,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): def _fetch_and_log_genre(self, obj: LibModel) -> None: """Fetch genre and log it.""" self._log.info(str(obj)) - genres_list, label = self._get_genre(obj) - obj.genres = genres_list - self._log.debug("Resolved ({}): {}", label, genres_list) + obj.genres, label = self._get_genre(obj) + self._log.debug("Resolved ({}): {}", label, obj.genres) + ui.show_model_changes(obj, fields=["genres"], print_obj=False) From 99831906c2eb0a16824309136dfa96e39acf074d Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 15:12:32 +0100 Subject: [PATCH 28/60] simplify check for fallback in 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 | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 67145b18b..1fc191a81 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -473,22 +473,15 @@ class LastGenrePlugin(plugins.BeetsPlugin): return result # Nothing found, leave original if configured and valid. - if obj.genre and self.config["keep_existing"]: - # Check if at least one genre is valid - valid_genres = [ - g - for g in genres - if not self.whitelist or self._is_valid(g.lower()) - ] - if valid_genres: + if genres and self.config["keep_existing"]: + if valid_genres := self._filter_valid(genres): return valid_genres, "original fallback" - else: - # If the original genre doesn't match a whitelisted genre, check - # if we can canonicalize it to find a matching, whitelisted genre! - if result := _try_resolve_stage( - "original fallback", keep_genres, [] - ): - return result + # If the original genre doesn't match a whitelisted genre, check + # if we can canonicalize it to find a matching, whitelisted genre! + if result := _try_resolve_stage( + "original fallback", keep_genres, [] + ): + return result # Return fallback as a list. if fallback := self.config["fallback"].get(): @@ -505,7 +498,6 @@ class LastGenrePlugin(plugins.BeetsPlugin): obj.genres, label = self._get_genre(obj) self._log.debug("Resolved ({}): {}", label, obj.genres) - ui.show_model_changes(obj, fields=["genres"], print_obj=False) @singledispatchmethod From 36a30b3c650939fbd07ed80d5f2e334873431d82 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 20:12:15 +0100 Subject: [PATCH 29/60] Implement automatic database-level genre migration - Add Library._make_table() override to automatically migrate genres when database schema is updated - Migration splits comma/semicolon/slash-separated genre strings into genres list - Writes changes to both database and media files with progress reporting - Remove lazy migration from correct_list_fields() - now handled at database level - Remove migration-specific tests (migration is now automatic, not lazy) - Update changelog to reflect automatic migration behavior Related PR review comment changes: - Replace _is_valid with _filter_valid method in lastgenre plugin - Use unique_list and remove genre field from Beatport plugin - Simplify LastGenre tests - remove separator logic - Document separator deprecation in lastgenre plugin - Add deprecation warning for genre parameter in Info.__init__() --- beets/autotag/__init__.py | 30 ----------- beets/autotag/hooks.py | 23 +++++++- beets/library/library.py | 94 ++++++++++++++++++++++++++++++++- beetsplug/beatport.py | 12 ++--- beetsplug/lastgenre/__init__.py | 32 +++++------ docs/changelog.rst | 15 +++--- docs/plugins/lastgenre.rst | 16 ++++-- test/plugins/test_discogs.py | 6 +-- test/plugins/test_lastgenre.py | 65 +++-------------------- test/test_autotag.py | 45 ---------------- 10 files changed, 166 insertions(+), 172 deletions(-) diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index eabc41833..4cc4ff30a 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -166,37 +166,7 @@ def correct_list_fields(m: LibModel) -> None: elif list_val: setattr(m, single_field, list_val[0]) - def migrate_legacy_genres() -> None: - """Migrate comma-separated genre strings to genres list. - - For users upgrading from previous versions, their genre field may - contain comma-separated values (e.g., "Rock, Alternative, Indie"). - This migration splits those values into the genres list on first access, - avoiding the need to reimport the entire library. - """ - genre_val = getattr(m, "genre", "") - genres_val = getattr(m, "genres", []) - - # Only migrate if genres list is empty and genre contains separators - if not genres_val and genre_val: - # Try common separators used by lastgenre and other tools - for separator in [", ", "; ", " / "]: - if separator in genre_val: - # Split and clean the genre string - split_genres = [ - g.strip() - for g in genre_val.split(separator) - if g.strip() - ] - if len(split_genres) > 1: - # Found a valid split - populate genres list - setattr(m, "genres", split_genres) - # Clear genre so ensure_first_value sets it correctly - setattr(m, "genre", "") - break - ensure_first_value("albumtype", "albumtypes") - migrate_legacy_genres() ensure_first_value("genre", "genres") if hasattr(m, "mb_artistids"): diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 7818d5f21..ef9e8bb30 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -16,6 +16,7 @@ from __future__ import annotations +import warnings from copy import deepcopy from dataclasses import dataclass from functools import cached_property @@ -80,6 +81,26 @@ class Info(AttrDict[Any]): media: str | None = None, **kwargs, ) -> None: + if genre: + warnings.warn( + "The 'genre' parameter is deprecated. Use 'genres' (list) instead.", + DeprecationWarning, + stacklevel=2, + ) + if not genres: + for separator in [", ", "; ", " / "]: + if separator in genre: + split_genres = [ + g.strip() + for g in genre.split(separator) + if g.strip() + ] + if len(split_genres) > 1: + genres = split_genres + break + if not genres: + genres = [genre] + self.album = album self.artist = artist self.artist_credit = artist_credit @@ -91,7 +112,7 @@ class Info(AttrDict[Any]): self.artists_sort = artists_sort or [] self.data_source = data_source self.data_url = data_url - self.genre = genre + self.genre = None self.genres = genres or [] self.media = media self.update(kwargs) diff --git a/beets/library/library.py b/beets/library/library.py index 39d559901..a534d26b3 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -5,14 +5,19 @@ from typing import TYPE_CHECKING import platformdirs import beets -from beets import dbcore +from beets import dbcore, logging, ui +from beets.autotag import correct_list_fields from beets.util import normpath from .models import Album, Item from .queries import PF_KEY_DEFAULT, parse_query_parts, parse_query_string if TYPE_CHECKING: - from beets.dbcore import Results + from collections.abc import Mapping + + from beets.dbcore import Results, types + +log = logging.getLogger("beets") class Library(dbcore.Database): @@ -142,3 +147,88 @@ class Library(dbcore.Database): item_or_id if isinstance(item_or_id, int) else item_or_id.album_id ) return self._get(Album, album_id) if album_id else None + + # Database schema migration. + + def _make_table(self, table: str, fields: Mapping[str, types.Type]): + """Set up the schema of the database, and migrate genres if needed.""" + with self.transaction() as tx: + rows = tx.query(f"PRAGMA table_info({table})") + current_fields = {row[1] for row in rows} + field_names = set(fields.keys()) + + # Check if genres column is being added to items table + genres_being_added = ( + table == "items" + and "genres" in field_names + and "genres" not in current_fields + and "genre" in current_fields + ) + + # Call parent to create/update table + super()._make_table(table, fields) + + # Migrate genre to genres if genres column was just added + if genres_being_added: + self._migrate_genre_to_genres() + + def _migrate_genre_to_genres(self): + """Migrate comma-separated genre strings to genres list. + + This migration runs automatically when the genres column is first + created in the database. It splits comma-separated genre values + and writes the changes to both the database and media files. + """ + items = list(self.items()) + migrated_count = 0 + total_items = len(items) + + if total_items == 0: + return + + ui.print_(f"Migrating genres for {total_items} items...") + + for index, item in enumerate(items, 1): + genre_val = item.genre or "" + genres_val = item.genres or [] + + # Check if migration is needed + needs_migration = False + split_genres = [] + if not genres_val and genre_val: + for separator in [", ", "; ", " / "]: + if separator in genre_val: + split_genres = [ + g.strip() + for g in genre_val.split(separator) + if g.strip() + ] + if len(split_genres) > 1: + needs_migration = True + break + + if needs_migration: + migrated_count += 1 + # Show progress every 100 items + if migrated_count % 100 == 0: + ui.print_( + f" Migrated {migrated_count} items " + f"({index}/{total_items} processed)..." + ) + # Migrate using the same logic as correct_list_fields + correct_list_fields(item) + item.store() + # Write to media file + try: + item.try_write() + except Exception as e: + log.warning( + "Could not write genres to {}: {}", + item.path, + e, + ) + + ui.print_( + f"Migration complete: {migrated_count} of {total_items} items " + f"updated with comma-separated genres" + ) diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 8918a10cb..8e93efc3a 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -33,6 +33,7 @@ import beets import beets.ui from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin +from beets.util import unique_list if TYPE_CHECKING: from collections.abc import Iterable, Iterator, Sequence @@ -235,8 +236,7 @@ class BeatportObject: self.artists = [(x["id"], str(x["name"])) for x in data["artists"]] if "genres" in data: genre_list = [str(x["name"]) for x in data["genres"]] - # Remove duplicates while preserving order - self.genres = list(dict.fromkeys(genre_list)) + self.genres = unique_list(genre_list) def artists_str(self) -> str | None: if self.artists is not None: @@ -255,7 +255,6 @@ class BeatportRelease(BeatportObject): label_name: str | None category: str | None url: str | None - genre: str | None tracks: list[BeatportTrack] | None = None @@ -265,7 +264,6 @@ class BeatportRelease(BeatportObject): self.catalog_number = data.get("catalogNumber") self.label_name = data.get("label", {}).get("name") self.category = data.get("category") - self.genre = data.get("genre") if "slug" in data: self.url = ( @@ -287,7 +285,6 @@ class BeatportTrack(BeatportObject): track_number: int | None bpm: str | None initial_key: str | None - genre: str | None def __init__(self, data: JSONDict): super().__init__(data) @@ -316,8 +313,7 @@ class BeatportTrack(BeatportObject): else: genre_list = [] - # Remove duplicates while preserving order - self.genres = list(dict.fromkeys(genre_list)) + self.genres = unique_list(genre_list) class BeatportPlugin(MetadataSourcePlugin): @@ -490,7 +486,6 @@ class BeatportPlugin(MetadataSourcePlugin): media="Digital", data_source=self.data_source, data_url=release.url, - genre=release.genre, genres=release.genres, year=release_date.year if release_date else None, month=release_date.month if release_date else None, @@ -516,7 +511,6 @@ class BeatportPlugin(MetadataSourcePlugin): data_url=track.url, bpm=track.bpm, initial_key=track.initial_key, - genre=track.genre, genres=track.genres, ) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 1fc191a81..2c9b2ed06 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -39,7 +39,7 @@ from beets.util import plurality, unique_list if TYPE_CHECKING: import optparse - from collections.abc import Callable + from collections.abc import Callable, Iterable from beets.importer import ImportSession, ImportTask from beets.library import LibModel @@ -213,7 +213,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): - Returns an empty list if the input tags list is empty. - If canonicalization is enabled, it extends the list by incorporating parent genres from the canonicalization tree. When a whitelist is set, - only parent tags that pass a validity check (_is_valid) are included; + only parent tags that pass the whitelist filter are included; otherwise, it adds the oldest ancestor. Adding parent tags is stopped when the count of tags reaches the configured limit (count). - The tags list is then deduplicated to ensure only unique genres are @@ -237,11 +237,9 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Add parents that are in the whitelist, or add the oldest # ancestor if no whitelist if self.whitelist: - parents = [ - x - for x in find_parents(tag, self.c14n_branches) - if self._is_valid(x) - ] + parents = self._filter_valid( + find_parents(tag, self.c14n_branches) + ) else: parents = [find_parents(tag, self.c14n_branches)[-1]] @@ -263,7 +261,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): # c14n only adds allowed genres but we may have had forbidden genres in # the original tags list - valid_tags = [t for t in tags if self._is_valid(t)] + valid_tags = self._filter_valid(tags) return valid_tags[:count] def fetch_genre( @@ -275,15 +273,19 @@ class LastGenrePlugin(plugins.BeetsPlugin): min_weight = self.config["min_weight"].get(int) return self._tags_for(lastfm_obj, min_weight) - def _is_valid(self, genre: str) -> bool: - """Check if the genre is valid. + def _filter_valid(self, genres: Iterable[str]) -> list[str]: + """Filter genres based on whitelist. - Depending on the whitelist property, valid means a genre is in the - whitelist or any genre is allowed. + Returns all genres if no whitelist is configured, otherwise returns + only genres that are in the whitelist. """ - if genre and (not self.whitelist or genre.lower() in self.whitelist): - return True - return False + # First, drop any falsy or whitespace-only genre strings to avoid + # retaining empty tags from multi-valued fields. + cleaned = [g for g in genres if g and g.strip()] + if not self.whitelist: + return cleaned + + return [g for g in cleaned if g.lower() in self.whitelist] # Cached last.fm entity lookups. diff --git a/docs/changelog.rst b/docs/changelog.rst index 2a4caf573..290f63168 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,12 +21,11 @@ New features updated to populate the ``genres`` field as a list. **Migration**: Existing libraries with comma-separated, semicolon-separated, - or slash-separated genre strings (e.g., ``"Rock, Alternative, Indie"``) will - be automatically migrated to the ``genres`` list when items are accessed. No - manual reimport or ``mbsync`` is required. For users who prefer explicit - control, a new ``beet migrate genres`` command is available to migrate the - entire library at once. Use ``beet migrate genres --pretend`` to preview - changes before applying them. + or slash-separated genre strings (e.g., ``"Rock, Alternative, Indie"``) are + automatically migrated to the ``genres`` list when you first run beets after + upgrading. The migration runs once when the database schema is updated, + splitting genre strings and writing the changes to both the database and media + files. No manual action or ``mbsync`` is required. .. Bug fixes @@ -47,6 +46,10 @@ Other changes - :doc:`plugins/edit`: Editing multi-valued fields now behaves more naturally, with list values handled directly to make metadata edits smoother and more predictable. +- :doc:`plugins/lastgenre`: The ``separator`` configuration option is + deprecated. Genres are now stored as a list in the ``genres`` field and + written to files as individual genre tags. The separator option has no effect + and will be removed in a future version. 2.6.2 (February 22, 2026) ------------------------- diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index ace7caaf0..b677b001e 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -90,9 +90,8 @@ By default, the plugin chooses the most popular tag on Last.fm as a genre. If you prefer to use a *list* of popular genre tags, you can increase the number of the ``count`` config option. -Lists of up to *count* genres will then be used instead of single genres. The -genres are separated by commas by default, but you can change this with the -``separator`` config option. +Lists of up to *count* genres will be stored in the ``genres`` field as a list +and written to your media files as separate genre tags. Last.fm_ provides a popularity factor, a.k.a. *weight*, for each tag ranging from 100 for the most popular tag down to 0 for the least popular. The plugin @@ -192,7 +191,16 @@ file. The available options are: Default: ``no``. - **source**: Which entity to look up in Last.fm. Can be either ``artist``, ``album`` or ``track``. Default: ``album``. -- **separator**: A separator for multiple genres. Default: ``', '``. +- **separator**: + + .. deprecated:: 2.6 + + The ``separator`` option is deprecated. Genres are now stored as a list in + the ``genres`` field and written to files as individual genre tags. This + option has no effect and will be removed in a future version. + + Default: ``', '``. + - **whitelist**: The filename of a custom genre list, ``yes`` to use the internal whitelist, or ``no`` to consider all genres valid. Default: ``yes``. - **title_case**: Convert the new tags to TitleCase before saving. Default: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 15d47db6c..cef84e3a9 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -362,7 +362,7 @@ class DGAlbumInfoTest(BeetsTestCase): release = self._make_release_from_positions(["1", "2"]) d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2" + assert d.genres == ["GENRE1", "GENRE2"] assert d.style == "STYLE1, STYLE2" def test_append_style_to_genre(self): @@ -371,7 +371,7 @@ class DGAlbumInfoTest(BeetsTestCase): release = self._make_release_from_positions(["1", "2"]) d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2, STYLE1, STYLE2" + assert d.genres == ["GENRE1", "GENRE2", "STYLE1", "STYLE2"] assert d.style == "STYLE1, STYLE2" def test_append_style_to_genre_no_style(self): @@ -381,7 +381,7 @@ class DGAlbumInfoTest(BeetsTestCase): release.data["styles"] = [] d = DiscogsPlugin().get_album_info(release) - assert d.genre == "GENRE1, GENRE2" + assert d.genres == ["GENRE1", "GENRE2"] assert d.style is None def test_strip_disambiguation(self): diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index e218ae307..1a53a5a72 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -401,25 +401,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["fallback genre"], "fallback"), ), - # 9 - null charachter as separator - ( - { - "force": True, - "keep_existing": True, - "source": "album", - "whitelist": True, - "separator": "\u0000", - "canonical": False, - "prefer_specific": False, - "count": 10, - }, - "Blues", - { - "album": ["Jazz"], - }, - (["Blues", "Jazz"], "keep + album, whitelist"), - ), - # 10 - limit a lot of results + # 9 - limit a lot of results ( { "force": True, @@ -429,7 +411,6 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "count": 5, "canonical": False, "prefer_specific": False, - "separator": ", ", }, "original unknown, Blues, Rock, Folk, Metal", { @@ -440,23 +421,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "keep + album, whitelist", ), ), - # 11 - force off does not rely on configured separator - ( - { - "force": False, - "keep_existing": False, - "source": "album", - "whitelist": True, - "count": 2, - "separator": ", ", - }, - "not ; configured | separator", - { - "album": ["Jazz", "Bebop"], - }, - (["not ; configured | separator"], "keep any, no-force"), - ), - # 12 - fallback to next stage (artist) if no allowed original present + # 10 - fallback to next stage (artist) if no allowed original present # and no album genre were fetched. ( { @@ -476,7 +441,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Jazz"], "keep + artist, whitelist"), ), - # 13 - canonicalization transforms non-whitelisted genres to canonical forms + # 11 - canonicalization transforms non-whitelisted genres to canonical forms # # "Acid Techno" is not in the default whitelist, thus gets resolved "up" in the # tree to "Techno" and "Electronic". @@ -496,7 +461,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Techno", "Electronic"], "album, whitelist"), ), - # 14 - canonicalization transforms whitelisted genres to canonical forms and + # 12 - canonicalization transforms whitelisted genres to canonical forms and # includes originals # # "Detroit Techno" is in the default whitelist, thus it stays and and also gets @@ -528,7 +493,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "keep + album, whitelist", ), ), - # 15 - canonicalization transforms non-whitelisted original genres to canonical + # 13 - canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the @@ -606,25 +571,11 @@ def test_get_genre(config_values, item_genre, mock_genres, expected_result): plugin.setup() # Loads default whitelist and canonicalization tree item = _common.item() - # Set genres as a list - if item_genre is a string, convert it to list if item_genre: - # For compatibility with old separator-based tests, split if needed - if ( - "separator" in config_values - and config_values["separator"] in item_genre - ): - sep = config_values["separator"] - item.genres = [ - g.strip() for g in item_genre.split(sep) if g.strip() - ] + if ", " in item_genre: + item.genres = [g.strip() for g in item_genre.split(", ")] else: - # Assume comma-separated if no specific separator - if ", " in item_genre: - item.genres = [ - g.strip() for g in item_genre.split(", ") if g.strip() - ] - else: - item.genres = [item_genre] + item.genres = [item_genre] else: item.genres = [] diff --git a/test/test_autotag.py b/test/test_autotag.py index e094d0ddc..e6a122ae2 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -543,48 +543,3 @@ class TestGenreSync: assert item.genres == [] assert item.genre == "" - - def test_migrate_comma_separated_genres(self): - """Migrate legacy comma-separated genre strings.""" - item = Item(genre="Rock, Alternative, Indie", genres=[]) - correct_list_fields(item) - - # Should split into genres list - assert item.genres == ["Rock", "Alternative", "Indie"] - # Genre becomes first item after migration - assert item.genre == "Rock" - - def test_migrate_semicolon_separated_genres(self): - """Migrate legacy semicolon-separated genre strings.""" - item = Item(genre="Rock; Alternative; Indie", genres=[]) - correct_list_fields(item) - - assert item.genres == ["Rock", "Alternative", "Indie"] - assert item.genre == "Rock" - - def test_migrate_slash_separated_genres(self): - """Migrate legacy slash-separated genre strings.""" - item = Item(genre="Rock / Alternative / Indie", genres=[]) - correct_list_fields(item) - - assert item.genres == ["Rock", "Alternative", "Indie"] - assert item.genre == "Rock" - - def test_no_migration_when_genres_exists(self): - """Don't migrate if genres list already populated.""" - item = Item(genre="Jazz, Blues", genres=["Rock", "Pop"]) - correct_list_fields(item) - - # Existing genres list should be preserved - # The full genre string is prepended (migration doesn't run when genres exists) - assert item.genres == ["Jazz, Blues", "Rock", "Pop"] - assert item.genre == "Jazz, Blues" - - def test_no_migration_single_genre(self): - """Don't split single genres without separators.""" - item = Item(genre="Rock", genres=[]) - correct_list_fields(item) - - # Single genre (no separator) should not trigger migration - assert item.genres == ["Rock"] - assert item.genre == "Rock" From e99d3ca06151d11919ee992980a37794d593df85 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 20:48:15 +0100 Subject: [PATCH 30/60] Simplify MusicBrainz genres assignment Remove intermediate variable and assign directly to info.genres. Addresses PR review comment. --- beetsplug/musicbrainz.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 695ce8790..75933e6f9 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -644,11 +644,10 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): for source in sources: for genreitem in source: genres[genreitem["name"]] += int(genreitem["count"]) - genre_list = [ + info.genres = [ genre for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) ] - info.genres = genre_list # We might find links to external sources (Discogs, Bandcamp, ...) external_ids = self.config["external_ids"].get() From 76c4eeedbb9903528cf32e8894175142a1cd2367 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 28 Dec 2025 20:54:43 +0100 Subject: [PATCH 31/60] Remove manual migrate command Migration now happens automatically when the database schema is updated (in Library._make_table()), so the manual 'beet migrate' command is no longer needed. Addresses PR review comment. --- beets/ui/commands/__init__.py | 8 ++- beets/ui/commands/migrate.py | 98 ----------------------------------- 2 files changed, 3 insertions(+), 103 deletions(-) delete mode 100644 beets/ui/commands/migrate.py diff --git a/beets/ui/commands/__init__.py b/beets/ui/commands/__init__.py index b4eebb53f..e1d0389a3 100644 --- a/beets/ui/commands/__init__.py +++ b/beets/ui/commands/__init__.py @@ -24,7 +24,6 @@ from .fields import fields_cmd from .help import HelpCommand from .import_ import import_cmd from .list import list_cmd -from .migrate import migrate_cmd from .modify import modify_cmd from .move import move_cmd from .remove import remove_cmd @@ -53,13 +52,12 @@ default_commands = [ HelpCommand(), import_cmd, list_cmd, - migrate_cmd, - modify_cmd, - move_cmd, + update_cmd, remove_cmd, stats_cmd, - update_cmd, version_cmd, + modify_cmd, + move_cmd, write_cmd, config_cmd, completion_cmd, diff --git a/beets/ui/commands/migrate.py b/beets/ui/commands/migrate.py deleted file mode 100644 index 2cb7e0d59..000000000 --- a/beets/ui/commands/migrate.py +++ /dev/null @@ -1,98 +0,0 @@ -"""The 'migrate' command: migrate library data for format changes.""" - -from beets import logging, ui -from beets.autotag import correct_list_fields - -# Global logger. -log = logging.getLogger("beets") - - -def migrate_genres(lib, pretend=False): - """Migrate comma-separated genre strings to genres list. - - For users upgrading from previous versions, their genre field may - contain comma-separated values (e.g., "Rock, Alternative, Indie"). - This command splits those values into the genres list, avoiding - the need to reimport the entire library. - """ - items = lib.items() - migrated_count = 0 - total_items = 0 - - ui.print_("Scanning library for items with comma-separated genres...") - - for item in items: - total_items += 1 - genre_val = item.genre or "" - genres_val = item.genres or [] - - # Check if migration is needed - needs_migration = False - if not genres_val and genre_val: - for separator in [", ", "; ", " / "]: - if separator in genre_val: - split_genres = [ - g.strip() - for g in genre_val.split(separator) - if g.strip() - ] - if len(split_genres) > 1: - needs_migration = True - break - - if needs_migration: - migrated_count += 1 - old_genre = item.genre - old_genres = item.genres or [] - - if pretend: - # Just show what would change - ui.print_( - f" Would migrate: {item.artist} - {item.title}\n" - f" genre: {old_genre!r} -> {split_genres[0]!r}\n" - f" genres: {old_genres!r} -> {split_genres!r}" - ) - else: - # Actually migrate - correct_list_fields(item) - item.store() - log.debug( - "migrated: {} - {} ({} -> {})", - item.artist, - item.title, - old_genre, - item.genres, - ) - - # Show summary - if pretend: - ui.print_( - f"\nWould migrate {migrated_count} of {total_items} items " - f"(run without --pretend to apply changes)" - ) - else: - ui.print_( - f"\nMigrated {migrated_count} of {total_items} items with " - f"comma-separated genres" - ) - - -def migrate_func(lib, opts, args): - """Handle the migrate command.""" - if not args or args[0] == "genres": - migrate_genres(lib, pretend=opts.pretend) - else: - raise ui.UserError(f"unknown migration target: {args[0]}") - - -migrate_cmd = ui.Subcommand( - "migrate", help="migrate library data for format changes" -) -migrate_cmd.parser.add_option( - "-p", - "--pretend", - action="store_true", - help="show what would be changed without applying", -) -migrate_cmd.parser.usage = "%prog migrate genres [options]" -migrate_cmd.func = migrate_func From 21fb5a561d3cf444d3851b75735692647a9a5bae Mon Sep 17 00:00:00 2001 From: dunkla Date: Sat, 10 Jan 2026 15:37:33 +0100 Subject: [PATCH 32/60] Fix lastgenre migration separator logic (ref https://github.com/beetbox/beets/pull/6169#issuecomment-3716893013) --- beets/library/library.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/beets/library/library.py b/beets/library/library.py index a534d26b3..16a8fcf93 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -196,7 +196,26 @@ class Library(dbcore.Database): needs_migration = False split_genres = [] if not genres_val and genre_val: - for separator in [", ", "; ", " / "]: + separators = [] + if ( + "lastgenre" in beets.config + and "separator" in beets.config["lastgenre"] + ): + try: + user_sep = beets.config["lastgenre"][ + "separator" + ].as_str() + if user_sep: + separators.append(user_sep) + except ( + beets.config.ConfigNotFoundError, + beets.config.ConfigTypeError, + ): + pass + + separators.extend([", ", "; ", " / "]) + + for separator in separators: if separator in genre_val: split_genres = [ g.strip() From 9224c9c960f73d044b1d9461f0455aa218c7f7fb Mon Sep 17 00:00:00 2001 From: dunkla Date: Sat, 10 Jan 2026 15:37:45 +0100 Subject: [PATCH 33/60] Use compact generator expression in Beatport (ref https://github.com/beetbox/beets/pull/6169#issuecomment-3716893013) --- beetsplug/beatport.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 8e93efc3a..3368825b8 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -306,14 +306,10 @@ class BeatportTrack(BeatportObject): self.initial_key = str((data.get("key") or {}).get("shortName")) # Extract genres list from subGenres or genres - if data.get("subGenres"): - genre_list = [str(x.get("name")) for x in data["subGenres"]] - elif data.get("genres"): - genre_list = [str(x.get("name")) for x in data["genres"]] - else: - genre_list = [] - - self.genres = unique_list(genre_list) + self.genres = unique_list( + str(x.get("name")) + for x in data.get("subGenres") or data.get("genres") or [] + ) class BeatportPlugin(MetadataSourcePlugin): From 67ce53d2c6e7dc2d187337aab904d4d51412e3f0 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sat, 10 Jan 2026 15:37:55 +0100 Subject: [PATCH 34/60] Remove conditional logic from lastgenre tests (ref https://github.com/beetbox/beets/pull/6169#issuecomment-3716893013) --- test/plugins/test_lastgenre.py | 40 +++++++++++++++------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 1a53a5a72..5dcf2c165 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -217,7 +217,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "prefer_specific": False, "count": 10, }, - "Blues", + ["Blues"], { "album": ["Jazz"], }, @@ -233,7 +233,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + ["original unknown", "Blues"], { "album": ["Jazz"], }, @@ -249,7 +249,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazz"], }, @@ -265,7 +265,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues", + ["original unknown", "Blues"], { "album": ["Jazz"], "artist": ["Pop"], @@ -282,7 +282,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "any genre", + ["any genre"], { "album": ["Jazz"], }, @@ -298,7 +298,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "album": ["Jazzin"], }, @@ -314,7 +314,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "unknown genre", + ["unknown genre"], { "track": None, "album": None, @@ -334,7 +334,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "any existing", + ["any existing"], { "track": None, "album": None, @@ -354,7 +354,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "Jazz", + ["Jazz"], { "track": None, "album": None, @@ -374,7 +374,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "Jazz", + ["Jazz"], { "track": None, "album": None, @@ -393,7 +393,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "", + [], { "track": None, "album": None, @@ -412,7 +412,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "original unknown, Blues, Rock, Folk, Metal", + ["original unknown", "Blues", "Rock", "Folk", "Metal"], { "album": ["Jazz", "Bebop", "Hardbop"], }, @@ -433,7 +433,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "canonical": False, "prefer_specific": False, }, - "not whitelisted original", + ["not whitelisted original"], { "track": None, "album": None, @@ -455,7 +455,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "prefer_specific": False, "count": 10, }, - "", + [], { "album": ["acid techno"], }, @@ -478,7 +478,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "count": 10, "extended_debug": True, }, - "detroit techno", + ["detroit techno"], { "album": ["acid house"], }, @@ -509,7 +509,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "prefer_specific": False, "count": 10, }, - "Cosmic Disco", + ["Cosmic Disco"], { "album": ["Detroit Techno"], }, @@ -571,13 +571,7 @@ def test_get_genre(config_values, item_genre, mock_genres, expected_result): plugin.setup() # Loads default whitelist and canonicalization tree item = _common.item() - if item_genre: - if ", " in item_genre: - item.genres = [g.strip() for g in item_genre.split(", ")] - else: - item.genres = [item_genre] - else: - item.genres = [] + item.genres = item_genre # Run assert plugin._get_genre(item) == expected_result From 9003107ee7c8bfae4985b0792b848634737fc13a Mon Sep 17 00:00:00 2001 From: dunkla Date: Sat, 10 Jan 2026 15:38:04 +0100 Subject: [PATCH 35/60] Remove noisy comments from beatport tests (ref https://github.com/beetbox/beets/pull/6169#issuecomment-3716893013) --- test/plugins/test_beatport.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index b79e4dcc7..916227f40 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -634,7 +634,6 @@ class BeatportResponseEmptyTest(unittest.TestCase): self.test_tracks[0]["subGenres"] = [] - # BeatportTrack now has genres as a list assert tracks[0].genres == [self.test_tracks[0]["genres"][0]["name"]] def test_genre_empty(self): @@ -644,5 +643,4 @@ class BeatportResponseEmptyTest(unittest.TestCase): self.test_tracks[0]["genres"] = [] - # BeatportTrack now has genres as a list assert tracks[0].genres == [self.test_tracks[0]["subGenres"][0]["name"]] From 10d197e24259aa7dc9046c42772352a31d62747c Mon Sep 17 00:00:00 2001 From: dunkla Date: Sat, 10 Jan 2026 15:39:40 +0100 Subject: [PATCH 36/60] Update lastgenre docstring and remove misleading comment (ref https://github.com/beetbox/beets/pull/6169#issuecomment-3716893013) --- beetsplug/lastgenre/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 2c9b2ed06..7f3d3ea86 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -339,14 +339,12 @@ class LastGenrePlugin(plugins.BeetsPlugin): return tags def _get_existing_genres(self, obj: LibModel) -> list[str]: - """Return a list of genres for this Item or Album. Empty string genres - are removed.""" + """Return a list of genres for this Item or Album.""" if isinstance(obj, library.Item): genres_list = obj.get("genres", with_album=False) else: genres_list = obj.get("genres") - # Filter out empty strings return genres_list def _combine_resolve_and_log( From 0191ecf5764db26c4a86b5b83377f6576f19f9b8 Mon Sep 17 00:00:00 2001 From: dunkla Date: Sun, 11 Jan 2026 14:09:17 +0100 Subject: [PATCH 37/60] Fix mypy incompatible return type in lastgenre --- 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 7f3d3ea86..bfa55ba90 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -378,7 +378,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): def _try_resolve_stage( stage_label: str, keep_genres: list[str], new_genres: list[str] - ) -> tuple[str, str] | None: + ) -> tuple[list[str], str] | None: """Try to resolve genres for a given stage and log the result.""" resolved_genres = self._combine_resolve_and_log( keep_genres, new_genres From eac9e1fd973e1eb8cc184413fc44a9d3e23cd06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 8 Feb 2026 07:09:01 +0000 Subject: [PATCH 38/60] Add support for migrations --- beets/dbcore/db.py | 47 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 8640a5678..deb31ba71 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -26,14 +26,7 @@ import threading import time from abc import ABC from collections import defaultdict -from collections.abc import ( - Callable, - Generator, - Iterable, - Iterator, - Mapping, - Sequence, -) +from collections.abc import Mapping from functools import cached_property from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, ClassVar, Generic, NamedTuple @@ -1088,11 +1081,14 @@ class Database: self._db_lock = threading.Lock() # Set up database schema. + self._ensure_migration_state_table() for model_cls in self._models: self._make_table(model_cls._table, model_cls._fields) self._make_attribute_table(model_cls._flex_table) self._create_indices(model_cls._table, model_cls._indices) + self._migrate() + # Primitive access control: connections and transactions. def _connection(self) -> Connection: @@ -1292,6 +1288,41 @@ class Database: f"ON {table} ({', '.join(index.columns)});" ) + # Generic migration state handling. + + def _ensure_migration_state_table(self) -> None: + with self.transaction() as tx: + tx.script(""" + CREATE TABLE IF NOT EXISTS migrations ( + name TEXT NOT NULL, + table_name TEXT NOT NULL, + PRIMARY KEY(name, table_name) + ); + """) + + def _migrate(self) -> None: + """Perform any necessary migration for the database.""" + + def migration_exists(self, name: str, table: str) -> bool: + """Return whether a named migration has been marked complete.""" + with self.transaction() as tx: + return tx.execute( + """ + SELECT EXISTS( + SELECT 1 FROM migrations WHERE name = ? AND table_name = ? + ) + """, + (name, table), + ).fetchone()[0] + + def record_migration(self, name: str, table: str) -> None: + """Set completion state for a named migration.""" + with self.transaction() as tx: + tx.mutate( + "INSERT INTO migrations(name, table_name) VALUES (?, ?)", + (name, table), + ) + # Querying. def _fetch( From 8edd0fc966aef0a44c82bcbe73635ab73aab6f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 8 Feb 2026 21:24:00 +0000 Subject: [PATCH 39/60] Add generic Migration implementation --- beets/dbcore/db.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index deb31ba71..4b0ba4f15 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -24,9 +24,10 @@ import sqlite3 import sys import threading import time -from abc import ABC +from abc import ABC, abstractmethod from collections import defaultdict from collections.abc import Mapping +from dataclasses import dataclass from functools import cached_property from sqlite3 import Connection, sqlite_version_info from typing import TYPE_CHECKING, Any, AnyStr, ClassVar, Generic, NamedTuple @@ -1029,6 +1030,27 @@ class Transaction: self.db._connection().executescript(statements) +@dataclass +class Migration(ABC): + db: Database + + @cached_classproperty + def name(cls) -> str: + """Class name (except Migration) converted to snake case.""" + name = cls.__name__.removesuffix("Migration") # type: ignore[attr-defined] + return re.sub(r"(?<=[a-z])(?=[A-Z])", "_", name).lower() + + def migrate_table(self, table: str) -> None: + """Migrate a specific table.""" + if not self.db.migration_exists(self.name, table): + self._migrate_data(table) + self.db.record_migration(self.name, table) + + @abstractmethod + def _migrate_data(self, table: str) -> None: + """Migrate data for a specific table.""" + + class Database: """A container for Model objects that wraps an SQLite database as the backend. @@ -1038,6 +1060,9 @@ class Database: """The Model subclasses representing tables in this database. """ + _migrations: Sequence[tuple[type[Migration], Sequence[type[Model]]]] = () + """Migrations that are to be performed for the configured models.""" + supports_extensions = hasattr(sqlite3.Connection, "enable_load_extension") """Whether or not the current version of SQLite supports extensions""" @@ -1302,6 +1327,10 @@ class Database: def _migrate(self) -> None: """Perform any necessary migration for the database.""" + for migration_cls, model_classes in self._migrations: + migration = migration_cls(self) + for model_cls in model_classes: + migration.migrate_table(model_cls._table) def migration_exists(self, name: str, table: str) -> bool: """Return whether a named migration has been marked complete.""" From 2ecbe59f48ddc07bc9ba31c8c16102442ef7870f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 8 Feb 2026 21:28:13 +0000 Subject: [PATCH 40/60] Add migration for multi-value genres field * Move genre-to-genres migration into a dedicated Migration class and wire it into Library._migrations for items and albums. * Add batched SQL updates via mutate_many and share the multi-value delimiter as a constant. * Cover migration behavior with new tests. I initially attempted to migrate using our model infrastructure / Model.store(), see the comparison below: Durations migrating my library of ~9000 items and ~2300 albums: 1. Using our Python logic: 11 minutes 2. Using SQL directly: 4 seconds That's why I've gone ahead with option 2. --- beets/dbcore/db.py | 33 ++++++--- beets/dbcore/types.py | 3 +- beets/library/library.py | 115 ++------------------------------ beets/library/migrations.py | 94 ++++++++++++++++++++++++++ beets/test/helper.py | 2 + test/library/__init__.py | 0 test/library/test_migrations.py | 56 ++++++++++++++++ 7 files changed, 182 insertions(+), 121 deletions(-) create mode 100644 beets/library/migrations.py create mode 100644 test/library/__init__.py create mode 100644 test/library/test_migrations.py diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 4b0ba4f15..af4315267 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -16,7 +16,6 @@ from __future__ import annotations -import contextlib import functools import os import re @@ -27,6 +26,7 @@ import time from abc import ABC, abstractmethod from collections import defaultdict from collections.abc import Mapping +from contextlib import contextmanager from dataclasses import dataclass from functools import cached_property from sqlite3 import Connection, sqlite_version_info @@ -1002,12 +1002,15 @@ class Transaction: cursor = self.db._connection().execute(statement, subvals) return cursor.fetchall() - def mutate(self, statement: str, subvals: Sequence[SQLiteType] = ()) -> Any: - """Execute an SQL statement with substitution values and return - the row ID of the last affected row. + @contextmanager + def _handle_mutate(self) -> Iterator[None]: + """Handle mutation bookkeeping and database access errors. + + Yield control to mutation execution code. If execution succeeds, + mark this transaction as mutated. """ try: - cursor = self.db._connection().execute(statement, subvals) + yield except sqlite3.OperationalError as e: # In two specific cases, SQLite reports an error while accessing # the underlying database file. We surface these exceptions as @@ -1017,11 +1020,23 @@ class Transaction: "unable to open database file", ): raise DBAccessError(e.args[0]) - else: - raise + raise else: self._mutated = True - return cursor.lastrowid + + def mutate(self, statement: str, subvals: Sequence[SQLiteType] = ()) -> Any: + """Run one write statement with shared mutation/error handling.""" + with self._handle_mutate(): + return self.db._connection().execute(statement, subvals).lastrowid + + def mutate_many( + self, statement: str, subvals: Sequence[tuple[SQLiteType, ...]] = () + ) -> Any: + """Run batched writes with shared mutation/error handling.""" + with self._handle_mutate(): + return ( + self.db._connection().executemany(statement, subvals).lastrowid + ) def script(self, statements: str): """Execute a string containing multiple SQL statements.""" @@ -1214,7 +1229,7 @@ class Database: _thread_id, conn = self._connections.popitem() conn.close() - @contextlib.contextmanager + @contextmanager def _tx_stack(self) -> Generator[list[Transaction]]: """A context manager providing access to the current thread's transaction stack. The context manager synchronizes access to diff --git a/beets/dbcore/types.py b/beets/dbcore/types.py index 8907584a4..e50693474 100644 --- a/beets/dbcore/types.py +++ b/beets/dbcore/types.py @@ -30,6 +30,7 @@ from . import query SQLiteType = query.SQLiteType BLOB_TYPE = query.BLOB_TYPE +MULTI_VALUE_DELIMITER = "\\␀" class ModelType(typing.Protocol): @@ -481,4 +482,4 @@ DATE = DateType() SEMICOLON_SPACE_DSV = DelimitedString("; ") # Will set the proper null char in mediafile -MULTI_VALUE_DSV = DelimitedString("\\␀") +MULTI_VALUE_DSV = DelimitedString(MULTI_VALUE_DELIMITER) diff --git a/beets/library/library.py b/beets/library/library.py index 16a8fcf93..b161b7399 100644 --- a/beets/library/library.py +++ b/beets/library/library.py @@ -5,25 +5,22 @@ from typing import TYPE_CHECKING import platformdirs import beets -from beets import dbcore, logging, ui -from beets.autotag import correct_list_fields +from beets import dbcore from beets.util import normpath +from .migrations import MultiGenreFieldMigration from .models import Album, Item from .queries import PF_KEY_DEFAULT, parse_query_parts, parse_query_string if TYPE_CHECKING: - from collections.abc import Mapping - - from beets.dbcore import Results, types - -log = logging.getLogger("beets") + from beets.dbcore import Results class Library(dbcore.Database): """A database of music containing songs and albums.""" _models = (Item, Album) + _migrations = ((MultiGenreFieldMigration, (Item, Album)),) def __init__( self, @@ -147,107 +144,3 @@ class Library(dbcore.Database): item_or_id if isinstance(item_or_id, int) else item_or_id.album_id ) return self._get(Album, album_id) if album_id else None - - # Database schema migration. - - def _make_table(self, table: str, fields: Mapping[str, types.Type]): - """Set up the schema of the database, and migrate genres if needed.""" - with self.transaction() as tx: - rows = tx.query(f"PRAGMA table_info({table})") - current_fields = {row[1] for row in rows} - field_names = set(fields.keys()) - - # Check if genres column is being added to items table - genres_being_added = ( - table == "items" - and "genres" in field_names - and "genres" not in current_fields - and "genre" in current_fields - ) - - # Call parent to create/update table - super()._make_table(table, fields) - - # Migrate genre to genres if genres column was just added - if genres_being_added: - self._migrate_genre_to_genres() - - def _migrate_genre_to_genres(self): - """Migrate comma-separated genre strings to genres list. - - This migration runs automatically when the genres column is first - created in the database. It splits comma-separated genre values - and writes the changes to both the database and media files. - """ - items = list(self.items()) - migrated_count = 0 - total_items = len(items) - - if total_items == 0: - return - - ui.print_(f"Migrating genres for {total_items} items...") - - for index, item in enumerate(items, 1): - genre_val = item.genre or "" - genres_val = item.genres or [] - - # Check if migration is needed - needs_migration = False - split_genres = [] - if not genres_val and genre_val: - separators = [] - if ( - "lastgenre" in beets.config - and "separator" in beets.config["lastgenre"] - ): - try: - user_sep = beets.config["lastgenre"][ - "separator" - ].as_str() - if user_sep: - separators.append(user_sep) - except ( - beets.config.ConfigNotFoundError, - beets.config.ConfigTypeError, - ): - pass - - separators.extend([", ", "; ", " / "]) - - for separator in separators: - if separator in genre_val: - split_genres = [ - g.strip() - for g in genre_val.split(separator) - if g.strip() - ] - if len(split_genres) > 1: - needs_migration = True - break - - if needs_migration: - migrated_count += 1 - # Show progress every 100 items - if migrated_count % 100 == 0: - ui.print_( - f" Migrated {migrated_count} items " - f"({index}/{total_items} processed)..." - ) - # Migrate using the same logic as correct_list_fields - correct_list_fields(item) - item.store() - # Write to media file - try: - item.try_write() - except Exception as e: - log.warning( - "Could not write genres to {}: {}", - item.path, - e, - ) - - ui.print_( - f"Migration complete: {migrated_count} of {total_items} items " - f"updated with comma-separated genres" - ) diff --git a/beets/library/migrations.py b/beets/library/migrations.py new file mode 100644 index 000000000..e2fa80f63 --- /dev/null +++ b/beets/library/migrations.py @@ -0,0 +1,94 @@ +from __future__ import annotations + +from contextlib import contextmanager, suppress +from functools import cached_property +from typing import TYPE_CHECKING, NamedTuple, TypeVar + +from confuse.exceptions import ConfigError + +import beets +from beets import ui +from beets.dbcore.db import Migration +from beets.dbcore.types import MULTI_VALUE_DELIMITER +from beets.util import unique_list + +if TYPE_CHECKING: + from collections.abc import Iterator + +T = TypeVar("T") + + +class GenreRow(NamedTuple): + id: int + genre: str + genres: str | None + + +def chunks(lst: list[T], n: int) -> Iterator[list[T]]: + """Yield successive n-sized chunks from lst.""" + for i in range(0, len(lst), n): + yield lst[i : i + n] + + +class MultiGenreFieldMigration(Migration): + @cached_property + def separators(self) -> list[str]: + separators = [] + with suppress(ConfigError): + separators.append(beets.config["lastgenre"]["separator"].as_str()) + + separators.extend([", ", "; ", " / "]) + return unique_list(filter(None, separators)) + + @contextmanager + def with_factory(self, factory: type[NamedTuple]) -> Iterator[None]: + """Temporarily set the row factory to a specific type.""" + original_factory = self.db._connection().row_factory + self.db._connection().row_factory = lambda _, row: factory(*row) + try: + yield + finally: + self.db._connection().row_factory = original_factory + + def get_genres(self, genre: str) -> str: + for separator in self.separators: + if separator in genre: + return genre.replace(separator, MULTI_VALUE_DELIMITER) + + return genre + + def _migrate_data(self, table: str) -> None: + """Migrate legacy genre values to the multi-value genres field.""" + + with self.db.transaction() as tx, self.with_factory(GenreRow): + rows: list[GenreRow] = tx.query( # type: ignore[assignment] + f""" + SELECT id, genre, genres + FROM {table} + WHERE genre IS NOT NULL AND genre != '' + """ + ) + + total = len(rows) + to_migrate = [e for e in rows if not e.genres] + if not to_migrate: + return + + migrated = total - len(to_migrate) + + ui.print_(f"Migrating genres for {total} {table}...") + for batch in chunks(to_migrate, 1000): + with self.db.transaction() as tx: + tx.mutate_many( + f"UPDATE {table} SET genres = ? WHERE id = ?", + [(self.get_genres(e.genre), e.id) for e in batch], + ) + + migrated += len(batch) + + ui.print_( + f" Migrated {migrated} {table} " + f"({migrated}/{total} processed)..." + ) + + ui.print_(f"Migration complete: {migrated} of {total} {table} updated") diff --git a/beets/test/helper.py b/beets/test/helper.py index 7762ab866..218b778c7 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -156,6 +156,8 @@ class TestHelper(RunMixin, ConfigMixin): fixtures. """ + lib: Library + resource_path = Path(os.fsdecode(_common.RSRC)) / "full.mp3" db_on_disk: ClassVar[bool] = False diff --git a/test/library/__init__.py b/test/library/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/test/library/test_migrations.py b/test/library/test_migrations.py new file mode 100644 index 000000000..dba0d8718 --- /dev/null +++ b/test/library/test_migrations.py @@ -0,0 +1,56 @@ +import pytest + +from beets.library.migrations import MultiGenreFieldMigration +from beets.library.models import Album, Item +from beets.test.helper import TestHelper + + +class TestMultiGenreFieldMigration: + @pytest.fixture + def helper(self, monkeypatch): + # do not apply migrations upon library initialization + monkeypatch.setattr("beets.library.library.Library._migrations", ()) + helper = TestHelper() + helper.setup_beets() + + # and now configure the migrations to be tested + monkeypatch.setattr( + "beets.library.library.Library._migrations", + ((MultiGenreFieldMigration, (Item, Album)),), + ) + yield helper + + helper.teardown_beets() + + def test_migrates_only_rows_with_missing_genres(self, helper: TestHelper): + helper.config["lastgenre"]["separator"] = " - " + + expected_item_genres = [] + for genre, initial_genres, expected_genres in [ + # already existing value is not overwritten + ("Item Rock", ("Ignored",), ("Ignored",)), + ("", (), ()), + ("Rock", (), ("Rock",)), + # multiple genres are split on one of default separators + ("Item Rock; Alternative", (), ("Item Rock", "Alternative")), + # multiple genres are split the first (lastgenre) separator ONLY + ("Item - Rock, Alternative", (), ("Item", "Rock, Alternative")), + ]: + helper.add_item(genre=genre, genres=initial_genres) + expected_item_genres.append(expected_genres) + + unmigrated_album = helper.add_album( + genre="Album Rock / Alternative", genres=[] + ) + expected_item_genres.append(("Album Rock", "Alternative")) + + helper.lib._migrate() + + actual_item_genres = [tuple(i.genres) for i in helper.lib.items()] + assert actual_item_genres == expected_item_genres + + unmigrated_album.load() + assert unmigrated_album.genres == ["Album Rock", "Alternative"] + + assert helper.lib.migration_exists("multi_genre_field", "items") + assert helper.lib.migration_exists("multi_genre_field", "albums") From 4dda8e3e49489f0b1b6e83dbc9a35d9fbc85e0a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 9 Feb 2026 22:28:45 +0000 Subject: [PATCH 41/60] Fix deprecation warning --- beets/autotag/hooks.py | 26 +++++++++----------------- test/autotag/test_hooks.py | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 17 deletions(-) create mode 100644 test/autotag/test_hooks.py diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index ef9e8bb30..617d5051a 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -16,7 +16,6 @@ from __future__ import annotations -import warnings from copy import deepcopy from dataclasses import dataclass from functools import cached_property @@ -25,6 +24,7 @@ from typing import TYPE_CHECKING, Any, TypeVar from typing_extensions import Self from beets.util import cached_classproperty +from beets.util.deprecation import deprecate_for_maintainers if TYPE_CHECKING: from beets.library import Item @@ -81,25 +81,17 @@ class Info(AttrDict[Any]): media: str | None = None, **kwargs, ) -> None: - if genre: - warnings.warn( - "The 'genre' parameter is deprecated. Use 'genres' (list) instead.", - DeprecationWarning, - stacklevel=2, + if genre is not None: + deprecate_for_maintainers( + "The 'genre' parameter", "'genres' (list)", stacklevel=3 ) if not genres: - for separator in [", ", "; ", " / "]: - if separator in genre: - split_genres = [ - g.strip() - for g in genre.split(separator) - if g.strip() - ] - if len(split_genres) > 1: - genres = split_genres - break - if not genres: + try: + sep = next(s for s in [", ", "; ", " / "] if s in genre) + except StopIteration: genres = [genre] + else: + genres = list(map(str.strip, genre.split(sep))) self.album = album self.artist = artist diff --git a/test/autotag/test_hooks.py b/test/autotag/test_hooks.py new file mode 100644 index 000000000..e5de089e8 --- /dev/null +++ b/test/autotag/test_hooks.py @@ -0,0 +1,17 @@ +import pytest + +from beets.autotag.hooks import Info + + +@pytest.mark.parametrize( + "genre, expected_genres", + [ + ("Rock", ("Rock",)), + ("Rock; Alternative", ("Rock", "Alternative")), + ], +) +def test_genre_deprecation(genre, expected_genres): + with pytest.warns( + DeprecationWarning, match="The 'genre' parameter is deprecated" + ): + assert tuple(Info(genre=genre).genres) == expected_genres From cf36ed07546d78c27df6d1d4d57ebda0d1750be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 9 Feb 2026 22:54:24 +0000 Subject: [PATCH 42/60] Only handle multiple genres in discogs --- beetsplug/discogs/__init__.py | 23 ++++++++--------------- docs/plugins/discogs.rst | 18 +++++++++++------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/beetsplug/discogs/__init__.py b/beetsplug/discogs/__init__.py index bdbeb8fc0..b33af83a2 100644 --- a/beetsplug/discogs/__init__.py +++ b/beetsplug/discogs/__init__.py @@ -352,12 +352,11 @@ class DiscogsPlugin(MetadataSourcePlugin): mediums = [t["medium"] for t in tracks] country = result.data.get("country") data_url = result.data.get("uri") - style = self.format(result.data.get("styles")) - base_genre = self.format(result.data.get("genres")) + styles: list[str] = result.data.get("styles") or [] + genres: list[str] = result.data.get("genres") or [] - genre = base_genre - if self.config["append_style_genre"] and genre is not None and style: - genre += f"{self.config['separator'].as_str()}{style}" + if self.config["append_style_genre"]: + genres.extend(styles) discogs_albumid = self._extract_id(result.data.get("uri")) @@ -411,8 +410,10 @@ class DiscogsPlugin(MetadataSourcePlugin): releasegroup_id=master_id, catalognum=catalogno, country=country, - style=style, - genre=genre, + style=( + self.config["separator"].as_str().join(sorted(styles)) or None + ), + genres=sorted(genres), media=media, original_year=original_year, data_source=self.data_source, @@ -433,14 +434,6 @@ class DiscogsPlugin(MetadataSourcePlugin): return None - def format(self, classification: Iterable[str]) -> str | None: - if classification: - return ( - self.config["separator"].as_str().join(sorted(classification)) - ) - else: - return None - def get_tracks( self, tracklist: list[Track], diff --git a/docs/plugins/discogs.rst b/docs/plugins/discogs.rst index 780042026..3734b57e7 100644 --- a/docs/plugins/discogs.rst +++ b/docs/plugins/discogs.rst @@ -116,17 +116,21 @@ Default .. conf:: append_style_genre :default: no - Appends the Discogs style (if found) to the genre tag. This can be useful if - you want more granular genres to categorize your music. For example, - a release in Discogs might have a genre of "Electronic" and a style of - "Techno": enabling this setting would set the genre to be "Electronic, - Techno" (assuming default separator of ``", "``) instead of just - "Electronic". + Appends the Discogs style (if found) to the ``genres`` tag. This can be + useful if you want more granular genres to categorize your music. For + example, a release in Discogs might have a genre of "Electronic" and a style + of "Techno": enabling this setting would append "Techno" to the ``genres`` + list. .. conf:: separator :default: ", " - How to join multiple genre and style values from Discogs into a string. + How to join multiple style values from Discogs into a string. + + .. versionchanged:: 2.7.0 + + This option now only applies to the ``style`` field as beets now only + handles lists of ``genres``. .. conf:: strip_disambiguation :default: yes From b8f1b9d174afb353b93e7ee3f04c31cb40fdd796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 10 Feb 2026 01:47:45 +0000 Subject: [PATCH 43/60] Stop overwriting this test file name --- test/rsrc/unicode’d.mp3 | Bin 75297 -> 75297 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/test/rsrc/unicode’d.mp3 b/test/rsrc/unicode’d.mp3 index f7e8b6285ac6eb3d606a6f7fcaee76a4f8f9e735..2b306cc13e8cdf9592b1b5a6f45c4201c2dd3861 100644 GIT binary patch delta 46 zcmZ2@hGpRymJN}NQXvr$5ey6rf(#7IK8{YVJ`5!psR}uXNvS!TBN%V|+U))3nkE2F Ce-IA< delta 23 fcmZ2@hGpRymJN}Nn@bq4|C*feLwd9KpKF=`iG2&@ From 5d7fb4e1586c5fa5c4e79a6010dd65ca37e3c37b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Tue, 10 Feb 2026 01:49:10 +0000 Subject: [PATCH 44/60] Remove genre field --- beets/autotag/__init__.py | 1 - beets/dbcore/db.py | 86 +++++++++++++++++++++------------ beets/library/migrations.py | 5 +- beets/library/models.py | 7 +-- beetsplug/bpd/__init__.py | 5 +- beetsplug/smartplaylist.py | 4 +- test/library/test_migrations.py | 16 ++++++ test/test_autotag.py | 68 -------------------------- test/test_library.py | 28 ----------- 9 files changed, 83 insertions(+), 137 deletions(-) diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index 4cc4ff30a..feeefbf28 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -167,7 +167,6 @@ def correct_list_fields(m: LibModel) -> None: setattr(m, single_field, list_val[0]) ensure_first_value("albumtype", "albumtypes") - ensure_first_value("genre", "genres") if hasattr(m, "mb_artistids"): ensure_first_value("mb_artistid", "mb_artistids") diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index af4315267..cec6abc46 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -30,7 +30,16 @@ from contextlib import contextmanager from dataclasses import dataclass from functools import cached_property from sqlite3 import Connection, sqlite_version_info -from typing import TYPE_CHECKING, Any, AnyStr, ClassVar, Generic, NamedTuple +from typing import ( + TYPE_CHECKING, + Any, + AnyStr, + ClassVar, + Generic, + Literal, + NamedTuple, + TypedDict, +) from typing_extensions import ( Self, @@ -1055,17 +1064,22 @@ class Migration(ABC): name = cls.__name__.removesuffix("Migration") # type: ignore[attr-defined] return re.sub(r"(?<=[a-z])(?=[A-Z])", "_", name).lower() - def migrate_table(self, table: str) -> None: + def migrate_table(self, table: str, *args, **kwargs) -> None: """Migrate a specific table.""" if not self.db.migration_exists(self.name, table): - self._migrate_data(table) + self._migrate_data(table, *args, **kwargs) self.db.record_migration(self.name, table) @abstractmethod - def _migrate_data(self, table: str) -> None: + def _migrate_data(self, table: str, current_fields: set[str]) -> None: """Migrate data for a specific table.""" +class TableInfo(TypedDict): + columns: set[str] + migrations: set[str] + + class Database: """A container for Model objects that wraps an SQLite database as the backend. @@ -1129,6 +1143,32 @@ class Database: self._migrate() + @cached_property + def db_tables(self) -> dict[str, TableInfo]: + column_queries = [ + f""" + SELECT '{m._table}' AS table_name, 'columns' AS source, name + FROM pragma_table_info('{m._table}') + """ + for m in self._models + ] + with self.transaction() as tx: + rows = tx.query(f""" + {" UNION ALL ".join(column_queries)} + UNION ALL + SELECT table_name, 'migrations' AS source, name FROM migrations + """) + + tables_data: dict[str, TableInfo] = defaultdict( + lambda: TableInfo(columns=set(), migrations=set()) + ) + + source: Literal["columns", "migrations"] + for table_name, source, name in rows: + tables_data[table_name][source].add(name) + + return tables_data + # Primitive access control: connections and transactions. def _connection(self) -> Connection: @@ -1269,36 +1309,27 @@ class Database: """Set up the schema of the database. `fields` is a mapping from field names to `Type`s. Columns are added if necessary. """ - # Get current schema. - with self.transaction() as tx: - rows = tx.query(f"PRAGMA table_info({table})") - current_fields = {row[1] for row in rows} - - field_names = set(fields.keys()) - if current_fields.issuperset(field_names): - # Table exists and has all the required columns. - return - - if not current_fields: + if table not in self.db_tables: # No table exists. columns = [] for name, typ in fields.items(): columns.append(f"{name} {typ.sql}") setup_sql = f"CREATE TABLE {table} ({', '.join(columns)});\n" - else: # Table exists does not match the field set. setup_sql = "" + current_fields = self.db_tables[table]["columns"] for name, typ in fields.items(): - if name in current_fields: - continue - setup_sql += ( - f"ALTER TABLE {table} ADD COLUMN {name} {typ.sql};\n" - ) + if name not in current_fields: + setup_sql += ( + f"ALTER TABLE {table} ADD COLUMN {name} {typ.sql};\n" + ) with self.transaction() as tx: tx.script(setup_sql) + self.db_tables[table]["columns"] = set(fields) + def _make_attribute_table(self, flex_table: str): """Create a table and associated index for flexible attributes for the given entity (if they don't exist). @@ -1345,19 +1376,12 @@ class Database: for migration_cls, model_classes in self._migrations: migration = migration_cls(self) for model_cls in model_classes: - migration.migrate_table(model_cls._table) + table = model_cls._table + migration.migrate_table(table, self.db_tables[table]["columns"]) def migration_exists(self, name: str, table: str) -> bool: """Return whether a named migration has been marked complete.""" - with self.transaction() as tx: - return tx.execute( - """ - SELECT EXISTS( - SELECT 1 FROM migrations WHERE name = ? AND table_name = ? - ) - """, - (name, table), - ).fetchone()[0] + return name in self.db_tables[table]["migrations"] def record_migration(self, name: str, table: str) -> None: """Set completion state for a named migration.""" diff --git a/beets/library/migrations.py b/beets/library/migrations.py index e2fa80f63..16f4c6761 100644 --- a/beets/library/migrations.py +++ b/beets/library/migrations.py @@ -57,8 +57,11 @@ class MultiGenreFieldMigration(Migration): return genre - def _migrate_data(self, table: str) -> None: + def _migrate_data(self, table: str, current_fields: set[str]) -> None: """Migrate legacy genre values to the multi-value genres field.""" + if "genre" not in current_fields: + # No legacy genre field, so nothing to migrate. + return with self.db.transaction() as tx, self.with_factory(GenreRow): rows: list[GenreRow] = tx.query( # type: ignore[assignment] diff --git a/beets/library/models.py b/beets/library/models.py index 71445c203..9b8b6d291 100644 --- a/beets/library/models.py +++ b/beets/library/models.py @@ -241,7 +241,6 @@ class Album(LibModel): "albumartists_sort": types.MULTI_VALUE_DSV, "albumartists_credit": types.MULTI_VALUE_DSV, "album": types.STRING, - "genre": types.STRING, "genres": types.MULTI_VALUE_DSV, "style": types.STRING, "discogs_albumid": types.INTEGER, @@ -277,7 +276,7 @@ class Album(LibModel): "original_day": types.PaddedInt(2), } - _search_fields = ("album", "albumartist", "genre") + _search_fields = ("album", "albumartist", "genres") @cached_classproperty def _types(cls) -> dict[str, types.Type]: @@ -298,7 +297,6 @@ class Album(LibModel): "albumartist_credit", "albumartists_credit", "album", - "genre", "genres", "style", "discogs_albumid", @@ -652,7 +650,6 @@ class Item(LibModel): "albumartists_sort": types.MULTI_VALUE_DSV, "albumartist_credit": types.STRING, "albumartists_credit": types.MULTI_VALUE_DSV, - "genre": types.STRING, "genres": types.MULTI_VALUE_DSV, "style": types.STRING, "discogs_albumid": types.INTEGER, @@ -735,7 +732,7 @@ class Item(LibModel): "comments", "album", "albumartist", - "genre", + "genres", ) # Set of item fields that are backed by `MediaFile` fields. diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 9496e9a78..835848d42 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1137,7 +1137,10 @@ class Server(BaseServer): pass for tagtype, field in self.tagtype_map.items(): - info_lines.append(f"{tagtype}: {getattr(item, field)}") + field_value = getattr(item, field) + if isinstance(field_value, list): + field_value = "; ".join(field_value) + info_lines.append(f"{tagtype}: {field_value}") return info_lines diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py index a5cc8e362..ff5e25612 100644 --- a/beetsplug/smartplaylist.py +++ b/beetsplug/smartplaylist.py @@ -359,8 +359,8 @@ class SmartPlaylistPlugin(BeetsPlugin): if extm3u: attr = [(k, entry.item[k]) for k in keys] al = [ - f' {key}="{quote(str(value), safe="/:")}"' - for key, value in attr + f' {k}="{quote("; ".join(v) if isinstance(v, list) else str(v), safe="/:")}"' # noqa: E501 + for k, v in attr ] attrs = "".join(al) comment = ( diff --git a/test/library/test_migrations.py b/test/library/test_migrations.py index dba0d8718..2c0dece8b 100644 --- a/test/library/test_migrations.py +++ b/test/library/test_migrations.py @@ -1,5 +1,6 @@ import pytest +from beets.dbcore import types from beets.library.migrations import MultiGenreFieldMigration from beets.library.models import Album, Item from beets.test.helper import TestHelper @@ -10,6 +11,19 @@ class TestMultiGenreFieldMigration: def helper(self, monkeypatch): # do not apply migrations upon library initialization monkeypatch.setattr("beets.library.library.Library._migrations", ()) + # add genre field to both models to make sure this column is created + monkeypatch.setattr( + "beets.library.models.Item._fields", + {**Item._fields, "genre": types.STRING}, + ) + monkeypatch.setattr( + "beets.library.models.Album._fields", + {**Album._fields, "genre": types.STRING}, + ) + monkeypatch.setattr( + "beets.library.models.Album.item_keys", + {*Album.item_keys, "genre"}, + ) helper = TestHelper() helper.setup_beets() @@ -52,5 +66,7 @@ class TestMultiGenreFieldMigration: unmigrated_album.load() assert unmigrated_album.genres == ["Album Rock", "Alternative"] + # remove cached initial db tables data + del helper.lib.db_tables assert helper.lib.migration_exists("multi_genre_field", "items") assert helper.lib.migration_exists("multi_genre_field", "albums") diff --git a/test/test_autotag.py b/test/test_autotag.py index e6a122ae2..119ca15e8 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -475,71 +475,3 @@ def test_correct_list_fields( single_val, list_val = item[single_field], item[list_field] assert (not single_val and not list_val) or single_val == list_val[0] - - -# Tests for multi-value genres functionality -class TestGenreSync: - """Test the genre/genres field synchronization.""" - - def test_genres_list_to_genre_first(self): - """Genres list sets genre to first item.""" - item = Item(genres=["Rock", "Alternative", "Indie"]) - correct_list_fields(item) - - assert item.genre == "Rock" - assert item.genres == ["Rock", "Alternative", "Indie"] - - def test_genre_string_to_genres_list(self): - """Genre string becomes first item in genres list.""" - item = Item(genre="Rock") - correct_list_fields(item) - - assert item.genre == "Rock" - assert item.genres == ["Rock"] - - def test_genre_and_genres_both_present(self): - """When both genre and genres exist, genre becomes first in list.""" - item = Item(genre="Jazz", genres=["Rock", "Alternative"]) - correct_list_fields(item) - - # genre should be prepended to genres list (deduplicated) - assert item.genre == "Jazz" - assert item.genres == ["Jazz", "Rock", "Alternative"] - - def test_empty_genre(self): - """Empty genre field.""" - item = Item(genre="") - correct_list_fields(item) - - assert item.genre == "" - assert item.genres == [] - - def test_empty_genres(self): - """Empty genres list.""" - item = Item(genres=[]) - correct_list_fields(item) - - assert item.genre == "" - assert item.genres == [] - - def test_none_values(self): - """Handle None values in genre/genres fields without errors.""" - # Test with None genre - item = Item(genre=None, genres=["Rock"]) - correct_list_fields(item) - assert item.genres == ["Rock"] - assert item.genre == "Rock" - - # Test with None genres - item = Item(genre="Jazz", genres=None) - correct_list_fields(item) - assert item.genre == "Jazz" - assert item.genres == ["Jazz"] - - def test_none_both(self): - """Handle None in both genre and genres.""" - item = Item(genre=None, genres=None) - correct_list_fields(item) - - assert item.genres == [] - assert item.genre == "" diff --git a/test/test_library.py b/test/test_library.py index bf3508e88..4df4e4b58 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -701,34 +701,6 @@ class DestinationFunctionTest(BeetsTestCase, PathFormattingMixin): self._setf("%first{Alice / Bob / Eve,2,0, / , & }") self._assert_dest(b"/base/Alice & Bob") - def test_first_genres_list(self): - # Test that setting genres as a list syncs to genre field properly - # and works with %first template function - from beets.autotag import correct_list_fields - - # Clear the default genre first - self.i.genre = "" - self.i.genres = ["Pop", "Rock", "Classical Crossover"] - correct_list_fields(self.i) - # genre field should now be synced to first item - assert self.i.genre == "Pop" - # %first should work on the genre field - self._setf("%first{$genre}") - self._assert_dest(b"/base/Pop") - - def test_first_genres_list_skip(self): - # Test that the genres list is accessible as a multi-value field - from beets.autotag import correct_list_fields - - # Clear the default genre first - self.i.genre = "" - self.i.genres = ["Pop", "Rock", "Classical Crossover"] - correct_list_fields(self.i) - # Access the second genre directly using index (genres is a list) - # The genres field should be available as a multi-value field - assert self.i.genres[1] == "Rock" - assert len(self.i.genres) == 3 - class DisambiguationTest(BeetsTestCase, PathFormattingMixin): def setUp(self): From a8d53f78de71a32ea0e3ed15099726728877ce30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 15 Feb 2026 13:17:22 +0000 Subject: [PATCH 45/60] Fix the rest of the tests --- beets/test/_common.py | 2 +- beetsplug/aura.py | 6 ++++-- beetsplug/bpd/__init__.py | 2 +- beetsplug/fish.py | 4 ++-- beetsplug/musicbrainz.py | 11 ++++++---- docs/plugins/smartplaylist.rst | 4 ++-- docs/reference/cli.rst | 4 ++-- docs/reference/config.rst | 10 +++++----- test/plugins/test_beatport.py | 4 ++-- test/plugins/test_ihate.py | 8 ++++---- test/plugins/test_musicbrainz.py | 2 +- test/plugins/test_smartplaylist.py | 14 ++++++------- test/test_importer.py | 22 ++++++++++---------- test/test_library.py | 12 ++++++----- test/test_query.py | 32 ++++++++++++------------------ test/test_sort.py | 20 +++++++++---------- test/ui/commands/test_list.py | 2 +- test/ui/commands/test_update.py | 18 ++++++++--------- test/ui/test_field_diff.py | 4 ++-- 19 files changed, 90 insertions(+), 91 deletions(-) diff --git a/beets/test/_common.py b/beets/test/_common.py index 4de47c337..b3c21aaa5 100644 --- a/beets/test/_common.py +++ b/beets/test/_common.py @@ -75,7 +75,7 @@ def item(lib=None, **kwargs): artist="the artist", albumartist="the album artist", album="the album", - genre="the genre", + genres=["the genre"], lyricist="the lyricist", composer="the composer", arranger="the arranger", diff --git a/beetsplug/aura.py b/beetsplug/aura.py index c1877db82..b30e66bf0 100644 --- a/beetsplug/aura.py +++ b/beetsplug/aura.py @@ -79,7 +79,8 @@ TRACK_ATTR_MAP = { "month": "month", "day": "day", "bpm": "bpm", - "genre": "genre", + "genre": "genres", + "genres": "genres", "recording-mbid": "mb_trackid", # beets trackid is MB recording "track-mbid": "mb_releasetrackid", "composer": "composer", @@ -109,7 +110,8 @@ ALBUM_ATTR_MAP = { "year": "year", "month": "month", "day": "day", - "genre": "genre", + "genre": "genres", + "genres": "genres", "release-mbid": "mb_albumid", "release-group-mbid": "mb_releasegroupid", } diff --git a/beetsplug/bpd/__init__.py b/beetsplug/bpd/__init__.py index 835848d42..16eb8c572 100644 --- a/beetsplug/bpd/__init__.py +++ b/beetsplug/bpd/__init__.py @@ -1354,7 +1354,7 @@ class Server(BaseServer): "AlbumArtist": "albumartist", "AlbumArtistSort": "albumartist_sort", "Label": "label", - "Genre": "genre", + "Genre": "genres", "Date": "year", "OriginalDate": "original_year", "Composer": "composer", diff --git a/beetsplug/fish.py b/beetsplug/fish.py index 82e035eb4..9de764656 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -16,10 +16,10 @@ """This plugin generates tab completions for Beets commands for the Fish shell , including completions for Beets commands, plugin commands, and option flags. Also generated are completions for all the album -and track fields, suggesting for example `genre:` or `album:` when querying the +and track fields, suggesting for example `genres:` or `album:` when querying the Beets database. Completions for the *values* of those fields are not generated by default but can be added via the `-e` / `--extravalues` flag. For example: -`beet fish -e genre -e albumartist` +`beet fish -e genres -e albumartist` """ import os diff --git a/beetsplug/musicbrainz.py b/beetsplug/musicbrainz.py index 75933e6f9..090bd617a 100644 --- a/beetsplug/musicbrainz.py +++ b/beetsplug/musicbrainz.py @@ -644,10 +644,13 @@ class MusicBrainzPlugin(MusicBrainzAPIMixin, MetadataSourcePlugin): for source in sources: for genreitem in source: genres[genreitem["name"]] += int(genreitem["count"]) - info.genres = [ - genre - for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) - ] + if genres: + info.genres = [ + genre + for genre, _count in sorted( + genres.items(), key=lambda g: -g[1] + ) + ] # We might find links to external sources (Discogs, Bandcamp, ...) external_ids = self.config["external_ids"].get() diff --git a/docs/plugins/smartplaylist.rst b/docs/plugins/smartplaylist.rst index f227559a8..48060ea79 100644 --- a/docs/plugins/smartplaylist.rst +++ b/docs/plugins/smartplaylist.rst @@ -121,7 +121,7 @@ instance the following configuration exports the ``id`` and ``genre`` fields: output: extm3u fields: - id - - genre + - genres playlists: - name: all.m3u query: '' @@ -132,7 +132,7 @@ look as follows: :: #EXTM3U - #EXTINF:805 id="1931" genre="Progressive%20Rock",Led Zeppelin - Stairway to Heaven + #EXTINF:805 id="1931" genres="Rock%3B%20Pop",Led Zeppelin - Stairway to Heaven ../music/singles/Led Zeppelin/Stairway to Heaven.mp3 To give a usage example, the webm3u_ and Beetstream_ plugins read the exported diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 15024022b..6f60d2232 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -143,9 +143,9 @@ Optional command flags: :ref:`set_fields` configuration dictionary. You can use the option multiple times on the command line, like so: - :: +.. code-block:: sh - beet import --set genre="Alternative Rock" --set mood="emotional" + beet import --set genres="Alternative Rock" --set mood="emotional" .. _py7zr: https://pypi.org/project/py7zr/ diff --git a/docs/reference/config.rst b/docs/reference/config.rst index b654c118f..fc0de37a7 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -853,11 +853,11 @@ set_fields A dictionary indicating fields to set to values for newly imported music. Here's an example: -:: +.. code-block:: yaml set_fields: - genre: 'To Listen' - collection: 'Unordered' + genres: To Listen + collection: Unordered Other field/value pairs supplied via the ``--set`` option on the command-line override any settings here for fields with the same name. @@ -1172,9 +1172,9 @@ Here's an example file: color: yes paths: - default: $genre/$albumartist/$album/$track $title + default: %first{$genres}/$albumartist/$album/$track $title singleton: Singletons/$artist - $title - comp: $genre/$album/$track $title + comp: %first{$genres}/$album/$track $title albumtype:soundtrack: Soundtracks/$album/$track $title .. only:: man diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index 916227f40..442f80037 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -474,7 +474,7 @@ class BeatportTest(BeetsTestCase): item.year = 2016 item.comp = False item.label_name = "Gravitas Recordings" - item.genre = "Glitch Hop" + item.genres = ["Glitch Hop"] item.year = 2016 item.month = 4 item.day = 11 @@ -583,7 +583,7 @@ class BeatportTest(BeetsTestCase): def test_genre_applied(self): for track, test_track in zip(self.tracks, self.test_tracks): - assert track.genres == [test_track.genre] + assert track.genres == test_track.genres class BeatportResponseEmptyTest(unittest.TestCase): diff --git a/test/plugins/test_ihate.py b/test/plugins/test_ihate.py index f941d566c..b64b8d91d 100644 --- a/test/plugins/test_ihate.py +++ b/test/plugins/test_ihate.py @@ -11,7 +11,7 @@ class IHatePluginTest(unittest.TestCase): def test_hate(self): match_pattern = {} test_item = Item( - genre="TestGenre", album="TestAlbum", artist="TestArtist" + genres=["TestGenre"], album="TestAlbum", artist="TestArtist" ) task = importer.SingletonImportTask(None, test_item) @@ -27,19 +27,19 @@ class IHatePluginTest(unittest.TestCase): assert IHatePlugin.do_i_hate_this(task, match_pattern) # Query is blocked by AND clause. - match_pattern = ["album:notthis genre:testgenre"] + match_pattern = ["album:notthis genres:testgenre"] assert not IHatePlugin.do_i_hate_this(task, match_pattern) # Both queries are blocked by AND clause with unmatched condition. match_pattern = [ - "album:notthis genre:testgenre", + "album:notthis genres:testgenre", "artist:testartist album:notthis", ] assert not IHatePlugin.do_i_hate_this(task, match_pattern) # Only one query should fire. match_pattern = [ - "album:testalbum genre:testgenre", + "album:testalbum genres:testgenre", "artist:testartist album:notthis", ] assert IHatePlugin.do_i_hate_this(task, match_pattern) diff --git a/test/plugins/test_musicbrainz.py b/test/plugins/test_musicbrainz.py index 4ebce1b01..e000e16ec 100644 --- a/test/plugins/test_musicbrainz.py +++ b/test/plugins/test_musicbrainz.py @@ -539,7 +539,7 @@ class MBAlbumInfoTest(MusicBrainzTestCase): config["musicbrainz"]["genres"] = False release = self._make_release() d = self.mb.album_info(release) - assert d.genre is None + assert d.genres == [] def test_ignored_media(self): config["match"]["ignored_media"] = ["IGNORED1", "IGNORED2"] diff --git a/test/plugins/test_smartplaylist.py b/test/plugins/test_smartplaylist.py index 7cc712330..d1125158f 100644 --- a/test/plugins/test_smartplaylist.py +++ b/test/plugins/test_smartplaylist.py @@ -76,11 +76,11 @@ class SmartPlaylistTest(BeetsTestCase): {"name": "one_non_empty_sort", "query": ["foo year+", "bar"]}, { "name": "multiple_sorts", - "query": ["foo year+", "bar genre-"], + "query": ["foo year+", "bar genres-"], }, { "name": "mixed", - "query": ["foo year+", "bar", "baz genre+ id-"], + "query": ["foo year+", "bar", "baz genres+ id-"], }, ] ) @@ -102,11 +102,11 @@ class SmartPlaylistTest(BeetsTestCase): # Multiple queries store individual sorts in the tuple assert all(isinstance(x, NullSort) for x in sorts["only_empty_sorts"]) assert sorts["one_non_empty_sort"] == [sort("year"), NullSort()] - assert sorts["multiple_sorts"] == [sort("year"), sort("genre", False)] + assert sorts["multiple_sorts"] == [sort("year"), sort("genres", False)] assert sorts["mixed"] == [ sort("year"), NullSort(), - MultipleSort([sort("genre"), sort("id", False)]), + MultipleSort([sort("genres"), sort("id", False)]), ] def test_matches(self): @@ -259,7 +259,7 @@ class SmartPlaylistTest(BeetsTestCase): type(i).title = PropertyMock(return_value="fake Title") type(i).length = PropertyMock(return_value=300.123) type(i).path = PropertyMock(return_value=b"/tagada.mp3") - a = {"id": 456, "genre": "Fake Genre"} + a = {"id": 456, "genres": ["Rock", "Pop"]} i.__getitem__.side_effect = a.__getitem__ i.evaluate_template.side_effect = lambda pl, _: pl.replace( b"$title", @@ -280,7 +280,7 @@ class SmartPlaylistTest(BeetsTestCase): config["smartplaylist"]["output"] = "extm3u" config["smartplaylist"]["relative_to"] = False config["smartplaylist"]["playlist_dir"] = str(dir) - config["smartplaylist"]["fields"] = ["id", "genre"] + config["smartplaylist"]["fields"] = ["id", "genres"] try: spl.update_playlists(lib) except Exception: @@ -297,7 +297,7 @@ class SmartPlaylistTest(BeetsTestCase): assert content == ( b"#EXTM3U\n" - b'#EXTINF:300 id="456" genre="Fake%20Genre",Fake Artist - fake Title\n' + b'#EXTINF:300 id="456" genres="Rock%3B%20Pop",Fake Artist - fake Title\n' b"/tagada.mp3\n" ) diff --git a/test/test_importer.py b/test/test_importer.py index 6ae7d562b..a560ca5af 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -310,7 +310,7 @@ class ImportSingletonTest(AutotagImportTestCase): config["import"]["set_fields"] = { "collection": collection, - "genre": genre, + "genres": genre, "title": "$title - formatted", "disc": disc, } @@ -322,7 +322,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() # TODO: Not sure this is necessary. - assert item.genre == genre + assert item.genres == [genre] assert item.collection == collection assert item.title == "Tag Track 1 - formatted" assert item.disc == disc @@ -337,7 +337,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() - assert item.genre == genre + assert item.genres == [genre] assert item.collection == collection assert item.title == "Applied Track 1 - formatted" assert item.disc == disc @@ -373,12 +373,12 @@ class ImportTest(PathsMixin, AutotagImportTestCase): config["import"]["from_scratch"] = True for mediafile in self.import_media: - mediafile.genre = "Tag Genre" + mediafile.genres = ["Tag Genre"] mediafile.save() self.importer.add_choice(importer.Action.APPLY) self.importer.run() - assert self.lib.items().get().genre == "" + assert not self.lib.items().get().genres def test_apply_from_scratch_keeps_format(self): config["import"]["from_scratch"] = True @@ -470,7 +470,7 @@ class ImportTest(PathsMixin, AutotagImportTestCase): disc = 0 config["import"]["set_fields"] = { - "genre": genre, + "genres": genre, "collection": collection, "comments": comments, "album": "$album - formatted", @@ -483,11 +483,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - album.load() # TODO: Not sure this is necessary. - assert album.genre == genre + assert album.genres == [genre] assert album.comments == comments for item in album.items(): - assert item.get("genre", with_album=False) == genre + assert item.get("genres", with_album=False) == [genre] assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( @@ -505,11 +504,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - album.load() - assert album.genre == genre + assert album.genres == [genre] assert album.comments == comments for item in album.items(): - assert item.get("genre", with_album=False) == genre + assert item.get("genres", with_album=False) == [genre] assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( diff --git a/test/test_library.py b/test/test_library.py index 4df4e4b58..de7ff693b 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -66,15 +66,17 @@ class StoreTest(ItemInDBTestCase): assert new_year == 1987 def test_store_only_writes_dirty_fields(self): - original_genre = self.i.genre - self.i._values_fixed["genre"] = "beatboxing" # change w/o dirtying + original_genres = self.i.genres + self.i._values_fixed["genres"] = ["beatboxing"] # change w/o dirtying self.i.store() new_genre = ( self.lib._connection() - .execute("select genre from items where title = ?", (self.i.title,)) - .fetchone()["genre"] + .execute( + "select genres from items where title = ?", (self.i.title,) + ) + .fetchone()["genres"] ) - assert new_genre == original_genre + assert [new_genre] == original_genres def test_store_clears_dirty_flags(self): self.i.composer = "tvp" diff --git a/test/test_query.py b/test/test_query.py index 0ddf83e3a..81532c436 100644 --- a/test/test_query.py +++ b/test/test_query.py @@ -71,7 +71,7 @@ class TestGet: album="baz", year=2001, comp=True, - genre="rock", + genres=["rock"], ), helper.create_item( title="second", @@ -80,7 +80,7 @@ class TestGet: album="baz", year=2002, comp=True, - genre="Rock", + genres=["Rock"], ), ] album = helper.lib.add_album(album_items) @@ -94,7 +94,7 @@ class TestGet: album="foo", year=2003, comp=False, - genre="Hard Rock", + genres=["Hard Rock"], comments="caf\xe9", ) @@ -125,12 +125,12 @@ class TestGet: ("comments:caf\xe9", ["third"]), ("comp:true", ["first", "second"]), ("comp:false", ["third"]), - ("genre:=rock", ["first"]), - ("genre:=Rock", ["second"]), - ('genre:="Hard Rock"', ["third"]), - ('genre:=~"hard rock"', ["third"]), - ("genre:=~rock", ["first", "second"]), - ('genre:="hard rock"', []), + ("genres:=rock", ["first"]), + ("genres:=Rock", ["second"]), + ('genres:="Hard Rock"', ["third"]), + ('genres:=~"hard rock"', ["third"]), + ("genres:=~rock", ["first", "second"]), + ('genres:="hard rock"', []), ("popebear", []), ("pope:bear", []), ("singleton:true", ["third"]), @@ -243,13 +243,7 @@ class TestGet: class TestMatch: @pytest.fixture(scope="class") def item(self): - return _common.item( - album="the album", - disc=6, - genre="the genre", - year=1, - bitrate=128000, - ) + return _common.item(album="the album", disc=6, year=1, bitrate=128000) @pytest.mark.parametrize( "q, should_match", @@ -260,9 +254,9 @@ class TestMatch: (SubstringQuery("album", "album"), True), (SubstringQuery("album", "ablum"), False), (SubstringQuery("disc", "6"), True), - (StringQuery("genre", "the genre"), True), - (StringQuery("genre", "THE GENRE"), True), - (StringQuery("genre", "genre"), False), + (StringQuery("album", "the album"), True), + (StringQuery("album", "THE ALBUM"), True), + (StringQuery("album", "album"), False), (NumericQuery("year", "1"), True), (NumericQuery("year", "10"), False), (NumericQuery("bitrate", "100000..200000"), True), diff --git a/test/test_sort.py b/test/test_sort.py index 460aa07b8..d7d651de5 100644 --- a/test/test_sort.py +++ b/test/test_sort.py @@ -33,7 +33,7 @@ class DummyDataTestCase(BeetsTestCase): albums = [ Album( album="Album A", - genre="Rock", + genres=["Rock"], year=2001, flex1="Flex1-1", flex2="Flex2-A", @@ -41,7 +41,7 @@ class DummyDataTestCase(BeetsTestCase): ), Album( album="Album B", - genre="Rock", + genres=["Rock"], year=2001, flex1="Flex1-2", flex2="Flex2-A", @@ -49,7 +49,7 @@ class DummyDataTestCase(BeetsTestCase): ), Album( album="Album C", - genre="Jazz", + genres=["Jazz"], year=2005, flex1="Flex1-1", flex2="Flex2-B", @@ -236,19 +236,19 @@ class SortAlbumFixedFieldTest(DummyDataTestCase): def test_sort_two_field_asc(self): q = "" - s1 = dbcore.query.FixedFieldSort("genre", True) + s1 = dbcore.query.FixedFieldSort("genres", True) s2 = dbcore.query.FixedFieldSort("album", True) sort = dbcore.query.MultipleSort() sort.add_sort(s1) sort.add_sort(s2) results = self.lib.albums(q, sort) - assert results[0]["genre"] <= results[1]["genre"] - assert results[1]["genre"] <= results[2]["genre"] - assert results[1]["genre"] == "Rock" - assert results[2]["genre"] == "Rock" + assert results[0]["genres"] <= results[1]["genres"] + assert results[1]["genres"] <= results[2]["genres"] + assert results[1]["genres"] == ["Rock"] + assert results[2]["genres"] == ["Rock"] assert results[1]["album"] <= results[2]["album"] # same thing with query string - q = "genre+ album+" + q = "genres+ album+" results2 = self.lib.albums(q) for r1, r2 in zip(results, results2): assert r1.id == r2.id @@ -388,7 +388,7 @@ class CaseSensitivityTest(DummyDataTestCase): album = Album( album="album", - genre="alternative", + genres=["alternative"], year="2001", flex1="flex1", flex2="flex2-A", diff --git a/test/ui/commands/test_list.py b/test/ui/commands/test_list.py index 372d75410..0828980ca 100644 --- a/test/ui/commands/test_list.py +++ b/test/ui/commands/test_list.py @@ -63,6 +63,6 @@ class ListTest(IOMixin, BeetsTestCase): assert "the artist - the album - 0001" == stdout.strip() def test_list_album_format(self): - stdout = self._run_list(album=True, fmt="$genre") + stdout = self._run_list(album=True, fmt="$genres") assert "the genre" in stdout assert "the album" not in stdout diff --git a/test/ui/commands/test_update.py b/test/ui/commands/test_update.py index 3fb687418..937ded10c 100644 --- a/test/ui/commands/test_update.py +++ b/test/ui/commands/test_update.py @@ -103,22 +103,22 @@ class UpdateTest(IOMixin, BeetsTestCase): def test_selective_modified_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) mf.title = "differentTitle" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=True, fields=["title"]) item = self.lib.items().get() assert b"differentTitle" in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_selective_modified_metadata_not_moved(self): mf = MediaFile(syspath(self.i.path)) mf.title = "differentTitle" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=False, fields=["title"]) item = self.lib.items().get() assert b"differentTitle" not in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_modified_album_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) @@ -141,22 +141,22 @@ class UpdateTest(IOMixin, BeetsTestCase): def test_selective_modified_album_metadata_moved(self): mf = MediaFile(syspath(self.i.path)) mf.album = "differentAlbum" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() self._update(move=True, fields=["album"]) item = self.lib.items().get() assert b"differentAlbum" in item.path - assert item.genre != "differentGenre" + assert item.genres != ["differentGenre"] def test_selective_modified_album_metadata_not_moved(self): mf = MediaFile(syspath(self.i.path)) mf.album = "differentAlbum" - mf.genre = "differentGenre" + mf.genres = ["differentGenre"] mf.save() - self._update(move=True, fields=["genre"]) + self._update(move=True, fields=["genres"]) item = self.lib.items().get() assert b"differentAlbum" not in item.path - assert item.genre == "differentGenre" + assert item.genres == ["differentGenre"] def test_mtime_match_skips_update(self): mf = MediaFile(syspath(self.i.path)) diff --git a/test/ui/test_field_diff.py b/test/ui/test_field_diff.py index d42e55a93..24bac0123 100644 --- a/test/ui/test_field_diff.py +++ b/test/ui/test_field_diff.py @@ -34,8 +34,8 @@ class TestFieldDiff: p({"title": "foo"}, {"title": "bar"}, "title", f"title: {diff_fmt('foo', 'bar')}", id="string_full_replace"), p({"title": "prefix foo"}, {"title": "prefix bar"}, "title", "title: prefix [text_diff_removed]foo[/] -> prefix [text_diff_added]bar[/]", id="string_partial_change"), p({"year": 2000}, {"year": 2001}, "year", f"year: {diff_fmt('2000', '2001')}", id="int_changed"), - p({}, {"genre": "Rock"}, "genre", "genre: -> [text_diff_added]Rock[/]", id="field_added"), - p({"genre": "Rock"}, {}, "genre", "genre: [text_diff_removed]Rock[/] -> ", id="field_removed"), + p({}, {"artist": "Artist"}, "artist", "artist: -> [text_diff_added]Artist[/]", id="field_added"), + p({"artist": "Artist"}, {}, "artist", "artist: [text_diff_removed]Artist[/] -> ", id="field_removed"), p({"track": 1}, {"track": 2}, "track", f"track: {diff_fmt('01', '02')}", id="formatted_value_changed"), p({"mb_trackid": None}, {"mb_trackid": "1234"}, "mb_trackid", "mb_trackid: -> [text_diff_added]1234[/]", id="none_to_value"), p({}, {"new_flex": "foo"}, "new_flex", "[text_diff_added]new_flex: foo[/]", id="flex_field_added"), From 52375472e83338bf7a0333fa2ca18fe1eed95265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 15 Feb 2026 13:17:26 +0000 Subject: [PATCH 46/60] Replace genre: with genres: in docs --- docs/plugins/fish.rst | 4 ++-- docs/plugins/ihate.rst | 8 ++++---- docs/plugins/zero.rst | 1 - 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/plugins/fish.rst b/docs/plugins/fish.rst index c1ae4f990..a26b06458 100644 --- a/docs/plugins/fish.rst +++ b/docs/plugins/fish.rst @@ -28,7 +28,7 @@ option flags available to you, which also applies to subcommands such as ``beet import -``. If you type ``beet ls`` followed by a space and then the and the ``TAB`` key, you will see a list of all the album/track fields that can be used in beets queries. For example, typing ``beet ls ge`` will complete to -``genre:`` and leave you ready to type the rest of your query. +``genres:`` and leave you ready to type the rest of your query. Options ------- @@ -42,7 +42,7 @@ commands and option flags. If you want generated completions to also contain album/track field *values* for the items in your library, you can use the ``-e`` or ``--extravalues`` option. For example: ``beet fish -e genre`` or ``beet fish -e genre -e albumartist`` In -the latter case, subsequently typing ``beet list genre: `` will display a +the latter case, subsequently typing ``beet list genres: `` will display a list of all the genres in your library and ``beet list albumartist: `` will show a list of the album artists in your library. Keep in mind that all of these values will be put into the generated completions file, so use this option with diff --git a/docs/plugins/ihate.rst b/docs/plugins/ihate.rst index 47e679dbd..6bb76d796 100644 --- a/docs/plugins/ihate.rst +++ b/docs/plugins/ihate.rst @@ -26,12 +26,12 @@ Here's an example: ihate: warn: - artist:rnb - - genre:soul + - genres:soul # Only warn about tribute albums in rock genre. - - genre:rock album:tribute + - genres:rock album:tribute skip: - - genre::russian\srock - - genre:polka + - genres::russian\srock + - genres:polka - artist:manowar - album:christmas diff --git a/docs/plugins/zero.rst b/docs/plugins/zero.rst index bf134e664..914e28faf 100644 --- a/docs/plugins/zero.rst +++ b/docs/plugins/zero.rst @@ -45,7 +45,6 @@ For example: zero: fields: month day genre genres comments comments: [EAC, LAME, from.+collection, 'ripped by'] - genre: [rnb, 'power metal'] genres: [rnb, 'power metal'] update_database: true From 6f886682eac28ed0046082c52a507df638e48014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 15 Feb 2026 13:38:53 +0000 Subject: [PATCH 47/60] Update changelog note --- docs/changelog.rst | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 290f63168..7d59a937a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,31 +14,36 @@ New features - Add native support for multiple genres per album/track. The ``genres`` field now stores genres as a list and is written to files as multiple individual - genre tags (e.g., separate GENRE tags for FLAC/MP3). The single ``genre`` - field is automatically synchronized to contain the first genre from the list - for backward compatibility. The :doc:`plugins/musicbrainz`, - :doc:`plugins/beatport`, and :doc:`plugins/lastgenre` plugins have been - updated to populate the ``genres`` field as a list. + genre tags (e.g., separate GENRE tags for FLAC/MP3). The + :doc:`plugins/musicbrainz`, :doc:`plugins/beatport`, :doc:`plugins/discogs` + and :doc:`plugins/lastgenre` plugins have been updated to populate the + ``genres`` field as a list. **Migration**: Existing libraries with comma-separated, semicolon-separated, or slash-separated genre strings (e.g., ``"Rock, Alternative, Indie"``) are automatically migrated to the ``genres`` list when you first run beets after upgrading. The migration runs once when the database schema is updated, - splitting genre strings and writing the changes to both the database and media - files. No manual action or ``mbsync`` is required. + splitting genre strings and writing the changes to the database. The updated + ``genres`` values will be written to media files the next time you run a + command that writes tags (such as ``beet write`` or during import). No manual + action or ``mbsync`` is required. .. Bug fixes ~~~~~~~~~ -.. - For plugin developers - ~~~~~~~~~~~~~~~~~~~~~ +For plugin developers +~~~~~~~~~~~~~~~~~~~~~ + +- If you maintain a metadata source plugin that populates the ``genre`` field, + please update it to populate a list of ``genres`` instead. You will see a + deprecation warning for now, but support for populating the single ``genre`` + field will be removed in version ``3.0.0``. Other changes ~~~~~~~~~~~~~ -- :ref:`modify-cmd`: Use the following separator to delimite multiple field +- :ref:`modify-cmd`: Use the following separator to delimit multiple field values: |semicolon_space|. For example ``beet modify albumtypes="album; ep"``. Previously, ``\␀`` was used as a separator. This applies to fields such as ``artists``, ``albumtypes`` etc. @@ -46,10 +51,10 @@ Other changes - :doc:`plugins/edit`: Editing multi-valued fields now behaves more naturally, with list values handled directly to make metadata edits smoother and more predictable. -- :doc:`plugins/lastgenre`: The ``separator`` configuration option is - deprecated. Genres are now stored as a list in the ``genres`` field and - written to files as individual genre tags. The separator option has no effect - and will be removed in a future version. +- :doc:`plugins/lastgenre`: The ``separator`` configuration option is removed. + Since genres are now stored as a list in the ``genres`` field and written to + files as individual genre tags, this option has no effect and has been + removed. 2.6.2 (February 22, 2026) ------------------------- From 67cf15b0bd7bb3ec1114c2a953af44adc34e613e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 15 Feb 2026 13:39:04 +0000 Subject: [PATCH 48/60] Remove lastgenre separator config --- beetsplug/lastgenre/__init__.py | 1 - docs/plugins/lastgenre.rst | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index bfa55ba90..41927a87c 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -111,7 +111,6 @@ class LastGenrePlugin(plugins.BeetsPlugin): "force": False, "keep_existing": False, "auto": True, - "separator": ", ", "prefer_specific": False, "title_case": True, "pretend": False, diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index b677b001e..fa68ce9db 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -191,16 +191,6 @@ file. The available options are: Default: ``no``. - **source**: Which entity to look up in Last.fm. Can be either ``artist``, ``album`` or ``track``. Default: ``album``. -- **separator**: - - .. deprecated:: 2.6 - - The ``separator`` option is deprecated. Genres are now stored as a list in - the ``genres`` field and written to files as individual genre tags. This - option has no effect and will be removed in a future version. - - Default: ``', '``. - - **whitelist**: The filename of a custom genre list, ``yes`` to use the internal whitelist, or ``no`` to consider all genres valid. Default: ``yes``. - **title_case**: Convert the new tags to TitleCase before saving. Default: From 62e232983aa714517392e05feaecd20cd550b129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 22 Feb 2026 12:42:19 +0000 Subject: [PATCH 49/60] Document ordering of the genre split separator --- beets/autotag/hooks.py | 2 +- beets/library/migrations.py | 2 +- docs/changelog.rst | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/beets/autotag/hooks.py b/beets/autotag/hooks.py index 617d5051a..63ee52267 100644 --- a/beets/autotag/hooks.py +++ b/beets/autotag/hooks.py @@ -87,7 +87,7 @@ class Info(AttrDict[Any]): ) if not genres: try: - sep = next(s for s in [", ", "; ", " / "] if s in genre) + sep = next(s for s in ["; ", ", ", " / "] if s in genre) except StopIteration: genres = [genre] else: diff --git a/beets/library/migrations.py b/beets/library/migrations.py index 16f4c6761..c061ddfc5 100644 --- a/beets/library/migrations.py +++ b/beets/library/migrations.py @@ -37,7 +37,7 @@ class MultiGenreFieldMigration(Migration): with suppress(ConfigError): separators.append(beets.config["lastgenre"]["separator"].as_str()) - separators.extend([", ", "; ", " / "]) + separators.extend(["; ", ", ", " / "]) return unique_list(filter(None, separators)) @contextmanager diff --git a/docs/changelog.rst b/docs/changelog.rst index 7d59a937a..6cd8d7623 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,14 @@ New features command that writes tags (such as ``beet write`` or during import). No manual action or ``mbsync`` is required. + The ``genre`` field is split by the first separator found in the string, in + the following order of precedence: + + 1. :doc:`plugins/lastgenre` ``separator`` configuration + 2. Semicolon followed by a space + 3. Comma followed by a space + 4. Slash wrapped by spaces + .. Bug fixes ~~~~~~~~~ From 2c63fe77ce07f1b3483df06d36a937934428326c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sun, 22 Feb 2026 16:22:03 +0000 Subject: [PATCH 50/60] Remove test case indices from test_lastgenre.py --- test/plugins/test_lastgenre.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/test/plugins/test_lastgenre.py b/test/plugins/test_lastgenre.py index 5dcf2c165..55524d3fc 100644 --- a/test/plugins/test_lastgenre.py +++ b/test/plugins/test_lastgenre.py @@ -206,7 +206,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): @pytest.mark.parametrize( "config_values, item_genre, mock_genres, expected_result", [ - # 0 - force and keep whitelisted + # force and keep whitelisted ( { "force": True, @@ -223,7 +223,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Blues", "Jazz"], "keep + album, whitelist"), ), - # 1 - force and keep whitelisted, unknown original + # force and keep whitelisted, unknown original ( { "force": True, @@ -239,7 +239,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Blues", "Jazz"], "keep + album, whitelist"), ), - # 2 - force and keep whitelisted on empty tag + # force and keep whitelisted on empty tag ( { "force": True, @@ -255,7 +255,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Jazz"], "album, whitelist"), ), - # 3 force and keep, artist configured + # force and keep, artist configured ( { "force": True, @@ -272,7 +272,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Blues", "Pop"], "keep + artist, whitelist"), ), - # 4 - don't force, disabled whitelist + # don't force, disabled whitelist ( { "force": False, @@ -288,7 +288,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["any genre"], "keep any, no-force"), ), - # 5 - don't force and empty is regular last.fm fetch; no whitelist too + # don't force and empty is regular last.fm fetch; no whitelist too ( { "force": False, @@ -304,7 +304,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Jazzin"], "album, any"), ), - # 6 - fallback to next stages until found + # fallback to next stages until found ( { "force": True, @@ -322,7 +322,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Unknown Genre", "Jazz"], "keep + artist, any"), ), - # 7 - Keep the original genre when force and keep_existing are on, and + # Keep the original genre when force and keep_existing are on, and # whitelist is disabled ( { @@ -342,7 +342,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["any existing"], "original fallback"), ), - # 7.1 - Keep the original genre when force and keep_existing are on, and + # Keep the original genre when force and keep_existing are on, and # whitelist is enabled, and genre is valid. ( { @@ -362,7 +362,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Jazz"], "original fallback"), ), - # 7.2 - Return the configured fallback when force is on but + # Return the configured fallback when force is on but # keep_existing is not. ( { @@ -382,7 +382,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["fallback genre"], "fallback"), ), - # 8 - fallback to fallback if no original + # fallback to fallback if no original ( { "force": True, @@ -401,7 +401,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["fallback genre"], "fallback"), ), - # 9 - limit a lot of results + # limit a lot of results ( { "force": True, @@ -421,7 +421,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "keep + album, whitelist", ), ), - # 10 - fallback to next stage (artist) if no allowed original present + # fallback to next stage (artist) if no allowed original present # and no album genre were fetched. ( { @@ -441,7 +441,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Jazz"], "keep + artist, whitelist"), ), - # 11 - canonicalization transforms non-whitelisted genres to canonical forms + # canonicalization transforms non-whitelisted genres to canonical forms # # "Acid Techno" is not in the default whitelist, thus gets resolved "up" in the # tree to "Techno" and "Electronic". @@ -461,7 +461,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): }, (["Techno", "Electronic"], "album, whitelist"), ), - # 12 - canonicalization transforms whitelisted genres to canonical forms and + # canonicalization transforms whitelisted genres to canonical forms and # includes originals # # "Detroit Techno" is in the default whitelist, thus it stays and and also gets @@ -493,7 +493,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "keep + album, whitelist", ), ), - # 13 - canonicalization transforms non-whitelisted original genres to canonical + # canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the @@ -518,7 +518,7 @@ class LastGenrePluginTest(IOMixin, PluginTestCase): "keep + album, whitelist", ), ), - # 16 - canonicalization transforms non-whitelisted original genres to canonical + # canonicalization transforms non-whitelisted original genres to canonical # forms and deduplication works, **even** when no new genres are found online. # # "Cosmic Disco" is not in the default whitelist, thus gets resolved "up" in the From 10d13992e66d9eeb238deafc2c0a4a2576e25522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 23 Feb 2026 04:51:41 +0000 Subject: [PATCH 51/60] Dedupe genres parsing in beatport --- beetsplug/beatport.py | 14 +++++--------- test/plugins/test_beatport.py | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/beetsplug/beatport.py b/beetsplug/beatport.py index 3368825b8..aa0693541 100644 --- a/beetsplug/beatport.py +++ b/beetsplug/beatport.py @@ -234,9 +234,11 @@ class BeatportObject: ) if "artists" in data: self.artists = [(x["id"], str(x["name"])) for x in data["artists"]] - if "genres" in data: - genre_list = [str(x["name"]) for x in data["genres"]] - self.genres = unique_list(genre_list) + + self.genres = unique_list( + x["name"] + for x in (*data.get("subGenres", []), *data.get("genres", [])) + ) def artists_str(self) -> str | None: if self.artists is not None: @@ -305,12 +307,6 @@ class BeatportTrack(BeatportObject): self.bpm = data.get("bpm") self.initial_key = str((data.get("key") or {}).get("shortName")) - # Extract genres list from subGenres or genres - self.genres = unique_list( - str(x.get("name")) - for x in data.get("subGenres") or data.get("genres") or [] - ) - class BeatportPlugin(MetadataSourcePlugin): _client: BeatportClient | None = None diff --git a/test/plugins/test_beatport.py b/test/plugins/test_beatport.py index 442f80037..96386d8b6 100644 --- a/test/plugins/test_beatport.py +++ b/test/plugins/test_beatport.py @@ -474,7 +474,7 @@ class BeatportTest(BeetsTestCase): item.year = 2016 item.comp = False item.label_name = "Gravitas Recordings" - item.genres = ["Glitch Hop"] + item.genres = ["Glitch Hop", "Breaks"] item.year = 2016 item.month = 4 item.day = 11 From a540a8174a57e3f719dfeda536c394a06b326dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 27 Feb 2026 17:52:50 +0000 Subject: [PATCH 52/60] Clarify tests --- test/test_importer.py | 26 +++++++++++++------------- test/test_library.py | 25 ++++++++----------------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index a560ca5af..a7d57dbb2 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -304,15 +304,15 @@ class ImportSingletonTest(AutotagImportTestCase): assert len(self.lib.albums()) == 2 def test_set_fields(self): - genre = "\U0001f3b7 Jazz" + genres = ["\U0001f3b7 Jazz", "Rock"] collection = "To Listen" disc = 0 config["import"]["set_fields"] = { + "genres": "; ".join(genres), "collection": collection, - "genres": genre, - "title": "$title - formatted", "disc": disc, + "title": "$title - formatted", } # As-is item import. @@ -322,7 +322,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() # TODO: Not sure this is necessary. - assert item.genres == [genre] + assert item.genres == genres assert item.collection == collection assert item.title == "Tag Track 1 - formatted" assert item.disc == disc @@ -337,7 +337,7 @@ class ImportSingletonTest(AutotagImportTestCase): for item in self.lib.items(): item.load() - assert item.genres == [genre] + assert item.genres == genres assert item.collection == collection assert item.title == "Applied Track 1 - formatted" assert item.disc == disc @@ -464,17 +464,17 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.lib.items().get().data_source def test_set_fields(self): - genre = "\U0001f3b7 Jazz" + genres = ["\U0001f3b7 Jazz", "Rock"] collection = "To Listen" - comments = "managed by beets" disc = 0 + comments = "managed by beets" config["import"]["set_fields"] = { - "genres": genre, + "genres": "; ".join(genres), "collection": collection, + "disc": disc, "comments": comments, "album": "$album - formatted", - "disc": disc, } # As-is album import. @@ -483,10 +483,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - assert album.genres == [genre] + assert album.genres == genres assert album.comments == comments for item in album.items(): - assert item.get("genres", with_album=False) == [genre] + assert item.get("genres", with_album=False) == genres assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( @@ -504,10 +504,10 @@ class ImportTest(PathsMixin, AutotagImportTestCase): self.importer.run() for album in self.lib.albums(): - assert album.genres == [genre] + assert album.genres == genres assert album.comments == comments for item in album.items(): - assert item.get("genres", with_album=False) == [genre] + assert item.get("genres", with_album=False) == genres assert item.get("collection", with_album=False) == collection assert item.get("comments", with_album=False) == comments assert ( diff --git a/test/test_library.py b/test/test_library.py index de7ff693b..5af6f76d8 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -56,27 +56,18 @@ class LoadTest(ItemInDBTestCase): class StoreTest(ItemInDBTestCase): def test_store_changes_database_value(self): - self.i.year = 1987 + new_year = 1987 + self.i.year = new_year self.i.store() - new_year = ( - self.lib._connection() - .execute("select year from items where title = ?", (self.i.title,)) - .fetchone()["year"] - ) - assert new_year == 1987 + + assert self.lib.get_item(self.i.id).year == new_year def test_store_only_writes_dirty_fields(self): - original_genres = self.i.genres - self.i._values_fixed["genres"] = ["beatboxing"] # change w/o dirtying + new_year = 1987 + self.i._values_fixed["year"] = new_year # change w/o dirtying self.i.store() - new_genre = ( - self.lib._connection() - .execute( - "select genres from items where title = ?", (self.i.title,) - ) - .fetchone()["genres"] - ) - assert [new_genre] == original_genres + + assert self.lib.get_item(self.i.id).year != new_year def test_store_clears_dirty_flags(self): self.i.composer = "tvp" From 085ff1267b3718a37b30d57b044d3e5fc004724c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 28 Feb 2026 10:06:31 +0000 Subject: [PATCH 53/60] Only add fields when we create new table --- beets/dbcore/db.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index cec6abc46..30f7ef7cf 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -1315,6 +1315,7 @@ class Database: for name, typ in fields.items(): columns.append(f"{name} {typ.sql}") setup_sql = f"CREATE TABLE {table} ({', '.join(columns)});\n" + self.db_tables[table]["columns"] = set(fields) else: # Table exists does not match the field set. setup_sql = "" @@ -1328,8 +1329,6 @@ class Database: with self.transaction() as tx: tx.script(setup_sql) - self.db_tables[table]["columns"] = set(fields) - def _make_attribute_table(self, flex_table: str): """Create a table and associated index for flexible attributes for the given entity (if they don't exist). From a40bd7ca3c21a174b4071aa8bbd017040c02a091 Mon Sep 17 00:00:00 2001 From: edvatar <88481784+toroleapinc@users.noreply.github.com> Date: Fri, 27 Feb 2026 11:40:44 -0500 Subject: [PATCH 54/60] docs: Document match.distance_weights in autotagger docs Add documentation for the distance_weights configuration option in the autotagger matching section. This includes all available fields with their default values and an example of how to customize them. Closes #6081 Signed-off-by: edvatar <88481784+toroleapinc@users.noreply.github.com> --- docs/reference/config.rst | 45 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index fc0de37a7..54a03042e 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -913,6 +913,51 @@ and the next-best match is above the *gap* threshold, the importer will suggest that match but not automatically confirm it. Otherwise, you'll see a list of options to choose from. +.. _distance-weights: + +distance_weights +~~~~~~~~~~~~~~~~ + +The ``distance_weights`` option allows you to customize how much each field +contributes to the overall distance score when matching albums and tracks. Higher +weights mean that differences in that field are penalized more heavily, making +them more important in the matching decision. + +The defaults are:: + + match: + distance_weights: + data_source: 2.0 + artist: 3.0 + album: 3.0 + media: 1.0 + mediums: 1.0 + year: 1.0 + country: 0.5 + label: 0.5 + catalognum: 0.5 + albumdisambig: 0.5 + album_id: 5.0 + tracks: 2.0 + missing_tracks: 0.9 + unmatched_tracks: 0.6 + track_title: 3.0 + track_artist: 2.0 + track_index: 1.0 + track_length: 2.0 + track_id: 5.0 + medium: 1.0 + +For example, if you don't care as much about matching the exact release year, +you can reduce its weight:: + + match: + distance_weights: + year: 0.1 + +You only need to specify the fields you want to override; unspecified fields +keep their default weights. + .. _max_rec: max_rec From dd1bda4bd0c8e2bbe64f2dcbffc63794ad0b70ec Mon Sep 17 00:00:00 2001 From: Serene-Arc <33189705+Serene-Arc@users.noreply.github.com> Date: Sun, 1 Mar 2026 18:01:18 +1000 Subject: [PATCH 55/60] Format docs --- docs/reference/config.rst | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 54a03042e..162b35770 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -919,11 +919,13 @@ distance_weights ~~~~~~~~~~~~~~~~ The ``distance_weights`` option allows you to customize how much each field -contributes to the overall distance score when matching albums and tracks. Higher -weights mean that differences in that field are penalized more heavily, making -them more important in the matching decision. +contributes to the overall distance score when matching albums and tracks. +Higher weights mean that differences in that field are penalized more heavily, +making them more important in the matching decision. -The defaults are:: +The defaults are: + +:: match: distance_weights: @@ -949,7 +951,9 @@ The defaults are:: medium: 1.0 For example, if you don't care as much about matching the exact release year, -you can reduce its weight:: +you can reduce its weight: + +:: match: distance_weights: From aa8123233644c1dd0ecb28b6249f21fe4b7b533c Mon Sep 17 00:00:00 2001 From: Serene <33189705+Serene-Arc@users.noreply.github.com> Date: Sun, 1 Mar 2026 21:10:49 +1000 Subject: [PATCH 56/60] Use proper syntax highlighting for code block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šarūnas Nejus --- docs/reference/config.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 162b35770..4326a80a1 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -925,7 +925,7 @@ making them more important in the matching decision. The defaults are: -:: +.. code-block:: yaml match: distance_weights: @@ -953,7 +953,7 @@ The defaults are: For example, if you don't care as much about matching the exact release year, you can reduce its weight: -:: +.. code-block:: yaml match: distance_weights: From 532b0dabb15bcba1a9771cc98523d604dec87b47 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Wed, 25 Feb 2026 06:49:48 +0100 Subject: [PATCH 57/60] Update my teams page entry --- docs/team.rst | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/team.rst b/docs/team.rst index 7ea12867a..f99fcae97 100644 --- a/docs/team.rst +++ b/docs/team.rst @@ -42,12 +42,11 @@ of what you can expect from these *knowledge owners*. ----- - The Discogs plugin -- Other metadata source plugins -- Generalization of source plugin logic (The MetaDataSourcePlugin abstract - class) - Good documentation throughout the project - The smartplaylist plugin -- Things around m3u and other playlist formats +- The lastgenre plugin +- Support for M3U and other playlist formats +- beets as a DJ companion tool (BPM, audio features, key) @RollingStar ------------ From df7db24871308ffea27e0960ed8c212617e6914b Mon Sep 17 00:00:00 2001 From: Dmitri Vassilenko Date: Mon, 16 Feb 2026 20:33:49 -0500 Subject: [PATCH 58/60] Fix symlink tests for macOS --- test/test_files.py | 2 +- test/test_importer.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_files.py b/test/test_files.py index d0d93987c..4ed2f8608 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -165,7 +165,7 @@ class MoveTest(BeetsTestCase): self.i.move(operation=MoveOperation.LINK) assert self.dest.exists() assert os.path.islink(syspath(self.dest)) - assert self.dest.resolve() == self.path + assert self.dest.resolve() == self.path.resolve() @unittest.skipUnless(_common.HAVE_SYMLINK, "need symlinks") def test_link_does_not_depart(self): diff --git a/test/test_importer.py b/test/test_importer.py index a7d57dbb2..56327af06 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -131,7 +131,7 @@ class NonAutotaggedImportTest(PathsMixin, AsIsImporterMixin, ImportTestCase): assert self.track_lib_path.exists() assert self.track_lib_path.is_symlink() - assert self.track_lib_path.resolve() == self.track_import_path + assert self.track_lib_path.resolve() == self.track_import_path.resolve() @unittest.skipUnless(_common.HAVE_HARDLINK, "need hardlinks") def test_import_hardlink_arrives(self): From bfd95f47d0a135800a7433e22004422c99588d1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20M=C3=B6llerstrand?= Date: Tue, 17 Feb 2026 08:10:58 +0100 Subject: [PATCH 59/60] fix: ftintitle can handle a list of ampersanded artists This was inspired by real life events: https://musicbrainz.org/release/7c4d7a15-6b30-4bef-8b20-af200186fbdb by the artist Danny L Harle has a a track with a featuring list that contains "Danny L Harle, Oklou & MNEK". --- beetsplug/ftintitle.py | 6 ++++++ docs/changelog.rst | 2 ++ test/plugins/test_ftintitle.py | 5 +++++ 3 files changed, 13 insertions(+) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index fde7ff92a..362bab523 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -71,6 +71,12 @@ def split_on_feat( if len(parts) == 2: return parts + # Try comma as separator + # (e.g. "Alice, Bob & Charlie" where Bob and Charlie are featuring) + if for_artist and "," in artist: + comma_parts = artist.split(",", 1) + return comma_parts[0].strip(), comma_parts[1].strip() + # Fall back to all tokens including generic separators if no explicit match if for_artist: regex = re.compile( diff --git a/docs/changelog.rst b/docs/changelog.rst index 6cd8d7623..b8812a706 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -86,6 +86,8 @@ Bug fixes backends, such as ``mp3gain``. :bug:`2946` - :doc:`plugins/mbpseudo`: Fix crash due to missing ``artist_credit`` field in the MusicBrainz API response. :bug:`6339` +- :doc:`plugins/ftintitle`: Fix handling of multiple featured artists with + ampersand. .. For plugin developers diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 560f44402..2bba41ad0 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -324,6 +324,11 @@ def test_find_feat_part( ("Alice feat. Bob", ("Alice", "Bob")), ("Alice featuring Bob", ("Alice", "Bob")), ("Alice & Bob", ("Alice", "Bob")), + ("Alice, Bob & Charlie", ("Alice", "Bob & Charlie")), + ( + "Alice, Bob & Charlie feat. Xavier", + ("Alice, Bob & Charlie", "Xavier"), + ), ("Alice and Bob", ("Alice", "Bob")), ("Alice With Bob", ("Alice", "Bob")), ("Alice defeat Bob", ("Alice defeat Bob", None)), From b17305aa1e312605df047812bcffb7145f6215d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Mon, 2 Mar 2026 14:54:34 +0000 Subject: [PATCH 60/60] Update changelog note --- docs/changelog.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b8812a706..8bbe8f4ea 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -36,9 +36,11 @@ New features 3. Comma followed by a space 4. Slash wrapped by spaces -.. - Bug fixes - ~~~~~~~~~ +Bug fixes +~~~~~~~~~ + +- :doc:`plugins/ftintitle`: Fix handling of multiple featured artists with + ampersand. For plugin developers ~~~~~~~~~~~~~~~~~~~~~ @@ -86,8 +88,6 @@ Bug fixes backends, such as ``mp3gain``. :bug:`2946` - :doc:`plugins/mbpseudo`: Fix crash due to missing ``artist_credit`` field in the MusicBrainz API response. :bug:`6339` -- :doc:`plugins/ftintitle`: Fix handling of multiple featured artists with - ampersand. .. For plugin developers