From 1b94baca63f67578b2b869cf103aa08857542bf1 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Tue, 4 Feb 2025 21:16:06 +0530 Subject: [PATCH 01/40] Handle potential OSError when unlinking temporary files in ArtResizer --- beets/util/artresizer.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 09cc29e0d..898ffeab8 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -655,7 +655,10 @@ class ArtResizer(metaclass=Shareable): ) finally: if result_path != path_in: - os.unlink(path_in) + try: + os.unlink(path_in) + except OSError: + pass return result_path @property From 01d61c722bf12cb8895cd403b0b50eb114f9209a Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Wed, 12 Mar 2025 15:07:14 +0530 Subject: [PATCH 02/40] add changelog --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 62cd0c4cc..ec7861f98 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -70,6 +70,8 @@ Bug fixes: * :doc:`plugins/lyrics`: Fix plugin crash when ``genius`` backend returns empty lyrics. :bug:`5583` +* Handle potential OSError when unlinking temporary files in ArtResizer. + :bug:`5615` For packagers: From f51559e16fef413f34b1e591c88512ef224c9cc1 Mon Sep 17 00:00:00 2001 From: rdy2go <47011689+rdy2go@users.noreply.github.com> Date: Sat, 21 Jun 2025 00:01:01 +0200 Subject: [PATCH 03/40] Fix 'from_scratch': delete all tags before writing new tags to file ## Github Issues Fixes #3706 Related #5165 ## Issue Comment tags are written to file even if option 'from_scratch' is used. The same tags are not written to the file if imported together with other files as album. Therefore 'from_scratch' is not working as described in the documentation. ## Solution 1. Add test: Adapt the function from the 'regular' import class and insert it in the class for the singleton import test. 2. Fix bug : Add check for 'from_scratch' option. If used, clear metadata before applying 'new' metadata with autotag. 3. No documentation change needed. Option now works as described in the documentation. 4. Add changelog. --- beets/importer/tasks.py | 2 ++ docs/changelog.rst | 3 +++ test/test_importer.py | 11 +++++++++++ 3 files changed, 16 insertions(+) diff --git a/beets/importer/tasks.py b/beets/importer/tasks.py index 75f04cf5a..4aa1f8a62 100644 --- a/beets/importer/tasks.py +++ b/beets/importer/tasks.py @@ -690,6 +690,8 @@ class SingletonImportTask(ImportTask): return [self.item] def apply_metadata(self): + if config["import"]["from_scratch"]: + self.item.clear() autotag.apply_item_metadata(self.item, self.match.info) def _emit_imported(self, lib): diff --git a/docs/changelog.rst b/docs/changelog.rst index 88b82e4da..bf830bade 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -39,6 +39,9 @@ Bug fixes: :bug:`5797` * :doc:`plugins/musicbrainz`: Fix the MusicBrainz search not taking into account the album/recording aliases +* :doc:`reference/cli`: Fix 'from_scratch' option for singleton imports: delete + all (old) metadata when new metadata is applied. + :bug:`3706` For packagers: diff --git a/test/test_importer.py b/test/test_importer.py index 9bb0e8a63..2fa5b32d3 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -315,6 +315,17 @@ class ImportSingletonTest(AutotagImportTestCase): self.importer.run() self.assert_file_in_lib(b"singletons", b"Applied Track 1.mp3") + def test_apply_from_scratch_removes_other_metadata(self): + config["import"]["from_scratch"] = True + + for mediafile in self.import_media: + mediafile.comments = "Tag Comment" + mediafile.save() + + self.importer.add_choice(importer.Action.APPLY) + self.importer.run() + assert self.lib.items().get().comments == "" + def test_skip_does_not_add_first_track(self): self.importer.add_choice(importer.Action.SKIP) self.importer.run() From 2c240b4788096c2c41b6a662ff59f05186206f47 Mon Sep 17 00:00:00 2001 From: rdy2go <47011689+rdy2go@users.noreply.github.com> Date: Sat, 2 Aug 2025 21:55:30 +0200 Subject: [PATCH 04/40] fix indentation --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 49be459dd..04dbb1cd9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -52,7 +52,7 @@ Bug fixes: * :doc:`reference/cli`: Fix 'from_scratch' option for singleton imports: delete all (old) metadata when new metadata is applied. :bug:`3706` - * :doc:`/plugins/spotify`: Fix the issue with that every query to spotify was +* :doc:`/plugins/spotify`: Fix the issue with that every query to spotify was ascii encoded. This resulted in bad matches for queries that contained special e.g. non latin characters as 盗作. If you want to keep the legacy behavior set the config option ``spotify.search_query_ascii: yes``. From dd824e69b2138af611efac8e9a34d772ffb01921 Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Mon, 10 Nov 2025 19:13:25 +0100 Subject: [PATCH 05/40] Clearart: Do not update files without an embedded image --- beetsplug/_utils/art.py | 7 +++++-- beetsplug/embedart.py | 1 + test/plugins/test_embedart.py | 13 +++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/beetsplug/_utils/art.py b/beetsplug/_utils/art.py index 656c303ce..264802ba5 100644 --- a/beetsplug/_utils/art.py +++ b/beetsplug/_utils/art.py @@ -210,5 +210,8 @@ def clear(log, lib, query): items = lib.items(query) log.info("Clearing album art from {} items", len(items)) for item in items: - log.debug("Clearing art for {}", item) - item.try_write(tags={"images": None}) + if mediafile.MediaFile(syspath(item.path)).images: + log.debug("Clearing art for {}", item) + item.try_write(tags={"images": None}) + else: + log.debug("No art to clean for {}", item) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index cbf40f570..ab02f13b5 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -17,6 +17,7 @@ import os.path import tempfile from mimetypes import guess_extension +from unittest import mock import requests diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index d40025374..3b07a6a72 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -17,6 +17,7 @@ import os import os.path import shutil import tempfile +import time import unittest from unittest.mock import MagicMock, patch @@ -225,10 +226,22 @@ class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): item = album.items()[0] self.io.addinput("y") self.run_command("embedart", "-f", self.small_artpath) + embedded_time = item.current_mtime() + time.sleep(1) + self.io.addinput("y") self.run_command("clearart") mediafile = MediaFile(syspath(item.path)) assert not mediafile.images + clear_time = item.current_mtime() + assert clear_time > embedded_time + time.sleep(1) + + # A run on a file without an image should not be modified + self.io.addinput("y") + self.run_command("clearart") + no_clear_time = item.current_mtime() + assert no_clear_time == clear_time def test_clear_art_with_no_input(self): self._setup_data() From 85168ba7fc0342dd3da551e3f8d3139b1c88e809 Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Mon, 10 Nov 2025 19:20:42 +0100 Subject: [PATCH 06/40] Remove wrong import --- beetsplug/embedart.py | 1 - 1 file changed, 1 deletion(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index ab02f13b5..cbf40f570 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -17,7 +17,6 @@ import os.path import tempfile from mimetypes import guess_extension -from unittest import mock import requests From 8889c4ab47f3bbe4f9fe1b4051bd13a88bbae22d Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Mon, 10 Nov 2025 22:38:37 +0100 Subject: [PATCH 07/40] Clear art on import --- beets/test/helper.py | 6 ++++-- beetsplug/_utils/art.py | 14 +++++++++----- beetsplug/embedart.py | 10 ++++++++++ test/plugins/test_embedart.py | 31 ++++++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 8 deletions(-) diff --git a/beets/test/helper.py b/beets/test/helper.py index 3cb1e4c3c..20ba4f4ab 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -364,15 +364,17 @@ class TestHelper(ConfigMixin): items.append(item) return self.lib.add_album(items) - def create_mediafile_fixture(self, ext="mp3", images=[]): + def create_mediafile_fixture(self, ext="mp3", images=[], target_dir=None): """Copy a fixture mediafile with the extension to `temp_dir`. `images` is a subset of 'png', 'jpg', and 'tiff'. For each specified extension a cover art image is added to the media file. """ + if not target_dir: + target_dir = self.temp_dir src = os.path.join(_common.RSRC, util.bytestring_path(f"full.{ext}")) - handle, path = mkstemp(dir=self.temp_dir) + handle, path = mkstemp(dir=target_dir) path = bytestring_path(path) os.close(handle) shutil.copyfile(syspath(src), syspath(path)) diff --git a/beetsplug/_utils/art.py b/beetsplug/_utils/art.py index 264802ba5..b11b30b95 100644 --- a/beetsplug/_utils/art.py +++ b/beetsplug/_utils/art.py @@ -206,12 +206,16 @@ def extract_first(log, outpath, items): return real_path +def clear_item(item, log): + if mediafile.MediaFile(syspath(item.path)).images: + log.debug("Clearing art for {}", item) + item.try_write(tags={"images": None}) + else: + log.debug("No art to clean for {}", item) + + def clear(log, lib, query): items = lib.items(query) log.info("Clearing album art from {} items", len(items)) for item in items: - if mediafile.MediaFile(syspath(item.path)).images: - log.debug("Clearing art for {}", item) - item.try_write(tags={"images": None}) - else: - log.debug("No art to clean for {}", item) + clear_item(item, log) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index cbf40f570..08e63836c 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -62,6 +62,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): "ifempty": False, "remove_art_file": False, "quality": 0, + "clearart_on_import": False, } ) @@ -82,6 +83,9 @@ class EmbedCoverArtPlugin(BeetsPlugin): self.register_listener("art_set", self.process_album) + if self.config["clearart_on_import"].get(bool): + self.register_listener("import_task_files", self.import_task_files) + def commands(self): # Embed command. embed_cmd = ui.Subcommand( @@ -278,3 +282,9 @@ class EmbedCoverArtPlugin(BeetsPlugin): os.remove(syspath(album.artpath)) album.artpath = None album.store() + + def import_task_files(self, session, task): + """Automatically clearart of imported files.""" + for item in task.imported_items(): + self._log.debug("clearart-on-import {.filepath}", item) + art.clear_item(item, self._log) diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 3b07a6a72..ae66fbc6f 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -27,6 +27,7 @@ from mediafile import MediaFile from beets import config, logging, ui from beets.test import _common from beets.test.helper import ( + ImportHelper, BeetsTestCase, FetchImageHelper, IOMixin, @@ -76,7 +77,9 @@ def require_artresizer_compare(test): return wrapper -class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): +class EmbedartCliTest( + ImportHelper, IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase +): plugin = "embedart" small_artpath = os.path.join(_common.RSRC, b"image-2x3.jpg") abbey_artpath = os.path.join(_common.RSRC, b"abbey.jpg") @@ -286,6 +289,32 @@ class EmbedartCliTest(IOMixin, PluginMixin, FetchImageHelper, BeetsTestCase): mediafile = MediaFile(syspath(item.path)) assert not mediafile.images + def test_clearart_on_import_disabled(self): + file_path = self.create_mediafile_fixture( + images=["jpg"], target_dir=self.import_path + ) + self.import_media.append(file_path) + with self.configure_plugin({"clearart_on_import": False}): + importer = self.setup_importer(autotag=False, write=True) + importer.run() + + item = self.lib.items()[0] + assert MediaFile(os.path.join(item.path)).images + + def test_clearart_on_import_enabled(self): + file_path = self.create_mediafile_fixture( + images=["jpg"], target_dir=self.import_path + ) + self.import_media.append(file_path) + # Force re-init the plugin to register the listener + self.unload_plugins() + with self.configure_plugin({"clearart_on_import": True}): + importer = self.setup_importer(autotag=False, write=True) + importer.run() + + item = self.lib.items()[0] + assert not MediaFile(os.path.join(item.path)).images + class DummyArtResizer(ArtResizer): """An `ArtResizer` which pretends that ImageMagick is available, and has From 95f21b6e429ecbf68f5a399cf6498256aa3f544d Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Mon, 10 Nov 2025 22:38:58 +0100 Subject: [PATCH 08/40] Remove log if no art to clear --- beetsplug/_utils/art.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/beetsplug/_utils/art.py b/beetsplug/_utils/art.py index b11b30b95..fce650c5b 100644 --- a/beetsplug/_utils/art.py +++ b/beetsplug/_utils/art.py @@ -210,8 +210,6 @@ def clear_item(item, log): if mediafile.MediaFile(syspath(item.path)).images: log.debug("Clearing art for {}", item) item.try_write(tags={"images": None}) - else: - log.debug("No art to clean for {}", item) def clear(log, lib, query): From d11c074a859b6bcef05cad8a3947bd09651608ec Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Mon, 10 Nov 2025 23:44:02 +0100 Subject: [PATCH 09/40] Improve test to not sleep --- test/plugins/test_embedart.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index ae66fbc6f..2b6f59e26 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -17,7 +17,6 @@ import os import os.path import shutil import tempfile -import time import unittest from unittest.mock import MagicMock, patch @@ -229,21 +228,19 @@ class EmbedartCliTest( item = album.items()[0] self.io.addinput("y") self.run_command("embedart", "-f", self.small_artpath) - embedded_time = item.current_mtime() - time.sleep(1) + embedded_time = os.path.getmtime(syspath(item.path)) self.io.addinput("y") self.run_command("clearart") mediafile = MediaFile(syspath(item.path)) assert not mediafile.images - clear_time = item.current_mtime() + clear_time = os.path.getmtime(syspath(item.path)) assert clear_time > embedded_time - time.sleep(1) # A run on a file without an image should not be modified self.io.addinput("y") self.run_command("clearart") - no_clear_time = item.current_mtime() + no_clear_time = os.path.getmtime(syspath(item.path)) assert no_clear_time == clear_time def test_clear_art_with_no_input(self): From 16c4f6e4331099ffa05d5fc9bbf4f15456bba611 Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Thu, 20 Nov 2025 18:48:37 +0100 Subject: [PATCH 10/40] Fix lint --- test/plugins/test_embedart.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/test_embedart.py b/test/plugins/test_embedart.py index 2b6f59e26..a7038b152 100644 --- a/test/plugins/test_embedart.py +++ b/test/plugins/test_embedart.py @@ -26,9 +26,9 @@ from mediafile import MediaFile from beets import config, logging, ui from beets.test import _common from beets.test.helper import ( - ImportHelper, BeetsTestCase, FetchImageHelper, + ImportHelper, IOMixin, PluginMixin, ) From 74a10266f01eb261adfdc9f3e4bec36e0afce760 Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Tue, 6 Jan 2026 22:43:08 +0100 Subject: [PATCH 11/40] Add documentation and changelog --- docs/changelog.rst | 4 ++++ docs/plugins/embedart.rst | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index b9a5c1f3f..1c1d6a2b6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -31,6 +31,10 @@ New features: no_convert, never_convert_lossy_files, same format, and max_bitrate - :doc:`plugins/titlecase`: Add the `titlecase` plugin to allow users to resolve differences in metadata source styles. +- :doc:`plugins/embedart`: Embedded arts can now be cleared during import with the + ``clearart_on_import`` config option. Also, ``beet clearart`` is only going to + update the files matching the query and with an embedded art, leaving + untouched the files without. Bug fixes: diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index abbe2460d..b8de07cc7 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -70,6 +70,8 @@ file. The available options are: :doc:`FetchArt ` plugin to download art with the purpose of directly embedding it into the file's metadata without an "intermediate" album art file. Default: ``no``. +- **clearart_on_import**: Enable automatic embedded art clearing. Default: ``no``. + Note: ``compare_threshold`` option requires ImageMagick_, and ``maxwidth`` requires either ImageMagick_ or Pillow_. @@ -110,4 +112,7 @@ embedded album art: automatically. - ``beet clearart QUERY``: removes all embedded images from all items matching the query. The command prompts for confirmation before making the change - unless you specify the ``-y`` (``--yes``) option. + unless you specify the ``-y`` (``--yes``) option. The files listed for + confirmation are the ones matching the query independently of having an + embedded art. However, only the files with an embedded art are updated, + leaving untouched the files without. From fe8dc1e65ee3fc45870a8d0cce004f034eb9cb51 Mon Sep 17 00:00:00 2001 From: Eric Masseran Date: Wed, 7 Jan 2026 20:47:04 +0100 Subject: [PATCH 12/40] Align lint --- docs/changelog.rst | 6 +++--- docs/plugins/embedart.rst | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index dd495de48..9c79a85bc 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -44,9 +44,9 @@ New features: of brackets are supported and a new ``bracket_keywords`` configuration option allows customizing the keywords. Setting ``bracket_keywords`` to an empty list matches any bracket content regardless of keywords. -- :doc:`plugins/embedart`: Embedded arts can now be cleared during import with the - ``clearart_on_import`` config option. Also, ``beet clearart`` is only going to - update the files matching the query and with an embedded art, leaving +- :doc:`plugins/embedart`: Embedded arts can now be cleared during import with + the ``clearart_on_import`` config option. Also, ``beet clearart`` is only + going to update the files matching the query and with an embedded art, leaving untouched the files without. Bug fixes: diff --git a/docs/plugins/embedart.rst b/docs/plugins/embedart.rst index b8de07cc7..0140ceff8 100644 --- a/docs/plugins/embedart.rst +++ b/docs/plugins/embedart.rst @@ -70,8 +70,8 @@ file. The available options are: :doc:`FetchArt ` plugin to download art with the purpose of directly embedding it into the file's metadata without an "intermediate" album art file. Default: ``no``. -- **clearart_on_import**: Enable automatic embedded art clearing. Default: ``no``. - +- **clearart_on_import**: Enable automatic embedded art clearing. Default: + ``no``. Note: ``compare_threshold`` option requires ImageMagick_, and ``maxwidth`` requires either ImageMagick_ or Pillow_. From ebd0e70012f7e7d55e6fd9cbb564b9e4a5fdab1a Mon Sep 17 00:00:00 2001 From: m_igashi <@M_Igashi> Date: Wed, 14 Jan 2026 01:37:55 +0100 Subject: [PATCH 13/40] Add mp3rgain support to ReplayGain command backend mp3rgain is a modern Rust rewrite of mp3gain that provides: - CLI-compatible drop-in replacement for mp3gain - Support for both MP3 and AAC/M4A formats (like aacgain) - Fixes for CVE-2021-34085 (Critical, CVSS 9.8) and CVE-2019-18359 (Medium) - Memory-safe implementation in Rust - Works on modern systems (Windows 11, macOS Apple Silicon) Changes: - Add mp3rgain to the command search list (prioritized first) - Update format_supported() with more robust command name detection using os.path.basename() and startswith() instead of substring matching - Update documentation with installation instructions See: https://github.com/M-Igashi/mp3rgain --- beetsplug/replaygain.py | 20 ++++++++++++++------ docs/plugins/replaygain.rst | 32 +++++++++++++++++++++----------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 4e8b429ea..af5dcd001 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -565,7 +565,7 @@ class CommandBackend(Backend): ) else: # Check whether the program is in $PATH. - for cmd in ("mp3gain", "aacgain"): + for cmd in ("mp3rgain", "mp3gain", "aacgain"): try: call([cmd, "-v"], self._log) self.command = cmd @@ -573,7 +573,7 @@ class CommandBackend(Backend): pass if not self.command: raise FatalReplayGainError( - "no replaygain command found: install mp3gain or aacgain" + "no replaygain command found: install mp3rgain, mp3gain, or aacgain" ) self.noclip = config["noclip"].get(bool) @@ -608,10 +608,18 @@ class CommandBackend(Backend): def format_supported(self, item: Item) -> bool: """Checks whether the given item is supported by the selected tool.""" - if "mp3gain" in self.command and item.format != "MP3": - return False - elif "aacgain" in self.command and item.format not in ("MP3", "AAC"): - return False + # Get the base name of the command for comparison + cmd_name = os.path.basename(self.command).lower() + + if cmd_name.startswith("mp3rgain"): + # mp3rgain supports MP3 and AAC/M4A formats + return item.format in ("MP3", "AAC") + elif cmd_name.startswith("aacgain"): + # aacgain supports MP3 and AAC formats + return item.format in ("MP3", "AAC") + elif cmd_name.startswith("mp3gain"): + # mp3gain only supports MP3 + return item.format == "MP3" return True def compute_gain( diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index c7e51d25d..16f4e3088 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -10,9 +10,9 @@ Installation ------------ This plugin can use one of many backends to compute the ReplayGain values: -GStreamer, mp3gain (and its cousin, aacgain), Python Audio Tools or ffmpeg. -ffmpeg and mp3gain can be easier to install. mp3gain supports less audio formats -than the other backend. +GStreamer, mp3gain (and its cousins, aacgain and mp3rgain), Python Audio Tools +or ffmpeg. ffmpeg and mp3gain can be easier to install. mp3gain supports fewer +audio formats than the other backends. Once installed, this plugin analyzes all files during the import process. This can be a slow process; to instead analyze after the fact, disable automatic @@ -51,16 +51,24 @@ configuration file: The GStreamer backend does not support parallel analysis. -mp3gain and aacgain -~~~~~~~~~~~~~~~~~~~ +mp3gain, aacgain, and mp3rgain +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In order to use this backend, you will need to install the mp3gain_ command-line -tool or the aacgain_ fork thereof. Here are some hints: +tool, the aacgain_ fork, or mp3rgain_. Here are some hints: -- On Mac OS X, you can use Homebrew_. Type ``brew install aacgain``. +- On Mac OS X, you can use Homebrew_. Type ``brew install aacgain`` or + ``brew install mp3rgain``. - On Linux, mp3gain_ is probably in your repositories. On Debian or Ubuntu, for - example, you can run ``apt-get install mp3gain``. -- On Windows, download and install the original mp3gain_. + example, you can run ``apt-get install mp3gain``. Alternatively, mp3rgain is + available via Nix (``nix-env -iA nixpkgs.mp3rgain``) or AUR for Arch Linux. +- On Windows, download and install mp3rgain_ (recommended) or the original + mp3gain_. + +mp3rgain_ is a modern Rust rewrite of mp3gain that also supports AAC/M4A files. +It addresses security vulnerabilities (CVE-2021-34085, CVE-2019-18359) present +in the original mp3gain and works on modern systems including Windows 11 and +macOS with Apple Silicon. .. _aacgain: https://aacgain.altosdesign.com @@ -68,6 +76,8 @@ tool or the aacgain_ fork thereof. Here are some hints: .. _mp3gain: http://mp3gain.sourceforge.net/download.php +.. _mp3rgain: https://github.com/M-Igashi/mp3rgain + Then, enable the plugin (see :ref:`using-plugins`) and specify the "command" backend in your configuration file: @@ -144,8 +154,8 @@ file. The available options are: These options only work with the "command" backend: -- **command**: The path to the ``mp3gain`` or ``aacgain`` executable (if beets - cannot find it by itself). For example: +- **command**: The path to the ``mp3rgain``, ``mp3gain``, or ``aacgain`` + executable (if beets cannot find it by itself). For example: ``/Applications/MacMP3Gain.app/Contents/Resources/aacgain``. Default: Search in your ``$PATH``. - **noclip**: Reduce the amount of ReplayGain adjustment to whatever amount From fdfeb3507692ab15d6fd4299171e6e1c8191c185 Mon Sep 17 00:00:00 2001 From: rdy2go <47011689+rdy2go@users.noreply.github.com> Date: Thu, 15 Jan 2026 16:07:54 +0100 Subject: [PATCH 14/40] add changelog for and to resolve PR #5828 --- docs/changelog.rst | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9f30ffd9a..84bb0cc02 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -90,6 +90,8 @@ Bug fixes: - :doc:`/plugins/ftintitle`: Fixed artist name splitting to prioritize explicit featuring tokens (feat, ft, featuring) over generic separators (&, and), preventing incorrect splits when both are present. +- :doc:`reference/cli`: Fix 'from_scratch' option for singleton imports: delete + all (old) metadata when new metadata is applied. :bug:`3706` For plugin developers: @@ -292,11 +294,8 @@ Bug fixes: - Fix ``HiddenFileTest`` by using ``bytestring_path()``. - tests: Fix tests failing without ``langdetect`` (by making it required). :bug:`5797` -- :doc:`plugins/musicbrainz`: Fix the MusicBrainz search not taking into - account the album/recording aliases -- :doc:`reference/cli`: Fix 'from_scratch' option for singleton imports: delete - all (old) metadata when new metadata is applied. - :bug:`3706` +- :doc:`plugins/musicbrainz`: Fix the MusicBrainz search not taking into account + the album/recording aliases - :doc:`/plugins/spotify`: Fix the issue with that every query to spotify was ascii encoded. This resulted in bad matches for queries that contained special e.g. non latin characters as 盗作. If you want to keep the legacy behavior set From 1ff254215a4dde9fe28bcc284163e05d38fc20c4 Mon Sep 17 00:00:00 2001 From: frigginbrownie Date: Wed, 18 Dec 2024 22:55:05 -0600 Subject: [PATCH 15/40] Update convert.py --- beetsplug/convert.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 2e837c77f..af1279299 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -279,6 +279,10 @@ class ConvertPlugin(BeetsPlugin): ) = self._get_opts_and_config(empty_opts) items = task.imported_items() + + # Filter items based on should_transcode function + items = [item for item in items if should_transcode(item, fmt)] + self._parallel_convert( dest, False, From bfb24da51ceb3dffcb8f6d2fcf06a8f334d27f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 15 Jan 2026 15:53:06 +0000 Subject: [PATCH 16/40] Add note to the changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 84bb0cc02..640e46988 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -92,6 +92,9 @@ Bug fixes: preventing incorrect splits when both are present. - :doc:`reference/cli`: Fix 'from_scratch' option for singleton imports: delete all (old) metadata when new metadata is applied. :bug:`3706` +- :doc:`/plugins/convert`: ``auto_keep`` now respects ``no_convert`` and + ``never_convert_lossy_files`` when deciding whether to copy/transcode items, + avoiding extra lossy duplicates. For plugin developers: From e85f67ac7b91be7792bfbc610adcfed0d1cf3b0c Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:43:01 +0530 Subject: [PATCH 17/40] refactor: suppress OSError when unlinking temporary files in ArtResizer --- beets/util/artresizer.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 23dce3c9f..b9401cca8 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -21,6 +21,7 @@ import os.path import platform import re import subprocess +from contextlib import suppress from itertools import chain from urllib.parse import urlencode @@ -660,10 +661,8 @@ class ArtResizer(metaclass=Shareable): ) finally: if result_path != path_in: - try: + with suppress(OSError): os.unlink(path_in) - except OSError: - pass return result_path @property From b0bce805189dbcc84a4421108ca0c340d8adcf4c Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:50:09 +0530 Subject: [PATCH 18/40] remove changelog not related to pr --- docs/changelog.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 881b9faa2..097e3bc3b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -474,15 +474,8 @@ Bug fixes: result. Update the default ``sources`` configuration to prioritize ``lrclib`` over other sources since it returns reliable results quicker than others. :bug:`5102` -* :doc:`plugins/lyrics`: Fix the issue with ``genius`` backend not being able - to match lyrics when there is a slight variation in the artist name. - :bug:`4791` -* :doc:`plugins/lyrics`: Fix plugin crash when ``genius`` backend returns empty - lyrics. - :bug:`5583` * Handle potential OSError when unlinking temporary files in ArtResizer. :bug:`5615` -* ImageMagick 7.1.1-44 is now supported. - :doc:`plugins/lyrics`: Fix the issue with ``genius`` backend not being able to match lyrics when there is a slight variation in the artist name. :bug:`4791` - :doc:`plugins/lyrics`: Fix plugin crash when ``genius`` backend returns empty From 4ad5871ef020633d9f438d9112288b9b3d6f54f6 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Fri, 16 Jan 2026 15:53:34 +0530 Subject: [PATCH 19/40] fix: sort imports --- beets/util/artresizer.py | 2 +- docs/changelog.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index 6f6a7b99e..ae1476101 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -24,8 +24,8 @@ import platform import re import subprocess from abc import ABC, abstractmethod -from enum import Enum from contextlib import suppress +from enum import Enum from itertools import chain from typing import TYPE_CHECKING, Any, ClassVar from urllib.parse import urlencode diff --git a/docs/changelog.rst b/docs/changelog.rst index 097e3bc3b..f7a6f9d48 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -474,7 +474,7 @@ Bug fixes: result. Update the default ``sources`` configuration to prioritize ``lrclib`` over other sources since it returns reliable results quicker than others. :bug:`5102` -* Handle potential OSError when unlinking temporary files in ArtResizer. +- Handle potential OSError when unlinking temporary files in ArtResizer. :bug:`5615` - :doc:`plugins/lyrics`: Fix the issue with ``genius`` backend not being able to match lyrics when there is a slight variation in the artist name. :bug:`4791` From 179dc7d0701e0252da4c35d627e8da1da1f6ff90 Mon Sep 17 00:00:00 2001 From: m_igashi <@M_Igashi> Date: Fri, 16 Jan 2026 16:06:17 +0100 Subject: [PATCH 20/40] style: remove trailing whitespace from blank line --- beetsplug/replaygain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index af5dcd001..7197971c2 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -610,7 +610,7 @@ class CommandBackend(Backend): """Checks whether the given item is supported by the selected tool.""" # Get the base name of the command for comparison cmd_name = os.path.basename(self.command).lower() - + if cmd_name.startswith("mp3rgain"): # mp3rgain supports MP3 and AAC/M4A formats return item.format in ("MP3", "AAC") From 683da049a09586ce0485a4d4823d2d4cce441887 Mon Sep 17 00:00:00 2001 From: m_igashi <@M_Igashi> Date: Fri, 16 Jan 2026 16:17:45 +0100 Subject: [PATCH 21/40] style: format replaygain.rst with docstrfmt --- docs/plugins/replaygain.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index 16f4e3088..6fa456bb5 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -57,8 +57,8 @@ mp3gain, aacgain, and mp3rgain In order to use this backend, you will need to install the mp3gain_ command-line tool, the aacgain_ fork, or mp3rgain_. Here are some hints: -- On Mac OS X, you can use Homebrew_. Type ``brew install aacgain`` or - ``brew install mp3rgain``. +- On Mac OS X, you can use Homebrew_. Type ``brew install aacgain`` or ``brew + install mp3rgain``. - On Linux, mp3gain_ is probably in your repositories. On Debian or Ubuntu, for example, you can run ``apt-get install mp3gain``. Alternatively, mp3rgain is available via Nix (``nix-env -iA nixpkgs.mp3rgain``) or AUR for Arch Linux. From 5ea41b3fbb26d89de9efd39edfc00b9b9f41c8a5 Mon Sep 17 00:00:00 2001 From: m_igashi <@M_Igashi> Date: Sat, 17 Jan 2026 02:10:29 +0100 Subject: [PATCH 22/40] refactor: simplify CommandBackend with SUPPORTED_FORMATS_BY_TOOL - Add Tool type alias and SUPPORTED_FORMATS_BY_TOOL class variable - Refactor __init__ to use shutil.which() and set cmd_name early - Simplify format_supported() to use dictionary lookup --- beetsplug/replaygain.py | 65 +++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 7197971c2..25472b6e6 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -28,7 +28,9 @@ from abc import ABC, abstractmethod from dataclasses import dataclass from multiprocessing.pool import ThreadPool from threading import Event, Thread -from typing import TYPE_CHECKING, Any, TypeVar +import shutil +from pathlib import Path +from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui from beets.plugins import BeetsPlugin @@ -542,10 +544,20 @@ class FfmpegBackend(Backend): # mpgain/aacgain CLI tool backend. +Tool = Literal["mp3rgain", "aacgain", "mp3gain"] + + class CommandBackend(Backend): NAME = "command" + SUPPORTED_FORMATS_BY_TOOL: ClassVar[dict[Tool, set[str]]] = { + "mp3rgain": {"AAC", "MP3"}, + "aacgain": {"AAC", "MP3"}, + "mp3gain": {"MP3"}, + } do_parallel = True + cmd_name: Tool + def __init__(self, config: ConfigView, log: Logger): super().__init__(config, log) config.add( @@ -555,26 +567,35 @@ class CommandBackend(Backend): } ) - self.command: str = config["command"].as_str() + cmd_path: Path = Path(config["command"].as_str()) + supported_tools = set(self.SUPPORTED_FORMATS_BY_TOOL) - if self.command: - # Explicit executable path. - if not os.path.isfile(self.command): + if cmd_path.name: + # Explicit command specified + if cmd_path.name not in supported_tools: raise FatalReplayGainError( - f"replaygain command does not exist: {self.command}" + f"replaygain.command must be one of {supported_tools!r}," + f" not {cmd_path.name!r}" + ) + if command_exec := shutil.which(str(cmd_path)): + self.command = command_exec + self.cmd_name = cmd_path.name # type: ignore[assignment] + else: + raise FatalReplayGainError( + f"replaygain command not found: {cmd_path}" ) else: # Check whether the program is in $PATH. for cmd in ("mp3rgain", "mp3gain", "aacgain"): - try: - call([cmd, "-v"], self._log) - self.command = cmd - except OSError: - pass - if not self.command: - raise FatalReplayGainError( - "no replaygain command found: install mp3rgain, mp3gain, or aacgain" - ) + if command_exec := shutil.which(cmd): + self.command = command_exec + self.cmd_name = cmd # type: ignore[assignment] + break + else: + raise FatalReplayGainError( + "no replaygain command found: install mp3rgain, mp3gain, " + "or aacgain" + ) self.noclip = config["noclip"].get(bool) @@ -608,19 +629,7 @@ class CommandBackend(Backend): def format_supported(self, item: Item) -> bool: """Checks whether the given item is supported by the selected tool.""" - # Get the base name of the command for comparison - cmd_name = os.path.basename(self.command).lower() - - if cmd_name.startswith("mp3rgain"): - # mp3rgain supports MP3 and AAC/M4A formats - return item.format in ("MP3", "AAC") - elif cmd_name.startswith("aacgain"): - # aacgain supports MP3 and AAC formats - return item.format in ("MP3", "AAC") - elif cmd_name.startswith("mp3gain"): - # mp3gain only supports MP3 - return item.format == "MP3" - return True + return item.format in self.SUPPORTED_FORMATS_BY_TOOL[self.cmd_name] def compute_gain( self, From 29e1c283eba0f3c2e8a196fd30eefc675a7f1a46 Mon Sep 17 00:00:00 2001 From: m_igashi <@M_Igashi> Date: Sat, 17 Jan 2026 02:15:43 +0100 Subject: [PATCH 23/40] fix: sort imports alphabetically --- beetsplug/replaygain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 25472b6e6..5a5cb96e8 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -20,6 +20,7 @@ import enum import math import os import queue +import shutil import signal import subprocess import sys @@ -27,9 +28,8 @@ import warnings from abc import ABC, abstractmethod from dataclasses import dataclass from multiprocessing.pool import ThreadPool -from threading import Event, Thread -import shutil from pathlib import Path +from threading import Event, Thread from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui From 52284ff7ed9ae38a25c9c5e76c697e47e7c6d4a5 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Sat, 17 Jan 2026 14:30:22 +0530 Subject: [PATCH 24/40] fix: changelog entry --- docs/changelog.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index f7a6f9d48..5408d2a5c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -48,6 +48,8 @@ New features: Bug fixes: +- Handle potential OSError when unlinking temporary files in ArtResizer. + :bug:`5615` - :doc:`/plugins/spotify`: Updated Spotify API credentials. :bug:`6270` - :doc:`/plugins/smartplaylist`: Fixed an issue where multiple queries in a playlist configuration were not preserving their order, causing items to @@ -474,8 +476,6 @@ Bug fixes: result. Update the default ``sources`` configuration to prioritize ``lrclib`` over other sources since it returns reliable results quicker than others. :bug:`5102` -- Handle potential OSError when unlinking temporary files in ArtResizer. - :bug:`5615` - :doc:`plugins/lyrics`: Fix the issue with ``genius`` backend not being able to match lyrics when there is a slight variation in the artist name. :bug:`4791` - :doc:`plugins/lyrics`: Fix plugin crash when ``genius`` backend returns empty From 545e7eb0b6983d7bc76a702cdfe9488a0a51a3f3 Mon Sep 17 00:00:00 2001 From: m_igashi <@M_Igashi> Date: Sun, 18 Jan 2026 10:52:41 +0100 Subject: [PATCH 25/40] refactor: simplify CommandBackend and improve documentation - Remove auto-detection of command tools, require explicit command config - Simplify __init__ method by removing redundant else branch - Reorganize docs with separate sections for mp3gain, aacgain, mp3rgain - Fix CVE reference (CVE-2021-34085 is fixed in mp3gain 1.6.2) - Update command option description per review feedback --- beetsplug/replaygain.py | 37 ++++++----------- docs/plugins/replaygain.rst | 83 ++++++++++++++++++++++--------------- 2 files changed, 62 insertions(+), 58 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 5a5cb96e8..e83345059 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -570,32 +570,19 @@ class CommandBackend(Backend): cmd_path: Path = Path(config["command"].as_str()) supported_tools = set(self.SUPPORTED_FORMATS_BY_TOOL) - if cmd_path.name: - # Explicit command specified - if cmd_path.name not in supported_tools: - raise FatalReplayGainError( - f"replaygain.command must be one of {supported_tools!r}," - f" not {cmd_path.name!r}" - ) - if command_exec := shutil.which(str(cmd_path)): - self.command = command_exec - self.cmd_name = cmd_path.name # type: ignore[assignment] - else: - raise FatalReplayGainError( - f"replaygain command not found: {cmd_path}" - ) + if (cmd_name := cmd_path.name) not in supported_tools: + raise FatalReplayGainError( + f"replaygain.command must be one of {supported_tools!r}," + f" not {cmd_name!r}" + ) + + if command_exec := shutil.which(str(cmd_path)): + self.command = command_exec + self.cmd_name = cmd_name # type: ignore[assignment] else: - # Check whether the program is in $PATH. - for cmd in ("mp3rgain", "mp3gain", "aacgain"): - if command_exec := shutil.which(cmd): - self.command = command_exec - self.cmd_name = cmd # type: ignore[assignment] - break - else: - raise FatalReplayGainError( - "no replaygain command found: install mp3rgain, mp3gain, " - "or aacgain" - ) + raise FatalReplayGainError( + f"replaygain command not found: {cmd_path}" + ) self.noclip = config["noclip"].get(bool) diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index 6fa456bb5..2973dd959 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -51,24 +51,59 @@ configuration file: The GStreamer backend does not support parallel analysis. -mp3gain, aacgain, and mp3rgain +Supported ``command`` backends ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -In order to use this backend, you will need to install the mp3gain_ command-line -tool, the aacgain_ fork, or mp3rgain_. Here are some hints: +In order to use this backend, you will need to install a supported command-line +tool: + +- mp3gain_ (MP3 only) +- aacgain_ (MP3, AAC/M4A) +- mp3rgain_ (MP3, AAC/M4A) + +mp3gain ++++++++ -- On Mac OS X, you can use Homebrew_. Type ``brew install aacgain`` or ``brew - install mp3rgain``. - On Linux, mp3gain_ is probably in your repositories. On Debian or Ubuntu, for - example, you can run ``apt-get install mp3gain``. Alternatively, mp3rgain is - available via Nix (``nix-env -iA nixpkgs.mp3rgain``) or AUR for Arch Linux. -- On Windows, download and install mp3rgain_ (recommended) or the original - mp3gain_. + example, you can run ``apt-get install mp3gain``. +- On Windows, download and install mp3gain_. -mp3rgain_ is a modern Rust rewrite of mp3gain that also supports AAC/M4A files. -It addresses security vulnerabilities (CVE-2021-34085, CVE-2019-18359) present -in the original mp3gain and works on modern systems including Windows 11 and -macOS with Apple Silicon. +aacgain ++++++++ + +- On macOS, install via Homebrew_: ``brew install aacgain``. +- For other platforms, download from aacgain_ or use a compatible fork if + available for your system. + +mp3rgain +++++++++ + +mp3rgain_ is a modern Rust rewrite of ``mp3gain`` that also supports AAC/M4A +files. It addresses security vulnerability CVE-2019-18359 present in the +original mp3gain and works on modern systems including Windows 11 and macOS with +Apple Silicon. + +- On macOS, install via Homebrew_: ``brew install mp3rgain``. +- On Linux, install via Nix: ``nix-env -iA nixpkgs.mp3rgain`` or from your + distribution packaging (for example, AUR on Arch Linux). +- On Windows, download and install mp3rgain_. + +Configuration ++++++++++++++ + +.. code-block:: yaml + + replaygain: + backend: command + command: # mp3rgain, mp3gain, or aacgain + +If beets doesn't automatically find the command executable, you can configure +the path explicitly like so: + +.. code-block:: yaml + + replaygain: + command: /Applications/MacMP3Gain.app/Contents/Resources/aacgain .. _aacgain: https://aacgain.altosdesign.com @@ -78,22 +113,6 @@ macOS with Apple Silicon. .. _mp3rgain: https://github.com/M-Igashi/mp3rgain -Then, enable the plugin (see :ref:`using-plugins`) and specify the "command" -backend in your configuration file: - -:: - - replaygain: - backend: command - -If beets doesn't automatically find the ``mp3gain`` or ``aacgain`` executable, -you can configure the path explicitly like so: - -:: - - replaygain: - command: /Applications/MacMP3Gain.app/Contents/Resources/aacgain - Python Audio Tools ~~~~~~~~~~~~~~~~~~ @@ -154,10 +173,8 @@ file. The available options are: These options only work with the "command" backend: -- **command**: The path to the ``mp3rgain``, ``mp3gain``, or ``aacgain`` - executable (if beets cannot find it by itself). For example: - ``/Applications/MacMP3Gain.app/Contents/Resources/aacgain``. Default: Search - in your ``$PATH``. +- **command**: Name or path to your command backend of choice: either of + ``mp3gain``, ``aacgain`` or ``mp3rgain``. - **noclip**: Reduce the amount of ReplayGain adjustment to whatever amount would keep clipping from occurring. Default: ``yes``. From 9efe87101ce03706660129633e03147a222765cf Mon Sep 17 00:00:00 2001 From: Henry Date: Sat, 22 Nov 2025 17:13:08 -0800 Subject: [PATCH 26/40] Fix #6177, remove derived types, refactor coalesce tracks --- beetsplug/discogs.py | 217 +++++++++++++++++++---------------- docs/changelog.rst | 2 + test/plugins/test_discogs.py | 2 +- 3 files changed, 121 insertions(+), 100 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 08d437d2d..6941cf891 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -27,7 +27,7 @@ import time import traceback from functools import cache from string import ascii_lowercase -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING import confuse from discogs_client import Client, Master, Release @@ -102,23 +102,7 @@ class Track(TypedDict): duration: str artists: list[Artist] extraartists: NotRequired[list[Artist]] - - -class TrackWithSubtracks(Track): - sub_tracks: list[TrackWithSubtracks] - - -class IntermediateTrackInfo(TrackInfo): - """Allows work with string mediums from - get_track_info""" - - def __init__( - self, - medium_str: str | None, - **kwargs, - ) -> None: - self.medium_str = medium_str - super().__init__(**kwargs) + sub_tracks: NotRequired[list[Track]] class DiscogsPlugin(MetadataSourcePlugin): @@ -520,9 +504,19 @@ class DiscogsPlugin(MetadataSourcePlugin): self, clean_tracklist: list[Track], album_artist_data: tuple[str, str, str | None], - ) -> tuple[list[TrackInfo], dict[int, str], int, list[str], list[str]]: + ) -> tuple[ + list[TrackInfo], + dict[int, str], + int, + list[str], + list[str], + list[str | None], + list[str | None], + ]: # Distinct works and intra-work divisions, as defined by index tracks. tracks: list[TrackInfo] = [] + mediums: list[str | None] = [] + medium_indices: list[str | None] = [] index_tracks = {} index = 0 divisions: list[str] = [] @@ -536,11 +530,19 @@ class DiscogsPlugin(MetadataSourcePlugin): # divisions. divisions += next_divisions del next_divisions[:] - track_info = self.get_track_info( + track_info, medium, medium_index = self.get_track_info( track, index, divisions, album_artist_data ) track_info.track_alt = track["position"] tracks.append(track_info) + if medium: + mediums.append(medium) + else: + mediums.append(None) + if medium_index: + medium_indices.append(medium_index) + else: + medium_indices.append(None) else: next_divisions.append(track["title"]) # We expect new levels of division at the beginning of the @@ -550,7 +552,15 @@ class DiscogsPlugin(MetadataSourcePlugin): except IndexError: pass index_tracks[index + 1] = track["title"] - return tracks, index_tracks, index, divisions, next_divisions + return ( + tracks, + index_tracks, + index, + divisions, + next_divisions, + mediums, + medium_indices, + ) def get_tracks( self, @@ -559,9 +569,7 @@ class DiscogsPlugin(MetadataSourcePlugin): ) -> list[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: - clean_tracklist: list[Track] = self.coalesce_tracks( - cast(list[TrackWithSubtracks], tracklist) - ) + clean_tracklist: list[Track] = self.coalesce_tracks(tracklist) except Exception as exc: # FIXME: this is an extra precaution for making sure there are no # side effects after #2222. It should be removed after further @@ -572,7 +580,15 @@ class DiscogsPlugin(MetadataSourcePlugin): processed = self._process_clean_tracklist( clean_tracklist, album_artist_data ) - tracks, index_tracks, *_ = processed + ( + tracks, + index_tracks, + index, + divisions, + next_divisions, + mediums, + medium_indices, + ) = processed # Fix up medium and medium_index for each track. Discogs position is # unreliable, but tracks are in order. medium = None @@ -581,32 +597,34 @@ class DiscogsPlugin(MetadataSourcePlugin): # If a medium has two sides (ie. vinyl or cassette), each pair of # consecutive sides should belong to the same medium. - if all([track.medium_str is not None for track in tracks]): - m = sorted({track.medium_str.lower() for track in tracks}) + if all([medium is not None for medium in mediums]): + m = sorted({medium.lower() if medium else "" for medium in mediums}) # If all track.medium are single consecutive letters, assume it is # a 2-sided medium. if "".join(m) in ascii_lowercase: sides_per_medium = 2 - for track in tracks: + for i, track in enumerate(tracks): # Handle special case where a different medium does not indicate a # new disc, when there is no medium_index and the ordinal of medium # is not sequential. For example, I, II, III, IV, V. Assume these # are the track index, not the medium. # side_count is the number of mediums or medium sides (in the case # of two-sided mediums) that were seen before. + medium_str = mediums[i] + medium_index = medium_indices[i] medium_is_index = ( - track.medium_str - and not track.medium_index + medium_str + and not medium_index and ( - len(track.medium_str) != 1 + len(medium_str) != 1 or # Not within standard incremental medium values (A, B, C, ...). - ord(track.medium_str) - 64 != side_count + 1 + ord(medium_str) - 64 != side_count + 1 ) ) - if not medium_is_index and medium != track.medium_str: + if not medium_is_index and medium != medium_str: side_count += 1 if sides_per_medium == 2: if side_count % sides_per_medium: @@ -617,7 +635,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # Medium changed. Reset index_count. medium_count += 1 index_count = 0 - medium = track.medium_str + medium = medium_str index_count += 1 medium_count = 1 if medium_count == 0 else medium_count @@ -633,61 +651,17 @@ class DiscogsPlugin(MetadataSourcePlugin): disctitle = None track.disctitle = disctitle - return cast(list[TrackInfo], tracks) + return tracks - def coalesce_tracks( - self, raw_tracklist: list[TrackWithSubtracks] - ) -> list[Track]: + def coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]: """Pre-process a tracklist, merging subtracks into a single track. The title for the merged track is the one from the previous index track, if present; otherwise it is a combination of the subtracks titles. """ - - def add_merged_subtracks( - tracklist: list[TrackWithSubtracks], - subtracks: list[TrackWithSubtracks], - ) -> None: - """Modify `tracklist` in place, merging a list of `subtracks` into - a single track into `tracklist`.""" - # Calculate position based on first subtrack, without subindex. - idx, medium_idx, sub_idx = self.get_track_index( - subtracks[0]["position"] - ) - position = f"{idx or ''}{medium_idx or ''}" - - if tracklist and not tracklist[-1]["position"]: - # Assume the previous index track contains the track title. - if sub_idx: - # "Convert" the track title to a real track, discarding the - # subtracks assuming they are logical divisions of a - # physical track (12.2.9 Subtracks). - tracklist[-1]["position"] = position - else: - # Promote the subtracks to real tracks, discarding the - # index track, assuming the subtracks are physical tracks. - index_track = tracklist.pop() - # Fix artists when they are specified on the index track. - if index_track.get("artists"): - for subtrack in subtracks: - if not subtrack.get("artists"): - subtrack["artists"] = index_track["artists"] - # Concatenate index with track title when index_tracks - # option is set - if self.config["index_tracks"]: - for subtrack in subtracks: - subtrack["title"] = ( - f"{index_track['title']}: {subtrack['title']}" - ) - tracklist.extend(subtracks) - else: - # Merge the subtracks, pick a title, and append the new track. - track = subtracks[0].copy() - track["title"] = " / ".join([t["title"] for t in subtracks]) - tracklist.append(track) - # Pre-process the tracklist, trying to identify subtracks. - subtracks: list[TrackWithSubtracks] = [] - tracklist: list[TrackWithSubtracks] = [] + + subtracks: list[Track] = [] + tracklist: list[Track] = [] prev_subindex = "" for track in raw_tracklist: # Regular subtrack (track with subindex). @@ -699,7 +673,7 @@ class DiscogsPlugin(MetadataSourcePlugin): subtracks.append(track) else: # Subtrack part of a new group (..., 1.3, *2.1*, ...). - add_merged_subtracks(tracklist, subtracks) + self._add_merged_subtracks(tracklist, subtracks) subtracks = [track] prev_subindex = subindex.rjust(len(raw_tracklist)) continue @@ -708,21 +682,64 @@ class DiscogsPlugin(MetadataSourcePlugin): if not track["position"] and "sub_tracks" in track: # Append the index track, assuming it contains the track title. tracklist.append(track) - add_merged_subtracks(tracklist, track["sub_tracks"]) + self._add_merged_subtracks(tracklist, track["sub_tracks"]) continue # Regular track or index track without nested sub_tracks. if subtracks: - add_merged_subtracks(tracklist, subtracks) + self._add_merged_subtracks(tracklist, subtracks) subtracks = [] prev_subindex = "" tracklist.append(track) # Merge and add the remaining subtracks, if any. if subtracks: - add_merged_subtracks(tracklist, subtracks) + self._add_merged_subtracks(tracklist, subtracks) - return cast(list[Track], tracklist) + return tracklist + + def _add_merged_subtracks( + self, + tracklist: list[Track], + subtracks: list[Track], + ) -> None: + """Modify `tracklist` in place, merging a list of `subtracks` into + a single track into `tracklist`.""" + # Calculate position based on first subtrack, without subindex. + idx, medium_idx, sub_idx = self.get_track_index( + subtracks[0]["position"] + ) + position = f"{idx or ''}{medium_idx or ''}" + + if tracklist and not tracklist[-1]["position"]: + # Assume the previous index track contains the track title. + if sub_idx: + # "Convert" the track title to a real track, discarding the + # subtracks assuming they are logical divisions of a + # physical track (12.2.9 Subtracks). + tracklist[-1]["position"] = position + else: + # Promote the subtracks to real tracks, discarding the + # index track, assuming the subtracks are physical tracks. + index_track = tracklist.pop() + # Fix artists when they are specified on the index track. + if index_track.get("artists"): + for subtrack in subtracks: + if not subtrack.get("artists"): + subtrack["artists"] = index_track["artists"] + # Concatenate index with track title when index_tracks + # option is set + if self.config["index_tracks"]: + for subtrack in subtracks: + subtrack["title"] = ( + f"{index_track['title']}: {subtrack['title']}" + ) + tracklist.extend(subtracks) + else: + # Merge the subtracks, pick a title, and append the new track. + track = subtracks[0].copy() + track["title"] = " / ".join([t["title"] for t in subtracks]) + tracklist.append(track) def strip_disambiguation(self, text: str) -> str: """Removes discogs specific disambiguations from a string. @@ -738,7 +755,7 @@ class DiscogsPlugin(MetadataSourcePlugin): index: int, divisions: list[str], album_artist_data: tuple[str, str, str | None], - ) -> IntermediateTrackInfo: + ) -> tuple[TrackInfo, str | None, str | None]: """Returns a TrackInfo object for a discogs track.""" artist, artist_anv, artist_id = album_artist_data @@ -784,16 +801,18 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_credit += ( f" {self.config['featured_string']} {featured_credit}" ) - return IntermediateTrackInfo( - title=title, - track_id=track_id, - artist_credit=artist_credit, - artist=artist, - artist_id=artist_id, - length=length, - index=index, - medium_str=medium, - medium_index=medium_index, + return ( + TrackInfo( + title=title, + track_id=track_id, + artist_credit=artist_credit, + artist=artist, + artist_id=artist_id, + length=length, + index=index, + ), + medium, + medium_index, ) @staticmethod diff --git a/docs/changelog.rst b/docs/changelog.rst index 5408d2a5c..93606cf1e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -97,6 +97,8 @@ Bug fixes: - :doc:`/plugins/convert`: ``auto_keep`` now respects ``no_convert`` and ``never_convert_lossy_files`` when deciding whether to copy/transcode items, avoiding extra lossy duplicates. +- :doc:`plugins/discogs`: Fixed unexpected flex attr from the Discogs plugin. + :bug:`6177` For plugin developers: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index eb65bc588..fd820ab43 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -608,7 +608,7 @@ def test_parse_featured_artists(track, expected_artist): """Tests the plugins ability to parse a featured artist. Initial check with one featured artist, two featured artists, and three. Ignores artists that are not listed as featured.""" - t = DiscogsPlugin().get_track_info( + t, _, _ = DiscogsPlugin().get_track_info( track, 1, 1, ("ARTIST", "ARTIST CREDIT", 2) ) assert t.artist == expected_artist From 1d6e05709e758a2522f9260ef28150c1d3ee90ab Mon Sep 17 00:00:00 2001 From: Henry Date: Sun, 7 Dec 2025 14:37:12 -0800 Subject: [PATCH 27/40] Fix #6068 - Multivalue fields are now supported & tested. --- beetsplug/discogs.py | 212 +++++++++++++++++++++++++---------- docs/changelog.rst | 1 + test/plugins/test_discogs.py | 103 ++++++++++++++--- 3 files changed, 238 insertions(+), 78 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 6941cf891..eb8465960 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -18,6 +18,7 @@ python3-discogs-client library. from __future__ import annotations +import copy import http.client import json import os @@ -105,6 +106,24 @@ class Track(TypedDict): sub_tracks: NotRequired[list[Track]] +class ArtistInfo(TypedDict): + artist: str + artists: list[str] + artist_credit: str + artists_credit: list[str] + artist_id: str + artists_ids: list[str] + + +class AlbumArtistInfo(ArtistInfo): + albumartist: str + albumartists: list[str] + albumartist_credit: str + albumartists_credit: list[str] + albumartist_id: str + albumartists_ids: list[str] + + class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): super().__init__() @@ -261,7 +280,6 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in album.tracks: if track.track_id == track_id: return track - return None def get_albums(self, query: str) -> Iterable[AlbumInfo]: @@ -346,6 +364,121 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist(artist_list, join_key="join") return self.strip_disambiguation(artist), artist_id + def build_albumartistinfo(self, artists: list[Artist]) -> AlbumArtistInfo: + info = self.build_artistinfo(artists, album_artist=True) + albumartist: AlbumArtistInfo = { + **info, + "albumartist": info["artist"], + "albumartist_id": info["artist_id"], + "albumartists": info["artists"], + "albumartists_ids": info["artists_ids"], + "albumartist_credit": info["artist_credit"], + "albumartists_credit": info["artists_credit"], + } + return albumartist + + def build_artistinfo( + self, + given_artists: list[Artist], + given_info: ArtistInfo | None = None, + album_artist: bool = False, + ) -> ArtistInfo: + """Iterates through a discogs result and builds + up the artist fields. Does not contribute to + artist_sort as Discogs does not define that. + + :param artists: A list of Discogs Artist objects + + :param album_artist: If building an album artist, + we need to account for the album_artist anv parameter. + :return an ArtistInfo dictionary. + """ + info: ArtistInfo = { + "artist": "", + "artist_id": "", + "artists": [], + "artists_ids": [], + "artist_credit": "", + "artists_credit": [], + } + if given_info: + info = copy.deepcopy(given_info) + + a_anv: bool = self.config["anv"]["artist"].get(bool) + ac_anv: bool = self.config["anv"]["artist_credit"].get(bool) + aa_anv: bool = self.config["anv"]["album_artist"].get(bool) + feat_str: str = f" {self.config['featured_string'].get(str)} " + + artist = "" + artist_anv = "" + artists: list[str] = [] + artists_anv: list[str] = [] + + join = "" + featured_flag = False + # Iterate through building the artist strings + for a in given_artists: + # Get the artist name + name = self.strip_disambiguation(a["name"]) + discogs_id = a["id"] + anv = a.get("anv", name) + role = a.get("role", "").lower() + # Check if the artist is Various + if name.lower() == "various": + name = config["va_name"].as_str() + anv = name + + if "featuring" in role: + if not featured_flag: + artist += feat_str + artist_anv += feat_str + artist += name + artist_anv += anv + featured_flag = True + else: + artist = self._join_artist(artist, name, join) + artist_anv = self._join_artist(artist_anv, anv, join) + elif role and "featuring" not in role: + continue + else: + artist = self._join_artist(artist, name, join) + artist_anv = self._join_artist(artist_anv, anv, join) + artists.append(name) + artists_anv.append(anv) + # Only the first ID is set for the singular field + if not info["artist_id"]: + info["artist_id"] = discogs_id + info["artists_ids"].append(discogs_id) + # Update join for the next artist + join = a.get("join", "") + # Assign fields as necessary + if (a_anv and not album_artist) or (aa_anv and album_artist): + info["artist"] += artist_anv + info["artists"] += artists_anv + else: + info["artist"] += artist + info["artists"] += artists + + if ac_anv: + info["artist_credit"] += artist_anv + info["artists_credit"] += artists_anv + else: + info["artist_credit"] += artist + info["artists_credit"] += artists + return info + + def _join_artist(self, base: str, artist: str, join: str) -> str: + # Expand the artist field + if not base: + base = artist + else: + if join: + base += f" {join} " + else: + base += ", " + base += artist + return base + def get_album_info(self, result: Release) -> AlbumInfo | None: """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet @@ -375,11 +508,8 @@ class DiscogsPlugin(MetadataSourcePlugin): return None artist_data = [a.data for a in result.artists] - album_artist, album_artist_id = self.get_artist_with_anv(artist_data) - album_artist_anv, _ = self.get_artist_with_anv( - artist_data, use_anv=True - ) - artist_credit = album_artist_anv + # Information for the album artist + albumartist: AlbumArtistInfo = self.build_albumartistinfo(artist_data) album = re.sub(r" +", " ", result.title) album_id = result.data["id"] @@ -388,18 +518,11 @@ class DiscogsPlugin(MetadataSourcePlugin): # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. tracks = self.get_tracks( - result.data["tracklist"], - (album_artist, album_artist_anv, album_artist_id), + result.data["tracklist"], self.build_artistinfo(artist_data) ) - # Assign ANV to the proper fields for tagging - if not self.config["anv"]["artist_credit"]: - artist_credit = album_artist - if self.config["anv"]["album_artist"]: - album_artist = album_artist_anv - # Extract information for the optional AlbumInfo fields, if possible. - va = result.data["artists"][0].get("name", "").lower() == "various" + va = albumartist["albumartist"] == config["va_name"].as_str() year = result.data.get("year") mediums = [t.medium for t in tracks] country = result.data.get("country") @@ -431,11 +554,7 @@ class DiscogsPlugin(MetadataSourcePlugin): cover_art_url = self.select_cover_art(result) # Additional cleanups - # (various artists name, catalog number, media, disambiguation). - if va: - va_name = config["va_name"].as_str() - album_artist = va_name - artist_credit = va_name + # (catalog number, media, disambiguation). if catalogno == "none": catalogno = None # Explicitly set the `media` for the tracks, since it is expected by @@ -458,9 +577,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return AlbumInfo( album=album, album_id=album_id, - artist=album_artist, - artist_credit=artist_credit, - artist_id=album_artist_id, + **albumartist, # Unpacks values to satisfy the keyword arguments tracks=tracks, albumtype=albumtype, va=va, @@ -478,7 +595,7 @@ class DiscogsPlugin(MetadataSourcePlugin): data_url=data_url, discogs_albumid=discogs_albumid, discogs_labelid=labelid, - discogs_artistid=album_artist_id, + discogs_artistid=albumartist["albumartist_id"], cover_art_url=cover_art_url, ) @@ -503,7 +620,7 @@ class DiscogsPlugin(MetadataSourcePlugin): def _process_clean_tracklist( self, clean_tracklist: list[Track], - album_artist_data: tuple[str, str, str | None], + albumartistinfo: ArtistInfo, ) -> tuple[ list[TrackInfo], dict[int, str], @@ -531,7 +648,7 @@ class DiscogsPlugin(MetadataSourcePlugin): divisions += next_divisions del next_divisions[:] track_info, medium, medium_index = self.get_track_info( - track, index, divisions, album_artist_data + track, index, divisions, albumartistinfo ) track_info.track_alt = track["position"] tracks.append(track_info) @@ -565,7 +682,7 @@ class DiscogsPlugin(MetadataSourcePlugin): def get_tracks( self, tracklist: list[Track], - album_artist_data: tuple[str, str, str | None], + albumartistinfo: ArtistInfo, ) -> list[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: @@ -578,7 +695,7 @@ class DiscogsPlugin(MetadataSourcePlugin): self._log.error("uncaught exception in coalesce_tracks: {}", exc) clean_tracklist = tracklist processed = self._process_clean_tracklist( - clean_tracklist, album_artist_data + clean_tracklist, albumartistinfo ) ( tracks, @@ -754,16 +871,11 @@ class DiscogsPlugin(MetadataSourcePlugin): track: Track, index: int, divisions: list[str], - album_artist_data: tuple[str, str, str | None], + albumartistinfo: ArtistInfo, ) -> tuple[TrackInfo, str | None, str | None]: """Returns a TrackInfo object for a discogs track.""" - artist, artist_anv, artist_id = album_artist_data - artist_credit = artist_anv - if not self.config["anv"]["artist_credit"]: - artist_credit = artist - if self.config["anv"]["artist"]: - artist = artist_anv + artistinfo = albumartistinfo.copy() title = track["title"] if self.config["index_tracks"]: @@ -775,39 +887,19 @@ class DiscogsPlugin(MetadataSourcePlugin): # If artists are found on the track, we will use those instead if artists := track.get("artists", []): - artist, artist_id = self.get_artist_with_anv( - artists, self.config["anv"]["artist"] - ) - artist_credit, _ = self.get_artist_with_anv( - artists, self.config["anv"]["artist_credit"] - ) + artistinfo = self.build_artistinfo(artists) + length = self.get_track_length(track["duration"]) # Add featured artists if extraartists := track.get("extraartists", []): - featured_list = [ - artist - for artist in extraartists - if "Featuring" in artist["role"] - ] - featured, _ = self.get_artist_with_anv( - featured_list, self.config["anv"]["artist"] - ) - featured_credit, _ = self.get_artist_with_anv( - featured_list, self.config["anv"]["artist_credit"] - ) - if featured: - artist += f" {self.config['featured_string']} {featured}" - artist_credit += ( - f" {self.config['featured_string']} {featured_credit}" - ) + artistinfo = self.build_artistinfo(extraartists, artistinfo) + return ( TrackInfo( title=title, track_id=track_id, - artist_credit=artist_credit, - artist=artist, - artist_id=artist_id, + **artistinfo, length=length, index=index, ), diff --git a/docs/changelog.rst b/docs/changelog.rst index 93606cf1e..3400c8893 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -45,6 +45,7 @@ New features: of brackets are supported and a new ``bracket_keywords`` configuration option allows customizing the keywords. Setting ``bracket_keywords`` to an empty list matches any bracket content regardless of keywords. +- :doc:`plugins/discogs`: Added support for multi value fields. :bug:`6068` Bug fixes: diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index fd820ab43..c11148c13 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -409,7 +409,9 @@ class DGAlbumInfoTest(BeetsTestCase): ) d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME & OTHER ARTIST" + assert d.artists == ["ARTIST NAME", "OTHER ARTIST"] assert d.tracks[0].artist == "TEST ARTIST" + assert d.tracks[0].artists == ["TEST ARTIST"] assert d.label == "LABEL NAME" def test_strip_disambiguation_false(self): @@ -448,35 +450,62 @@ class DGAlbumInfoTest(BeetsTestCase): ) d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME (2) & OTHER ARTIST (5)" + assert d.artists == ["ARTIST NAME (2)", "OTHER ARTIST (5)"] assert d.tracks[0].artist == "TEST ARTIST (5)" + assert d.tracks[0].artists == ["TEST ARTIST (5)"] assert d.label == "LABEL NAME (5)" config["discogs"]["strip_disambiguation"] = True @pytest.mark.parametrize( - "track_artist_anv,track_artist", - [(False, "ARTIST Feat. PERFORMER"), (True, "VARIATION Feat. VARIATION")], -) -@pytest.mark.parametrize( - "album_artist_anv,album_artist", - [(False, "ARTIST & SOLOIST"), (True, "VARIATION & VARIATION")], -) -@pytest.mark.parametrize( - "artist_credit_anv,track_artist_credit,album_artist_credit", + "track_artist_anv,track_artist,track_artists", [ - (False, "ARTIST Feat. PERFORMER", "ARTIST & SOLOIST"), - (True, "VARIATION Feat. VARIATION", "VARIATION & VARIATION"), + (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PEFORMER"]), + (True, "VARIATION Feat. VARIATION", ["VARIATION", "VARIATION"]), + ], +) +@pytest.mark.parametrize( + "album_artist_anv,album_artist,album_artists", + [ + (False, "ARTIST & SOLOIST", ["ARTIST", "SOLOIST"]), + (True, "VARIATION & VARIATION", ["VARIATION", "VARIATION"]), + ], +) +@pytest.mark.parametrize( + ( + "artist_credit_anv,track_artist_credit," + "track_artists_credit,album_artist_credit,album_artists_credit" + ), + [ + ( + False, + "ARTIST Feat. PERFORMER", + ["ARTIST", "PEFORMER"], + "ARTIST & SOLOIST", + ["ARTIST", "SOLOIST"], + ), + ( + True, + "VARIATION Feat. VARIATION", + ["VARIATION", "VARIATION"], + "VARIATION & VARIATION", + ["VARIATION", "VARIATION"], + ), ], ) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_anv( track_artist_anv, track_artist, + track_artists, album_artist_anv, album_artist, + album_artists, artist_credit_anv, track_artist_credit, + track_artists_credit, album_artist_credit, + album_artists_credit, ): """Test using artist name variations.""" data = { @@ -558,13 +587,21 @@ def test_anv_album_artist(): config["discogs"]["anv"]["artist_credit"] = False r = DiscogsPlugin().get_album_info(release) assert r.artist == "ARTIST" + assert r.artists == ["ARTIST"] + assert r.albumartist == "ARTIST" + assert r.albumartist_credit == "ARTIST" + assert r.albumartists == ["ARTIST"] + assert r.albumartists_credit == ["ARTIST"] assert r.artist_credit == "ARTIST" + assert r.artists_credit == ["ARTIST"] assert r.tracks[0].artist == "VARIATION" + assert r.tracks[0].artists == ["VARIATION"] assert r.tracks[0].artist_credit == "ARTIST" + assert r.tracks[0].artists_credit == ["ARTIST"] @pytest.mark.parametrize( - "track, expected_artist", + "track, expected_artist, expected_artists", [ ( { @@ -600,18 +637,25 @@ def test_anv_album_artist(): ], }, "NEW ARTIST, VOCALIST Feat. SOLOIST, PERFORMER, MUSICIAN", + ["NEW ARTIST", "VOCALIST", "SOLOIST", "PERFORMER", "MUSICIAN"], ), ], ) @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) -def test_parse_featured_artists(track, expected_artist): +def test_parse_featured_artists(track, expected_artist, expected_artists): """Tests the plugins ability to parse a featured artist. - Initial check with one featured artist, two featured artists, - and three. Ignores artists that are not listed as featured.""" - t, _, _ = DiscogsPlugin().get_track_info( - track, 1, 1, ("ARTIST", "ARTIST CREDIT", 2) - ) + Ignores artists that are not listed as featured.""" + artistinfo = { + "artist": "ARTIST", + "artist_id": "1", + "artists": ["ARTIST"], + "artists_ids": ["1"], + "artist_credit": "ARTIST", + "artists_credit": ["ARTIST"], + } + t, _, _ = DiscogsPlugin().get_track_info(track, 1, 1, artistinfo) assert t.artist == expected_artist + assert t.artists == expected_artists @pytest.mark.parametrize( @@ -637,6 +681,29 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): assert result == (expected_media, expected_albumtype) +@pytest.mark.parametrize( + "given_artists,expected_info,config_va_name", + [ + ( + [{"name": "Various", "id": "1"}], + { + "artist": "VARIOUS ARTISTS", + "artist_id": "1", + "artists": ["VARIOUS ARTISTS"], + "artists_ids": ["1"], + "artist_credit": "VARIOUS ARTISTS", + "artists_credit": ["VARIOUS ARTISTS"], + }, + "VARIOUS ARTISTS", + ) + ], +) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_va_buildartistinfo(given_artists, expected_info, config_va_name): + config["va_name"] = config_va_name + assert DiscogsPlugin().build_artistinfo(given_artists) == expected_info + + @pytest.mark.parametrize( "position, medium, index, subindex", [ From f0aef6e213384dc50c909b8f19422d09879aca0a Mon Sep 17 00:00:00 2001 From: Henry Date: Tue, 9 Dec 2025 21:24:36 -0800 Subject: [PATCH 28/40] Cleanup for #6177, #6068 --- beetsplug/discogs.py | 106 +++++++++++++---------------------- test/plugins/test_discogs.py | 13 ++++- 2 files changed, 51 insertions(+), 68 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index eb8465960..0a86d245d 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -121,7 +121,17 @@ class AlbumArtistInfo(ArtistInfo): albumartist_credit: str albumartists_credit: list[str] albumartist_id: str - albumartists_ids: list[str] + + +class TracklistInfo: + def __init__(self): + self.index: int = 0 + self.index_tracks: dict[int, str] = {} + self.tracks: list[TrackInfo] = [] + self.divisions: list[str] = [] + self.next_divisions: list[str] = [] + self.mediums: list[str | None] = [] + self.medium_indices: list[str | None] = [] class DiscogsPlugin(MetadataSourcePlugin): @@ -371,7 +381,6 @@ class DiscogsPlugin(MetadataSourcePlugin): "albumartist": info["artist"], "albumartist_id": info["artist_id"], "albumartists": info["artists"], - "albumartists_ids": info["artists_ids"], "albumartist_credit": info["artist_credit"], "albumartists_credit": info["artists_credit"], } @@ -386,12 +395,6 @@ class DiscogsPlugin(MetadataSourcePlugin): """Iterates through a discogs result and builds up the artist fields. Does not contribute to artist_sort as Discogs does not define that. - - :param artists: A list of Discogs Artist objects - - :param album_artist: If building an album artist, - we need to account for the album_artist anv parameter. - :return an ArtistInfo dictionary. """ info: ArtistInfo = { "artist": "", @@ -420,7 +423,7 @@ class DiscogsPlugin(MetadataSourcePlugin): for a in given_artists: # Get the artist name name = self.strip_disambiguation(a["name"]) - discogs_id = a["id"] + discogs_id = str(a["id"]) anv = a.get("anv", name) role = a.get("role", "").lower() # Check if the artist is Various @@ -621,63 +624,41 @@ class DiscogsPlugin(MetadataSourcePlugin): self, clean_tracklist: list[Track], albumartistinfo: ArtistInfo, - ) -> tuple[ - list[TrackInfo], - dict[int, str], - int, - list[str], - list[str], - list[str | None], - list[str | None], - ]: + ) -> TracklistInfo: # Distinct works and intra-work divisions, as defined by index tracks. - tracks: list[TrackInfo] = [] - mediums: list[str | None] = [] - medium_indices: list[str | None] = [] - index_tracks = {} - index = 0 - divisions: list[str] = [] - next_divisions: list[str] = [] + t = TracklistInfo() for track in clean_tracklist: # Only real tracks have `position`. Otherwise, it's an index track. if track["position"]: - index += 1 - if next_divisions: + t.index += 1 + if t.next_divisions: # End of a block of index tracks: update the current # divisions. - divisions += next_divisions - del next_divisions[:] + t.divisions += t.next_divisions + del t.next_divisions[:] track_info, medium, medium_index = self.get_track_info( - track, index, divisions, albumartistinfo + track, t.index, t.divisions, albumartistinfo ) track_info.track_alt = track["position"] - tracks.append(track_info) + t.tracks.append(track_info) if medium: - mediums.append(medium) + t.mediums.append(medium) else: - mediums.append(None) + t.mediums.append(None) if medium_index: - medium_indices.append(medium_index) + t.medium_indices.append(medium_index) else: - medium_indices.append(None) + t.medium_indices.append(None) else: - next_divisions.append(track["title"]) + t.next_divisions.append(track["title"]) # We expect new levels of division at the beginning of the # tracklist (and possibly elsewhere). try: - divisions.pop() + t.divisions.pop() except IndexError: pass - index_tracks[index + 1] = track["title"] - return ( - tracks, - index_tracks, - index, - divisions, - next_divisions, - mediums, - medium_indices, - ) + t.index_tracks[t.index + 1] = track["title"] + return t def get_tracks( self, @@ -694,18 +675,9 @@ class DiscogsPlugin(MetadataSourcePlugin): self._log.debug("{}", traceback.format_exc()) self._log.error("uncaught exception in coalesce_tracks: {}", exc) clean_tracklist = tracklist - processed = self._process_clean_tracklist( + t: TracklistInfo = self._process_clean_tracklist( clean_tracklist, albumartistinfo ) - ( - tracks, - index_tracks, - index, - divisions, - next_divisions, - mediums, - medium_indices, - ) = processed # Fix up medium and medium_index for each track. Discogs position is # unreliable, but tracks are in order. medium = None @@ -714,22 +686,24 @@ class DiscogsPlugin(MetadataSourcePlugin): # If a medium has two sides (ie. vinyl or cassette), each pair of # consecutive sides should belong to the same medium. - if all([medium is not None for medium in mediums]): - m = sorted({medium.lower() if medium else "" for medium in mediums}) + if all([medium is not None for medium in t.mediums]): + m = sorted( + {medium.lower() if medium else "" for medium in t.mediums} + ) # If all track.medium are single consecutive letters, assume it is # a 2-sided medium. if "".join(m) in ascii_lowercase: sides_per_medium = 2 - for i, track in enumerate(tracks): + for i, track in enumerate(t.tracks): # Handle special case where a different medium does not indicate a # new disc, when there is no medium_index and the ordinal of medium # is not sequential. For example, I, II, III, IV, V. Assume these # are the track index, not the medium. # side_count is the number of mediums or medium sides (in the case # of two-sided mediums) that were seen before. - medium_str = mediums[i] - medium_index = medium_indices[i] + medium_str = t.mediums[i] + medium_index = t.medium_indices[i] medium_is_index = ( medium_str and not medium_index @@ -760,15 +734,15 @@ class DiscogsPlugin(MetadataSourcePlugin): # Get `disctitle` from Discogs index tracks. Assume that an index track # before the first track of each medium is a disc title. - for track in tracks: + for track in t.tracks: if track.medium_index == 1: - if track.index in index_tracks: - disctitle = index_tracks[track.index] + if track.index in t.index_tracks: + disctitle = t.index_tracks[track.index] else: disctitle = None track.disctitle = disctitle - return tracks + return t.tracks def coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]: """Pre-process a tracklist, merging subtracks into a single track. The diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index c11148c13..a34b8aee4 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -410,8 +410,11 @@ class DGAlbumInfoTest(BeetsTestCase): d = DiscogsPlugin().get_album_info(release) assert d.artist == "ARTIST NAME & OTHER ARTIST" assert d.artists == ["ARTIST NAME", "OTHER ARTIST"] + assert d.artists_ids == ["321", "321"] assert d.tracks[0].artist == "TEST ARTIST" assert d.tracks[0].artists == ["TEST ARTIST"] + assert d.tracks[0].artist_id == "11146" + assert d.tracks[0].artists_ids == ["11146"] assert d.label == "LABEL NAME" def test_strip_disambiguation_false(self): @@ -460,7 +463,7 @@ class DGAlbumInfoTest(BeetsTestCase): @pytest.mark.parametrize( "track_artist_anv,track_artist,track_artists", [ - (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PEFORMER"]), + (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PERFORMER"]), (True, "VARIATION Feat. VARIATION", ["VARIATION", "VARIATION"]), ], ) @@ -480,7 +483,7 @@ class DGAlbumInfoTest(BeetsTestCase): ( False, "ARTIST Feat. PERFORMER", - ["ARTIST", "PEFORMER"], + ["ARTIST", "PERFORMER"], "ARTIST & SOLOIST", ["ARTIST", "SOLOIST"], ), @@ -551,9 +554,14 @@ def test_anv( config["discogs"]["anv"]["artist_credit"] = artist_credit_anv r = DiscogsPlugin().get_album_info(release) assert r.artist == album_artist + assert r.albumartists == album_artists assert r.artist_credit == album_artist_credit + assert r.albumartist_credit == album_artist_credit + assert r.albumartists_credit == album_artists_credit assert r.tracks[0].artist == track_artist + assert r.tracks[0].artists == track_artists assert r.tracks[0].artist_credit == track_artist_credit + assert r.tracks[0].artists_credit == track_artists_credit @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) @@ -590,6 +598,7 @@ def test_anv_album_artist(): assert r.artists == ["ARTIST"] assert r.albumartist == "ARTIST" assert r.albumartist_credit == "ARTIST" + assert r.albumartist_id == "321" assert r.albumartists == ["ARTIST"] assert r.albumartists_credit == ["ARTIST"] assert r.artist_credit == "ARTIST" From 08a2c248b9153f2863b69dcaba5cff38d36b27ee Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Mon, 15 Dec 2025 14:05:29 -0800 Subject: [PATCH 29/40] Fix handling of commas and semicolons in artist join --- beetsplug/discogs.py | 6 +++++- test/plugins/test_discogs.py | 27 ++++++++++++++------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0a86d245d..dcf5bd77a 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -476,7 +476,11 @@ class DiscogsPlugin(MetadataSourcePlugin): base = artist else: if join: - base += f" {join} " + join = join.strip() + if join in ";,": + base += f"{join} " + else: + base += f" {join} " else: base += ", " base += artist diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index a34b8aee4..ca3959f19 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -464,14 +464,14 @@ class DGAlbumInfoTest(BeetsTestCase): "track_artist_anv,track_artist,track_artists", [ (False, "ARTIST Feat. PERFORMER", ["ARTIST", "PERFORMER"]), - (True, "VARIATION Feat. VARIATION", ["VARIATION", "VARIATION"]), + (True, "ART Feat. PERF", ["ART", "PERF"]), ], ) @pytest.mark.parametrize( "album_artist_anv,album_artist,album_artists", [ - (False, "ARTIST & SOLOIST", ["ARTIST", "SOLOIST"]), - (True, "VARIATION & VARIATION", ["VARIATION", "VARIATION"]), + (False, "DRUMMER, ARTIST & SOLOIST", ["DRUMMER", "ARTIST", "SOLOIST"]), + (True, "DRUM, ARTY & SOLO", ["DRUM", "ARTY", "SOLO"]), ], ) @pytest.mark.parametrize( @@ -484,15 +484,15 @@ class DGAlbumInfoTest(BeetsTestCase): False, "ARTIST Feat. PERFORMER", ["ARTIST", "PERFORMER"], - "ARTIST & SOLOIST", - ["ARTIST", "SOLOIST"], + "DRUMMER, ARTIST & SOLOIST", + ["DRUMMER", "ARTIST", "SOLOIST"], ), ( True, - "VARIATION Feat. VARIATION", - ["VARIATION", "VARIATION"], - "VARIATION & VARIATION", - ["VARIATION", "VARIATION"], + "ART Feat. PERF", + ["ART", "PERF"], + "DRUM, ARTY & SOLO", + ["DRUM", "ARTY", "SOLO"], ), ], ) @@ -524,7 +524,7 @@ def test_anv( { "name": "ARTIST", "tracks": "", - "anv": "VARIATION", + "anv": "ART", "id": 11146, } ], @@ -532,15 +532,16 @@ def test_anv( { "name": "PERFORMER", "role": "Featuring", - "anv": "VARIATION", + "anv": "PERF", "id": 787, } ], } ], "artists": [ - {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321, "join": "&"}, - {"name": "SOLOIST", "anv": "VARIATION", "id": 445, "join": ""}, + {"name": "DRUMMER", "anv": "DRUM", "id": 445, "join": ", "}, + {"name": "ARTIST (4)", "anv": "ARTY", "id": 321, "join": "&"}, + {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, ], "title": "title", } From 459fd39768b5c8e8531799881b3596b8ca98fd8e Mon Sep 17 00:00:00 2001 From: Henry Date: Fri, 19 Dec 2025 18:24:26 -0800 Subject: [PATCH 30/40] Fix behavior when ANV does not exist --- beetsplug/discogs.py | 4 ++- test/plugins/test_discogs.py | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index dcf5bd77a..8887b8811 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -424,7 +424,9 @@ class DiscogsPlugin(MetadataSourcePlugin): # Get the artist name name = self.strip_disambiguation(a["name"]) discogs_id = str(a["id"]) - anv = a.get("anv", name) + anv = a.get("anv", "") + if not anv: + anv = name role = a.get("role", "").lower() # Check if the artist is Various if name.lower() == "various": diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index ca3959f19..393dc4cc0 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -565,6 +565,56 @@ def test_anv( assert r.tracks[0].artists_credit == track_artists_credit +@pytest.mark.parametrize("artist_anv", [True, False]) +@pytest.mark.parametrize("albumartist_anv", [True, False]) +@pytest.mark.parametrize("artistcredit_anv", [True, False]) +@patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) +def test_anv_no_variation(artist_anv, albumartist_anv, artistcredit_anv): + """Test behavior when there is no ANV but the anv field is set""" + data = { + "id": 123, + "uri": "https://www.discogs.com/release/123456-something", + "tracklist": [ + { + "title": "track", + "position": "A", + "type_": "track", + "duration": "5:44", + "artists": [ + { + "name": "PERFORMER", + "tracks": "", + "anv": "", + "id": 1, + } + ], + } + ], + "artists": [ + {"name": "ARTIST", "anv": "", "id": 2}, + ], + "title": "title", + } + release = Bag( + data=data, + title=data["title"], + artists=[Bag(data=d) for d in data["artists"]], + ) + config["discogs"]["anv"]["album_artist"] = albumartist_anv + config["discogs"]["anv"]["artist"] = artist_anv + config["discogs"]["anv"]["artist_credit"] = artistcredit_anv + r = DiscogsPlugin().get_album_info(release) + assert r.artist == "ARTIST" + assert r.albumartists == ["ARTIST"] + assert r.artist_credit == "ARTIST" + assert r.albumartist_credit == "ARTIST" + assert r.albumartists_credit == ["ARTIST"] + assert r.tracks[0].artist == "PERFORMER" + assert r.tracks[0].artists == ["PERFORMER"] + assert r.tracks[0].artist_credit == "PERFORMER" + assert r.tracks[0].artists_credit == ["PERFORMER"] + + @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_anv_album_artist(): """Test using artist name variations when the album artist From 2d406a3ca5812992598d836a959814aca0a5ab54 Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Tue, 30 Dec 2025 11:49:20 -0800 Subject: [PATCH 31/40] Add comments, clean up types. --- beetsplug/discogs.py | 172 +++++++++++++++++++++-------------- test/plugins/test_discogs.py | 2 +- 2 files changed, 104 insertions(+), 70 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 8887b8811..f38384751 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -123,15 +123,14 @@ class AlbumArtistInfo(ArtistInfo): albumartist_id: str -class TracklistInfo: - def __init__(self): - self.index: int = 0 - self.index_tracks: dict[int, str] = {} - self.tracks: list[TrackInfo] = [] - self.divisions: list[str] = [] - self.next_divisions: list[str] = [] - self.mediums: list[str | None] = [] - self.medium_indices: list[str | None] = [] +class TracklistInfo(TypedDict): + index: int + index_tracks: dict[int, str] + tracks: list[TrackInfo] + divisions: list[str] + next_divisions: list[str] + mediums: list[str | None] + medium_indices: list[str | None] class DiscogsPlugin(MetadataSourcePlugin): @@ -374,8 +373,8 @@ class DiscogsPlugin(MetadataSourcePlugin): artist, artist_id = self.get_artist(artist_list, join_key="join") return self.strip_disambiguation(artist), artist_id - def build_albumartistinfo(self, artists: list[Artist]) -> AlbumArtistInfo: - info = self.build_artistinfo(artists, album_artist=True) + def _build_albumartistinfo(self, artists: list[Artist]) -> AlbumArtistInfo: + info = self._build_artistinfo(artists, for_album_artist=True) albumartist: AlbumArtistInfo = { **info, "albumartist": info["artist"], @@ -386,11 +385,11 @@ class DiscogsPlugin(MetadataSourcePlugin): } return albumartist - def build_artistinfo( + def _build_artistinfo( self, given_artists: list[Artist], given_info: ArtistInfo | None = None, - album_artist: bool = False, + for_album_artist: bool = False, ) -> ArtistInfo: """Iterates through a discogs result and builds up the artist fields. Does not contribute to @@ -404,19 +403,18 @@ class DiscogsPlugin(MetadataSourcePlugin): "artist_credit": "", "artists_credit": [], } + # If starting information is given we start from there + # Often used for cases with album artists. + # Deepcopy is used to prevent unintentional + # extra modifications if given_info: info = copy.deepcopy(given_info) - - a_anv: bool = self.config["anv"]["artist"].get(bool) - ac_anv: bool = self.config["anv"]["artist_credit"].get(bool) - aa_anv: bool = self.config["anv"]["album_artist"].get(bool) - feat_str: str = f" {self.config['featured_string'].get(str)} " - artist = "" artist_anv = "" artists: list[str] = [] artists_anv: list[str] = [] + feat_str: str = f" {self.config['featured_string'].as_str()} " join = "" featured_flag = False # Iterate through building the artist strings @@ -424,15 +422,13 @@ class DiscogsPlugin(MetadataSourcePlugin): # Get the artist name name = self.strip_disambiguation(a["name"]) discogs_id = str(a["id"]) - anv = a.get("anv", "") - if not anv: - anv = name + anv = a.get("anv", "") or name role = a.get("role", "").lower() # Check if the artist is Various if name.lower() == "various": name = config["va_name"].as_str() anv = name - + # If the artist is listed as featured if "featuring" in role: if not featured_flag: artist += feat_str @@ -440,10 +436,16 @@ class DiscogsPlugin(MetadataSourcePlugin): artist += name artist_anv += anv featured_flag = True + # Set the featured_flag + # to indicate we no longer need to + # prefix the marker for a featured + # artist else: artist = self._join_artist(artist, name, join) artist_anv = self._join_artist(artist_anv, anv, join) elif role and "featuring" not in role: + # Current artists that are in the credits + # and are not credited as featuring are ignored. continue else: artist = self._join_artist(artist, name, join) @@ -456,21 +458,9 @@ class DiscogsPlugin(MetadataSourcePlugin): info["artists_ids"].append(discogs_id) # Update join for the next artist join = a.get("join", "") - # Assign fields as necessary - if (a_anv and not album_artist) or (aa_anv and album_artist): - info["artist"] += artist_anv - info["artists"] += artists_anv - else: - info["artist"] += artist - info["artists"] += artists - - if ac_anv: - info["artist_credit"] += artist_anv - info["artists_credit"] += artists_anv - else: - info["artist_credit"] += artist - info["artists_credit"] += artists - return info + return self._assign_anv( + info, artist, artists, artist_anv, artists_anv, for_album_artist + ) def _join_artist(self, base: str, artist: str, join: str) -> str: # Expand the artist field @@ -488,6 +478,42 @@ class DiscogsPlugin(MetadataSourcePlugin): base += artist return base + def _assign_anv( + self, + info: ArtistInfo, + artist: str, + artists: list[str], + artist_anv: str, + artists_anv: list[str], + for_album_artist: bool, + ) -> ArtistInfo: + """Assign artist and variation fields based on + configuration settings. + """ + # Fetch configuration options for artist name variations + use_artist_anv: bool = self.config["anv"]["artist"].get(bool) + use_artistcredit_anv: bool = self.config["anv"]["artist_credit"].get( + bool + ) + use_albumartist_anv: bool = self.config["anv"]["album_artist"].get(bool) + + if (use_artist_anv and not for_album_artist) or ( + use_albumartist_anv and for_album_artist + ): + info["artist"] += artist_anv + info["artists"] += artists_anv + else: + info["artist"] += artist + info["artists"] += artists + + if use_artistcredit_anv: + info["artist_credit"] += artist_anv + info["artists_credit"] += artists_anv + else: + info["artist_credit"] += artist + info["artists_credit"] += artists + return info + def get_album_info(self, result: Release) -> AlbumInfo | None: """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet @@ -518,7 +544,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_data = [a.data for a in result.artists] # Information for the album artist - albumartist: AlbumArtistInfo = self.build_albumartistinfo(artist_data) + albumartist: AlbumArtistInfo = self._build_albumartistinfo(artist_data) album = re.sub(r" +", " ", result.title) album_id = result.data["id"] @@ -527,13 +553,13 @@ class DiscogsPlugin(MetadataSourcePlugin): # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. tracks = self.get_tracks( - result.data["tracklist"], self.build_artistinfo(artist_data) + result.data["tracklist"], self._build_artistinfo(artist_data) ) # Extract information for the optional AlbumInfo fields, if possible. va = albumartist["albumartist"] == config["va_name"].as_str() year = result.data.get("year") - mediums = [t.medium for t in tracks] + 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")) @@ -632,38 +658,46 @@ class DiscogsPlugin(MetadataSourcePlugin): albumartistinfo: ArtistInfo, ) -> TracklistInfo: # Distinct works and intra-work divisions, as defined by index tracks. - t = TracklistInfo() + t: TracklistInfo = { + "index": 0, + "index_tracks": {}, + "tracks": [], + "divisions": [], + "next_divisions": [], + "mediums": [], + "medium_indices": [], + } for track in clean_tracklist: # Only real tracks have `position`. Otherwise, it's an index track. if track["position"]: - t.index += 1 - if t.next_divisions: + t["index"] += 1 + if t["next_divisions"]: # End of a block of index tracks: update the current # divisions. - t.divisions += t.next_divisions - del t.next_divisions[:] + t["divisions"] += t["next_divisions"] + del t["next_divisions"][:] track_info, medium, medium_index = self.get_track_info( - track, t.index, t.divisions, albumartistinfo + track, t["index"], t["divisions"], albumartistinfo ) track_info.track_alt = track["position"] - t.tracks.append(track_info) + t["tracks"].append(track_info) if medium: - t.mediums.append(medium) + t["mediums"].append(medium) else: - t.mediums.append(None) + t["mediums"].append(None) if medium_index: - t.medium_indices.append(medium_index) + t["medium_indices"].append(medium_index) else: - t.medium_indices.append(None) + t["medium_indices"].append(None) else: - t.next_divisions.append(track["title"]) + t["next_divisions"].append(track["title"]) # We expect new levels of division at the beginning of the # tracklist (and possibly elsewhere). try: - t.divisions.pop() + t["divisions"].pop() except IndexError: pass - t.index_tracks[t.index + 1] = track["title"] + t["index_tracks"][t["index"] + 1] = track["title"] return t def get_tracks( @@ -673,13 +707,13 @@ class DiscogsPlugin(MetadataSourcePlugin): ) -> list[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: - clean_tracklist: list[Track] = self.coalesce_tracks(tracklist) + clean_tracklist: list[Track] = self._coalesce_tracks(tracklist) except Exception as exc: # FIXME: this is an extra precaution for making sure there are no # side effects after #2222. It should be removed after further # testing. self._log.debug("{}", traceback.format_exc()) - self._log.error("uncaught exception in coalesce_tracks: {}", exc) + self._log.error("uncaught exception in _coalesce_tracks: {}", exc) clean_tracklist = tracklist t: TracklistInfo = self._process_clean_tracklist( clean_tracklist, albumartistinfo @@ -692,24 +726,24 @@ class DiscogsPlugin(MetadataSourcePlugin): # If a medium has two sides (ie. vinyl or cassette), each pair of # consecutive sides should belong to the same medium. - if all([medium is not None for medium in t.mediums]): + if all([medium is not None for medium in t["mediums"]]): m = sorted( - {medium.lower() if medium else "" for medium in t.mediums} + {medium.lower() if medium else "" for medium in t["mediums"]} ) # If all track.medium are single consecutive letters, assume it is # a 2-sided medium. if "".join(m) in ascii_lowercase: sides_per_medium = 2 - for i, track in enumerate(t.tracks): + for i, track in enumerate(t["tracks"]): # Handle special case where a different medium does not indicate a # new disc, when there is no medium_index and the ordinal of medium # is not sequential. For example, I, II, III, IV, V. Assume these # are the track index, not the medium. # side_count is the number of mediums or medium sides (in the case # of two-sided mediums) that were seen before. - medium_str = t.mediums[i] - medium_index = t.medium_indices[i] + medium_str = t["mediums"][i] + medium_index = t["medium_indices"][i] medium_is_index = ( medium_str and not medium_index @@ -740,17 +774,17 @@ class DiscogsPlugin(MetadataSourcePlugin): # Get `disctitle` from Discogs index tracks. Assume that an index track # before the first track of each medium is a disc title. - for track in t.tracks: + for track in t["tracks"]: if track.medium_index == 1: - if track.index in t.index_tracks: - disctitle = t.index_tracks[track.index] + if track.index in t["index_tracks"]: + disctitle = t["index_tracks"][track.index] else: disctitle = None track.disctitle = disctitle - return t.tracks + return t["tracks"] - def coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]: + def _coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]: """Pre-process a tracklist, merging subtracks into a single track. The title for the merged track is the one from the previous index track, if present; otherwise it is a combination of the subtracks titles. @@ -867,13 +901,13 @@ class DiscogsPlugin(MetadataSourcePlugin): # If artists are found on the track, we will use those instead if artists := track.get("artists", []): - artistinfo = self.build_artistinfo(artists) + artistinfo = self._build_artistinfo(artists) length = self.get_track_length(track["duration"]) # Add featured artists if extraartists := track.get("extraartists", []): - artistinfo = self.build_artistinfo(extraartists, artistinfo) + artistinfo = self._build_artistinfo(extraartists, artistinfo) return ( TrackInfo( diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 393dc4cc0..3beed628a 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -761,7 +761,7 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_va_buildartistinfo(given_artists, expected_info, config_va_name): config["va_name"] = config_va_name - assert DiscogsPlugin().build_artistinfo(given_artists) == expected_info + assert DiscogsPlugin()._build_artistinfo(given_artists) == expected_info @pytest.mark.parametrize( From 0e48c65171de864b9a340490e2fd50341c74d32e Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Wed, 7 Jan 2026 12:17:36 -0800 Subject: [PATCH 32/40] Clarify variable in _process_clean_tracklist --- beetsplug/discogs.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index f38384751..9357b633d 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -658,7 +658,7 @@ class DiscogsPlugin(MetadataSourcePlugin): albumartistinfo: ArtistInfo, ) -> TracklistInfo: # Distinct works and intra-work divisions, as defined by index tracks. - t: TracklistInfo = { + info: TracklistInfo = { "index": 0, "index_tracks": {}, "tracks": [], @@ -670,35 +670,35 @@ class DiscogsPlugin(MetadataSourcePlugin): for track in clean_tracklist: # Only real tracks have `position`. Otherwise, it's an index track. if track["position"]: - t["index"] += 1 - if t["next_divisions"]: + info["index"] += 1 + if info["next_divisions"]: # End of a block of index tracks: update the current # divisions. - t["divisions"] += t["next_divisions"] - del t["next_divisions"][:] + info["divisions"] += info["next_divisions"] + del info["next_divisions"][:] track_info, medium, medium_index = self.get_track_info( - track, t["index"], t["divisions"], albumartistinfo + track, info["index"], info["divisions"], albumartistinfo ) track_info.track_alt = track["position"] - t["tracks"].append(track_info) + info["tracks"].append(track_info) if medium: - t["mediums"].append(medium) + info["mediums"].append(medium) else: - t["mediums"].append(None) + info["mediums"].append(None) if medium_index: - t["medium_indices"].append(medium_index) + info["medium_indices"].append(medium_index) else: - t["medium_indices"].append(None) + info["medium_indices"].append(None) else: - t["next_divisions"].append(track["title"]) + info["next_divisions"].append(track["title"]) # We expect new levels of division at the beginning of the # tracklist (and possibly elsewhere). try: - t["divisions"].pop() + info["divisions"].pop() except IndexError: pass - t["index_tracks"][t["index"] + 1] = track["title"] - return t + info["index_tracks"][info["index"] + 1] = track["title"] + return info def get_tracks( self, From 59e7c591729f2112ce172bcf0b8d2cb3636b5419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Thu, 8 Jan 2026 17:28:30 +0000 Subject: [PATCH 33/40] Move building logic to dataclasses --- beetsplug/discogs.py | 442 ++++++++++++++++------------------- test/plugins/test_discogs.py | 38 ++- 2 files changed, 219 insertions(+), 261 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 9357b633d..d2de50091 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -18,7 +18,6 @@ python3-discogs-client library. from __future__ import annotations -import copy import http.client import json import os @@ -26,6 +25,7 @@ import re import socket import time import traceback +from dataclasses import asdict, dataclass, field from functools import cache from string import ascii_lowercase from typing import TYPE_CHECKING @@ -115,14 +115,6 @@ class ArtistInfo(TypedDict): artists_ids: list[str] -class AlbumArtistInfo(ArtistInfo): - albumartist: str - albumartists: list[str] - albumartist_credit: str - albumartists_credit: list[str] - albumartist_id: str - - class TracklistInfo(TypedDict): index: int index_tracks: dict[int, str] @@ -133,6 +125,184 @@ class TracklistInfo(TypedDict): medium_indices: list[str | None] +@dataclass +class ArtistState: + artist: str = "" + artists: list[str] = field(default_factory=list) + artist_credit: str = "" + artists_credit: list[str] = field(default_factory=list) + artist_id: str = "" + artists_ids: list[str] = field(default_factory=list) + + @property + def info(self) -> ArtistInfo: + return asdict(self) # type: ignore[return-value] + + def clone(self) -> ArtistState: + return ArtistState(**asdict(self)) + + @classmethod + def build( + cls, + plugin: DiscogsPlugin, + given_artists: list[Artist], + given_state: ArtistState | None = None, + for_album_artist: bool = False, + ) -> ArtistState: + """Iterates through a discogs result and builds + up the artist fields. Does not contribute to + artist_sort as Discogs does not define that. + """ + state = given_state.clone() if given_state else cls() + + artist = "" + artist_anv = "" + artists: list[str] = [] + artists_anv: list[str] = [] + + feat_str: str = f" {plugin.config['featured_string'].as_str()} " + join = "" + featured_flag = False + for a in given_artists: + name = plugin.strip_disambiguation(a["name"]) + discogs_id = str(a["id"]) + anv = a.get("anv", "") or name + role = a.get("role", "").lower() + if name.lower() == "various": + name = config["va_name"].as_str() + anv = name + if "featuring" in role: + if not featured_flag: + artist += feat_str + artist_anv += feat_str + artist += name + artist_anv += anv + featured_flag = True + else: + artist = cls.join_artist(artist, name, join) + artist_anv = cls.join_artist(artist_anv, anv, join) + elif role and "featuring" not in role: + continue + else: + artist = cls.join_artist(artist, name, join) + artist_anv = cls.join_artist(artist_anv, anv, join) + artists.append(name) + artists_anv.append(anv) + if not state.artist_id: + state.artist_id = discogs_id + state.artists_ids.append(discogs_id) + join = a.get("join", "") + cls._assign_anv( + plugin, + state, + artist, + artists, + artist_anv, + artists_anv, + for_album_artist, + ) + return state + + @staticmethod + def join_artist(base: str, artist: str, join: str) -> str: + # Expand the artist field + if not base: + base = artist + else: + if join: + join = join.strip() + if join in ";,": + base += f"{join} " + else: + base += f" {join} " + else: + base += ", " + base += artist + return base + + @staticmethod + def _assign_anv( + plugin: DiscogsPlugin, + state: ArtistState, + artist: str, + artists: list[str], + artist_anv: str, + artists_anv: list[str], + for_album_artist: bool, + ) -> None: + """Assign artist and variation fields based on + configuration settings. + """ + use_artist_anv: bool = plugin.config["anv"]["artist"].get(bool) + use_artistcredit_anv: bool = plugin.config["anv"]["artist_credit"].get( + bool + ) + use_albumartist_anv: bool = plugin.config["anv"]["album_artist"].get( + bool + ) + + if (use_artist_anv and not for_album_artist) or ( + use_albumartist_anv and for_album_artist + ): + state.artist += artist_anv + state.artists += artists_anv + else: + state.artist += artist + state.artists += artists + + if use_artistcredit_anv: + state.artist_credit += artist_anv + state.artists_credit += artists_anv + else: + state.artist_credit += artist + state.artists_credit += artists + + +@dataclass +class TracklistState: + index: int = 0 + index_tracks: dict[int, str] = field(default_factory=dict) + tracks: list[TrackInfo] = field(default_factory=list) + divisions: list[str] = field(default_factory=list) + next_divisions: list[str] = field(default_factory=list) + mediums: list[str | None] = field(default_factory=list) + medium_indices: list[str | None] = field(default_factory=list) + + @property + def info(self) -> TracklistInfo: + return asdict(self) # type: ignore[return-value] + + @classmethod + def build( + cls, + plugin: DiscogsPlugin, + clean_tracklist: list[Track], + albumartistinfo: ArtistState, + ) -> TracklistState: + state = cls() + for track in clean_tracklist: + if track["position"]: + state.index += 1 + if state.next_divisions: + state.divisions += state.next_divisions + state.next_divisions.clear() + track_info, medium, medium_index = plugin.get_track_info( + track, state.index, state.divisions, albumartistinfo + ) + track_info.track_alt = track["position"] + state.tracks.append(track_info) + state.mediums.append(medium or None) + state.medium_indices.append(medium_index or None) + else: + state.next_divisions.append(track["title"]) + try: + state.divisions.pop() + except IndexError: + pass + state.index_tracks[state.index + 1] = track["title"] + return state + + class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): super().__init__() @@ -354,166 +524,6 @@ class DiscogsPlugin(MetadataSourcePlugin): return media, albumtype - def get_artist_with_anv( - self, artists: list[Artist], use_anv: bool = False - ) -> tuple[str, str | None]: - """Iterates through a discogs result, fetching data - if the artist anv is to be used, maps that to the name. - Calls the parent class get_artist method.""" - artist_list: list[dict[str | int, str]] = [] - for artist_data in artists: - a: dict[str | int, str] = { - "name": artist_data["name"], - "id": artist_data["id"], - "join": artist_data.get("join", ""), - } - if use_anv and (anv := artist_data.get("anv", "")): - a["name"] = anv - artist_list.append(a) - artist, artist_id = self.get_artist(artist_list, join_key="join") - return self.strip_disambiguation(artist), artist_id - - def _build_albumartistinfo(self, artists: list[Artist]) -> AlbumArtistInfo: - info = self._build_artistinfo(artists, for_album_artist=True) - albumartist: AlbumArtistInfo = { - **info, - "albumartist": info["artist"], - "albumartist_id": info["artist_id"], - "albumartists": info["artists"], - "albumartist_credit": info["artist_credit"], - "albumartists_credit": info["artists_credit"], - } - return albumartist - - def _build_artistinfo( - self, - given_artists: list[Artist], - given_info: ArtistInfo | None = None, - for_album_artist: bool = False, - ) -> ArtistInfo: - """Iterates through a discogs result and builds - up the artist fields. Does not contribute to - artist_sort as Discogs does not define that. - """ - info: ArtistInfo = { - "artist": "", - "artist_id": "", - "artists": [], - "artists_ids": [], - "artist_credit": "", - "artists_credit": [], - } - # If starting information is given we start from there - # Often used for cases with album artists. - # Deepcopy is used to prevent unintentional - # extra modifications - if given_info: - info = copy.deepcopy(given_info) - artist = "" - artist_anv = "" - artists: list[str] = [] - artists_anv: list[str] = [] - - feat_str: str = f" {self.config['featured_string'].as_str()} " - join = "" - featured_flag = False - # Iterate through building the artist strings - for a in given_artists: - # Get the artist name - name = self.strip_disambiguation(a["name"]) - discogs_id = str(a["id"]) - anv = a.get("anv", "") or name - role = a.get("role", "").lower() - # Check if the artist is Various - if name.lower() == "various": - name = config["va_name"].as_str() - anv = name - # If the artist is listed as featured - if "featuring" in role: - if not featured_flag: - artist += feat_str - artist_anv += feat_str - artist += name - artist_anv += anv - featured_flag = True - # Set the featured_flag - # to indicate we no longer need to - # prefix the marker for a featured - # artist - else: - artist = self._join_artist(artist, name, join) - artist_anv = self._join_artist(artist_anv, anv, join) - elif role and "featuring" not in role: - # Current artists that are in the credits - # and are not credited as featuring are ignored. - continue - else: - artist = self._join_artist(artist, name, join) - artist_anv = self._join_artist(artist_anv, anv, join) - artists.append(name) - artists_anv.append(anv) - # Only the first ID is set for the singular field - if not info["artist_id"]: - info["artist_id"] = discogs_id - info["artists_ids"].append(discogs_id) - # Update join for the next artist - join = a.get("join", "") - return self._assign_anv( - info, artist, artists, artist_anv, artists_anv, for_album_artist - ) - - def _join_artist(self, base: str, artist: str, join: str) -> str: - # Expand the artist field - if not base: - base = artist - else: - if join: - join = join.strip() - if join in ";,": - base += f"{join} " - else: - base += f" {join} " - else: - base += ", " - base += artist - return base - - def _assign_anv( - self, - info: ArtistInfo, - artist: str, - artists: list[str], - artist_anv: str, - artists_anv: list[str], - for_album_artist: bool, - ) -> ArtistInfo: - """Assign artist and variation fields based on - configuration settings. - """ - # Fetch configuration options for artist name variations - use_artist_anv: bool = self.config["anv"]["artist"].get(bool) - use_artistcredit_anv: bool = self.config["anv"]["artist_credit"].get( - bool - ) - use_albumartist_anv: bool = self.config["anv"]["album_artist"].get(bool) - - if (use_artist_anv and not for_album_artist) or ( - use_albumartist_anv and for_album_artist - ): - info["artist"] += artist_anv - info["artists"] += artists_anv - else: - info["artist"] += artist - info["artists"] += artists - - if use_artistcredit_anv: - info["artist_credit"] += artist_anv - info["artists_credit"] += artists_anv - else: - info["artist_credit"] += artist - info["artists_credit"] += artists - return info - def get_album_info(self, result: Release) -> AlbumInfo | None: """Returns an AlbumInfo object for a discogs Release object.""" # Explicitly reload the `Release` fields, as they might not be yet @@ -544,7 +554,9 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_data = [a.data for a in result.artists] # Information for the album artist - albumartist: AlbumArtistInfo = self._build_albumartistinfo(artist_data) + albumartist = ArtistState.build( + self, artist_data, for_album_artist=True + ) album = re.sub(r" +", " ", result.title) album_id = result.data["id"] @@ -553,11 +565,11 @@ class DiscogsPlugin(MetadataSourcePlugin): # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. tracks = self.get_tracks( - result.data["tracklist"], self._build_artistinfo(artist_data) + result.data["tracklist"], ArtistState.build(self, artist_data) ) # Extract information for the optional AlbumInfo fields, if possible. - va = albumartist["albumartist"] == config["va_name"].as_str() + va = albumartist.artist == config["va_name"].as_str() year = result.data.get("year") mediums = [t["medium"] for t in tracks] country = result.data.get("country") @@ -612,7 +624,7 @@ class DiscogsPlugin(MetadataSourcePlugin): return AlbumInfo( album=album, album_id=album_id, - **albumartist, # Unpacks values to satisfy the keyword arguments + **albumartist.info, # Unpacks values to satisfy the keyword arguments tracks=tracks, albumtype=albumtype, va=va, @@ -630,7 +642,7 @@ class DiscogsPlugin(MetadataSourcePlugin): data_url=data_url, discogs_albumid=discogs_albumid, discogs_labelid=labelid, - discogs_artistid=albumartist["albumartist_id"], + discogs_artistid=albumartist.artist_id, cover_art_url=cover_art_url, ) @@ -652,58 +664,10 @@ class DiscogsPlugin(MetadataSourcePlugin): else: return None - def _process_clean_tracklist( - self, - clean_tracklist: list[Track], - albumartistinfo: ArtistInfo, - ) -> TracklistInfo: - # Distinct works and intra-work divisions, as defined by index tracks. - info: TracklistInfo = { - "index": 0, - "index_tracks": {}, - "tracks": [], - "divisions": [], - "next_divisions": [], - "mediums": [], - "medium_indices": [], - } - for track in clean_tracklist: - # Only real tracks have `position`. Otherwise, it's an index track. - if track["position"]: - info["index"] += 1 - if info["next_divisions"]: - # End of a block of index tracks: update the current - # divisions. - info["divisions"] += info["next_divisions"] - del info["next_divisions"][:] - track_info, medium, medium_index = self.get_track_info( - track, info["index"], info["divisions"], albumartistinfo - ) - track_info.track_alt = track["position"] - info["tracks"].append(track_info) - if medium: - info["mediums"].append(medium) - else: - info["mediums"].append(None) - if medium_index: - info["medium_indices"].append(medium_index) - else: - info["medium_indices"].append(None) - else: - info["next_divisions"].append(track["title"]) - # We expect new levels of division at the beginning of the - # tracklist (and possibly elsewhere). - try: - info["divisions"].pop() - except IndexError: - pass - info["index_tracks"][info["index"] + 1] = track["title"] - return info - def get_tracks( self, tracklist: list[Track], - albumartistinfo: ArtistInfo, + albumartistinfo: ArtistState, ) -> list[TrackInfo]: """Returns a list of TrackInfo objects for a discogs tracklist.""" try: @@ -715,9 +679,7 @@ class DiscogsPlugin(MetadataSourcePlugin): self._log.debug("{}", traceback.format_exc()) self._log.error("uncaught exception in _coalesce_tracks: {}", exc) clean_tracklist = tracklist - t: TracklistInfo = self._process_clean_tracklist( - clean_tracklist, albumartistinfo - ) + t = TracklistState.build(self, clean_tracklist, albumartistinfo) # Fix up medium and medium_index for each track. Discogs position is # unreliable, but tracks are in order. medium = None @@ -726,24 +688,24 @@ class DiscogsPlugin(MetadataSourcePlugin): # If a medium has two sides (ie. vinyl or cassette), each pair of # consecutive sides should belong to the same medium. - if all([medium is not None for medium in t["mediums"]]): + if all([medium is not None for medium in t.mediums]): m = sorted( - {medium.lower() if medium else "" for medium in t["mediums"]} + {medium.lower() if medium else "" for medium in t.mediums} ) # If all track.medium are single consecutive letters, assume it is # a 2-sided medium. if "".join(m) in ascii_lowercase: sides_per_medium = 2 - for i, track in enumerate(t["tracks"]): + for i, track in enumerate(t.tracks): # Handle special case where a different medium does not indicate a # new disc, when there is no medium_index and the ordinal of medium # is not sequential. For example, I, II, III, IV, V. Assume these # are the track index, not the medium. # side_count is the number of mediums or medium sides (in the case # of two-sided mediums) that were seen before. - medium_str = t["mediums"][i] - medium_index = t["medium_indices"][i] + medium_str = t.mediums[i] + medium_index = t.medium_indices[i] medium_is_index = ( medium_str and not medium_index @@ -774,15 +736,15 @@ class DiscogsPlugin(MetadataSourcePlugin): # Get `disctitle` from Discogs index tracks. Assume that an index track # before the first track of each medium is a disc title. - for track in t["tracks"]: + for track in t.tracks: if track.medium_index == 1: - if track.index in t["index_tracks"]: - disctitle = t["index_tracks"][track.index] + if track.index in t.index_tracks: + disctitle = t.index_tracks[track.index] else: disctitle = None track.disctitle = disctitle - return t["tracks"] + return t.tracks def _coalesce_tracks(self, raw_tracklist: list[Track]) -> list[Track]: """Pre-process a tracklist, merging subtracks into a single track. The @@ -885,11 +847,11 @@ class DiscogsPlugin(MetadataSourcePlugin): track: Track, index: int, divisions: list[str], - albumartistinfo: ArtistInfo, + albumartistinfo: ArtistState, ) -> tuple[TrackInfo, str | None, str | None]: """Returns a TrackInfo object for a discogs track.""" - artistinfo = albumartistinfo.copy() + artistinfo = albumartistinfo.clone() title = track["title"] if self.config["index_tracks"]: @@ -901,19 +863,19 @@ class DiscogsPlugin(MetadataSourcePlugin): # If artists are found on the track, we will use those instead if artists := track.get("artists", []): - artistinfo = self._build_artistinfo(artists) + artistinfo = ArtistState.build(self, artists) length = self.get_track_length(track["duration"]) # Add featured artists if extraartists := track.get("extraartists", []): - artistinfo = self._build_artistinfo(extraartists, artistinfo) + artistinfo = ArtistState.build(self, extraartists, artistinfo) return ( TrackInfo( title=title, track_id=track_id, - **artistinfo, + **artistinfo.info, length=length, index=index, ), diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 3beed628a..35bd15c9e 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -21,7 +21,7 @@ import pytest from beets import config from beets.test._common import Bag from beets.test.helper import BeetsTestCase, capture_log -from beetsplug.discogs import DiscogsPlugin +from beetsplug.discogs import ArtistState, DiscogsPlugin @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) @@ -555,10 +555,9 @@ def test_anv( config["discogs"]["anv"]["artist_credit"] = artist_credit_anv r = DiscogsPlugin().get_album_info(release) assert r.artist == album_artist - assert r.albumartists == album_artists + assert r.artists == album_artists assert r.artist_credit == album_artist_credit - assert r.albumartist_credit == album_artist_credit - assert r.albumartists_credit == album_artists_credit + assert r.artists_credit == album_artists_credit assert r.tracks[0].artist == track_artist assert r.tracks[0].artists == track_artists assert r.tracks[0].artist_credit == track_artist_credit @@ -605,10 +604,9 @@ def test_anv_no_variation(artist_anv, albumartist_anv, artistcredit_anv): config["discogs"]["anv"]["artist_credit"] = artistcredit_anv r = DiscogsPlugin().get_album_info(release) assert r.artist == "ARTIST" - assert r.albumartists == ["ARTIST"] + assert r.artists == ["ARTIST"] assert r.artist_credit == "ARTIST" - assert r.albumartist_credit == "ARTIST" - assert r.albumartists_credit == ["ARTIST"] + assert r.artists_credit == ["ARTIST"] assert r.tracks[0].artist == "PERFORMER" assert r.tracks[0].artists == ["PERFORMER"] assert r.tracks[0].artist_credit == "PERFORMER" @@ -647,12 +645,8 @@ def test_anv_album_artist(): r = DiscogsPlugin().get_album_info(release) assert r.artist == "ARTIST" assert r.artists == ["ARTIST"] - assert r.albumartist == "ARTIST" - assert r.albumartist_credit == "ARTIST" - assert r.albumartist_id == "321" - assert r.albumartists == ["ARTIST"] - assert r.albumartists_credit == ["ARTIST"] assert r.artist_credit == "ARTIST" + assert r.artist_id == "321" assert r.artists_credit == ["ARTIST"] assert r.tracks[0].artist == "VARIATION" assert r.tracks[0].artists == ["VARIATION"] @@ -705,14 +699,14 @@ def test_anv_album_artist(): def test_parse_featured_artists(track, expected_artist, expected_artists): """Tests the plugins ability to parse a featured artist. Ignores artists that are not listed as featured.""" - artistinfo = { - "artist": "ARTIST", - "artist_id": "1", - "artists": ["ARTIST"], - "artists_ids": ["1"], - "artist_credit": "ARTIST", - "artists_credit": ["ARTIST"], - } + artistinfo = ArtistState( + artist="ARTIST", + artist_id="1", + artists=["ARTIST"], + artists_ids=["1"], + artist_credit="ARTIST", + artists_credit=["ARTIST"], + ) t, _, _ = DiscogsPlugin().get_track_info(track, 1, 1, artistinfo) assert t.artist == expected_artist assert t.artists == expected_artists @@ -761,7 +755,9 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) def test_va_buildartistinfo(given_artists, expected_info, config_va_name): config["va_name"] = config_va_name - assert DiscogsPlugin()._build_artistinfo(given_artists) == expected_info + assert ( + ArtistState.build(DiscogsPlugin(), given_artists).info == expected_info + ) @pytest.mark.parametrize( From b3183a73e0b2527f85214f8fac70520ccb6a40ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 Jan 2026 00:52:34 +0000 Subject: [PATCH 34/40] Simplify building artist --- beetsplug/discogs.py | 236 ++++++++++++++++------------------- test/plugins/test_discogs.py | 15 +-- 2 files changed, 110 insertions(+), 141 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index d2de50091..a7206c7d6 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -26,9 +26,9 @@ import socket import time import traceback from dataclasses import asdict, dataclass, field -from functools import cache +from functools import cache, cached_property from string import ascii_lowercase -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, NamedTuple import confuse from discogs_client import Client, Master, Release @@ -127,135 +127,111 @@ class TracklistInfo(TypedDict): @dataclass class ArtistState: - artist: str = "" - artists: list[str] = field(default_factory=list) - artist_credit: str = "" - artists_credit: list[str] = field(default_factory=list) - artist_id: str = "" - artists_ids: list[str] = field(default_factory=list) + class ValidArtist(NamedTuple): + id: str + name: str + credit: str + join: str + is_feat: bool + + def get_artist(self, property_name: str) -> str: + return getattr(self, property_name) + ( + {",": ", ", "": ""}.get(self.join, f" {self.join} ") + ) + + raw_artists: list[Artist] + use_anv: bool + use_credit_anv: bool + featured_string: str + should_strip_disambiguation: bool @property def info(self) -> ArtistInfo: - return asdict(self) # type: ignore[return-value] + return {k: getattr(self, k) for k in ArtistInfo.__annotations__} # type: ignore[return-value] - def clone(self) -> ArtistState: - return ArtistState(**asdict(self)) + def strip_disambiguation(self, text: str) -> str: + """Removes discogs specific disambiguations from a string. + Turns 'Label Name (5)' to 'Label Name' or 'Artist (1) & Another Artist (2)' + to 'Artist & Another Artist'. Does nothing if strip_disambiguation is False.""" + if self.should_strip_disambiguation: + return DISAMBIGUATION_RE.sub("", text) + return text + + @cached_property + def valid_artists(self) -> list[ValidArtist]: + va_name = config["va_name"].as_str() + return [ + self.ValidArtist( + str(a["id"]), + self.strip_disambiguation(anv if self.use_anv else name), + self.strip_disambiguation(anv if self.use_credit_anv else name), + a["join"], + is_feat, + ) + for a in self.raw_artists + if ( + (name := va_name if a["name"] == "Various" else a["name"]) + and (anv := a["anv"] or name) + and ( + (is_feat := ("featuring" in a["role"].lower())) + or not a["role"] + ) + ) + ] + + @property + def artists_ids(self) -> list[str]: + return [a.id for a in self.valid_artists] + + @property + def artist_id(self) -> str: + return self.artists_ids[0] + + @property + def artists(self) -> list[str]: + return [a.name for a in self.valid_artists] + + @property + def artists_credit(self) -> list[str]: + return [a.credit for a in self.valid_artists] + + @property + def artist(self) -> str: + return self.join_artists("name") + + @property + def artist_credit(self) -> str: + return self.join_artists("credit") + + def join_artists(self, property_name: str) -> str: + non_featured = [a for a in self.valid_artists if not a.is_feat] + featured = [a for a in self.valid_artists if a.is_feat] + + artist = "".join(a.get_artist(property_name) for a in non_featured) + if featured: + if "feat" not in artist: + artist += f" {self.featured_string} " + + artist += ", ".join(a.get_artist(property_name) for a in featured) + + return artist @classmethod - def build( + def from_plugin( cls, plugin: DiscogsPlugin, - given_artists: list[Artist], - given_state: ArtistState | None = None, + artists: list[Artist], for_album_artist: bool = False, ) -> ArtistState: - """Iterates through a discogs result and builds - up the artist fields. Does not contribute to - artist_sort as Discogs does not define that. - """ - state = given_state.clone() if given_state else cls() - - artist = "" - artist_anv = "" - artists: list[str] = [] - artists_anv: list[str] = [] - - feat_str: str = f" {plugin.config['featured_string'].as_str()} " - join = "" - featured_flag = False - for a in given_artists: - name = plugin.strip_disambiguation(a["name"]) - discogs_id = str(a["id"]) - anv = a.get("anv", "") or name - role = a.get("role", "").lower() - if name.lower() == "various": - name = config["va_name"].as_str() - anv = name - if "featuring" in role: - if not featured_flag: - artist += feat_str - artist_anv += feat_str - artist += name - artist_anv += anv - featured_flag = True - else: - artist = cls.join_artist(artist, name, join) - artist_anv = cls.join_artist(artist_anv, anv, join) - elif role and "featuring" not in role: - continue - else: - artist = cls.join_artist(artist, name, join) - artist_anv = cls.join_artist(artist_anv, anv, join) - artists.append(name) - artists_anv.append(anv) - if not state.artist_id: - state.artist_id = discogs_id - state.artists_ids.append(discogs_id) - join = a.get("join", "") - cls._assign_anv( - plugin, - state, - artist, + return cls( artists, - artist_anv, - artists_anv, - for_album_artist, + plugin.config["anv"][ + "album_artist" if for_album_artist else "artist" + ].get(bool), + plugin.config["anv"]["artist_credit"].get(bool), + plugin.config["featured_string"].as_str(), + plugin.config["strip_disambiguation"].get(bool), ) - return state - - @staticmethod - def join_artist(base: str, artist: str, join: str) -> str: - # Expand the artist field - if not base: - base = artist - else: - if join: - join = join.strip() - if join in ";,": - base += f"{join} " - else: - base += f" {join} " - else: - base += ", " - base += artist - return base - - @staticmethod - def _assign_anv( - plugin: DiscogsPlugin, - state: ArtistState, - artist: str, - artists: list[str], - artist_anv: str, - artists_anv: list[str], - for_album_artist: bool, - ) -> None: - """Assign artist and variation fields based on - configuration settings. - """ - use_artist_anv: bool = plugin.config["anv"]["artist"].get(bool) - use_artistcredit_anv: bool = plugin.config["anv"]["artist_credit"].get( - bool - ) - use_albumartist_anv: bool = plugin.config["anv"]["album_artist"].get( - bool - ) - - if (use_artist_anv and not for_album_artist) or ( - use_albumartist_anv and for_album_artist - ): - state.artist += artist_anv - state.artists += artists_anv - else: - state.artist += artist - state.artists += artists - - if use_artistcredit_anv: - state.artist_credit += artist_anv - state.artists_credit += artists_anv - else: - state.artist_credit += artist - state.artists_credit += artists @dataclass @@ -554,7 +530,7 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_data = [a.data for a in result.artists] # Information for the album artist - albumartist = ArtistState.build( + albumartist = ArtistState.from_plugin( self, artist_data, for_album_artist=True ) @@ -565,7 +541,7 @@ class DiscogsPlugin(MetadataSourcePlugin): # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. tracks = self.get_tracks( - result.data["tracklist"], ArtistState.build(self, artist_data) + result.data["tracklist"], ArtistState.from_plugin(self, artist_data) ) # Extract information for the optional AlbumInfo fields, if possible. @@ -851,8 +827,6 @@ class DiscogsPlugin(MetadataSourcePlugin): ) -> tuple[TrackInfo, str | None, str | None]: """Returns a TrackInfo object for a discogs track.""" - artistinfo = albumartistinfo.clone() - title = track["title"] if self.config["index_tracks"]: prefix = ", ".join(divisions) @@ -861,15 +835,15 @@ class DiscogsPlugin(MetadataSourcePlugin): track_id = None medium, medium_index, _ = self.get_track_index(track["position"]) - # If artists are found on the track, we will use those instead - if artists := track.get("artists", []): - artistinfo = ArtistState.build(self, artists) - length = self.get_track_length(track["duration"]) - - # Add featured artists - if extraartists := track.get("extraartists", []): - artistinfo = ArtistState.build(self, extraartists, artistinfo) + # If artists are found on the track, we will use those instead + artistinfo = ArtistState.from_plugin( + self, + [ + *(track.get("artists") or albumartistinfo.raw_artists), + *track.get("extraartists", []), + ], + ) return ( TrackInfo( diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 35bd15c9e..54ff8dd75 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -699,15 +699,9 @@ def test_anv_album_artist(): def test_parse_featured_artists(track, expected_artist, expected_artists): """Tests the plugins ability to parse a featured artist. Ignores artists that are not listed as featured.""" - artistinfo = ArtistState( - artist="ARTIST", - artist_id="1", - artists=["ARTIST"], - artists_ids=["1"], - artist_credit="ARTIST", - artists_credit=["ARTIST"], - ) - t, _, _ = DiscogsPlugin().get_track_info(track, 1, 1, artistinfo) + plugin = DiscogsPlugin() + artistinfo = ArtistState.from_plugin(plugin, [_artist("ARTIST")]) + t, _, _ = plugin.get_track_info(track, 1, 1, artistinfo) assert t.artist == expected_artist assert t.artists == expected_artists @@ -756,7 +750,8 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): def test_va_buildartistinfo(given_artists, expected_info, config_va_name): config["va_name"] = config_va_name assert ( - ArtistState.build(DiscogsPlugin(), given_artists).info == expected_info + ArtistState.from_plugin(DiscogsPlugin(), given_artists).info + == expected_info ) From 7d83a68bddd7847977e5b4824aa0986fddfa5b8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Fri, 9 Jan 2026 00:53:41 +0000 Subject: [PATCH 35/40] Ensure all fields in artist dicts in tests --- test/plugins/test_discogs.py | 112 ++++++++++++++--------------------- 1 file changed, 43 insertions(+), 69 deletions(-) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 54ff8dd75..66cbe9371 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -24,6 +24,18 @@ from beets.test.helper import BeetsTestCase, capture_log from beetsplug.discogs import ArtistState, DiscogsPlugin +def _artist(name: str, **kwargs): + return { + "id": 1, + "name": name, + "join": "", + "role": "", + "anv": "", + "tracks": "", + "resource_url": "", + } | kwargs + + @patch("beetsplug.discogs.DiscogsPlugin.setup", Mock()) class DGAlbumInfoTest(BeetsTestCase): def _make_release(self, tracks=None): @@ -35,9 +47,7 @@ class DGAlbumInfoTest(BeetsTestCase): "uri": "https://www.discogs.com/release/release/13633721", "title": "ALBUM TITLE", "year": "3001", - "artists": [ - {"name": "ARTIST NAME", "id": "ARTIST ID", "join": ","} - ], + "artists": [_artist("ARTIST NAME", id="ARTIST ID", join=",")], "formats": [ { "descriptions": ["FORMAT DESC 1", "FORMAT DESC 2"], @@ -325,7 +335,7 @@ class DGAlbumInfoTest(BeetsTestCase): "id": 123, "uri": "https://www.discogs.com/release/123456-something", "tracklist": [self._make_track("A", "1", "01:01")], - "artists": [{"name": "ARTIST NAME", "id": 321, "join": ""}], + "artists": [_artist("ARTIST NAME", id=321)], "title": "TITLE", } release = Bag( @@ -385,14 +395,12 @@ class DGAlbumInfoTest(BeetsTestCase): "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} - ], + "artists": [_artist("TEST ARTIST (5)", id=11146)], } ], "artists": [ - {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, - {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + _artist("ARTIST NAME (2)", id=321, join="&"), + _artist("OTHER ARTIST (5)", id=321), ], "title": "title", "labels": [ @@ -429,14 +437,12 @@ class DGAlbumInfoTest(BeetsTestCase): "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - {"name": "TEST ARTIST (5)", "tracks": "", "id": 11146} - ], + "artists": [_artist("TEST ARTIST (5)", id=11146)], } ], "artists": [ - {"name": "ARTIST NAME (2)", "id": 321, "join": "&"}, - {"name": "OTHER ARTIST (5)", "id": 321, "join": ""}, + _artist("ARTIST NAME (2)", id=321, join="&"), + _artist("OTHER ARTIST (5)", id=321), ], "title": "title", "labels": [ @@ -520,28 +526,21 @@ def test_anv( "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - { - "name": "ARTIST", - "tracks": "", - "anv": "ART", - "id": 11146, - } - ], + "artists": [_artist("ARTIST", id=11146, anv="ART")], "extraartists": [ - { - "name": "PERFORMER", - "role": "Featuring", - "anv": "PERF", - "id": 787, - } + _artist( + "PERFORMER", + id=787, + role="Featuring", + anv="PERF", + ) ], } ], "artists": [ - {"name": "DRUMMER", "anv": "DRUM", "id": 445, "join": ", "}, - {"name": "ARTIST (4)", "anv": "ARTY", "id": 321, "join": "&"}, - {"name": "SOLOIST", "anv": "SOLO", "id": 445, "join": ""}, + _artist("DRUMMER", id=445, anv="DRUM", join=", "), + _artist("ARTIST (4)", id=321, anv="ARTY", join="&"), + _artist("SOLOIST", id=445, anv="SOLO"), ], "title": "title", } @@ -579,19 +578,10 @@ def test_anv_no_variation(artist_anv, albumartist_anv, artistcredit_anv): "position": "A", "type_": "track", "duration": "5:44", - "artists": [ - { - "name": "PERFORMER", - "tracks": "", - "anv": "", - "id": 1, - } - ], + "artists": [_artist("PERFORMER", id=1)], } ], - "artists": [ - {"name": "ARTIST", "anv": "", "id": 2}, - ], + "artists": [_artist("ARTIST", id=2)], "title": "title", } release = Bag( @@ -629,9 +619,7 @@ def test_anv_album_artist(): "duration": "5:44", } ], - "artists": [ - {"name": "ARTIST (4)", "anv": "VARIATION", "id": 321}, - ], + "artists": [_artist("ARTIST (4)", id=321, anv="VARIATION")], "title": "title", } release = Bag( @@ -664,33 +652,19 @@ def test_anv_album_artist(): "position": "1", "duration": "5:00", "artists": [ - {"name": "NEW ARTIST", "tracks": "", "id": 11146}, - {"name": "VOCALIST", "tracks": "", "id": 344, "join": "&"}, + _artist("NEW ARTIST", id=11146, join="&"), + _artist("VOCALIST", id=344, join="feat."), ], "extraartists": [ - { - "name": "SOLOIST", - "id": 3, - "role": "Featuring", - }, - { - "name": "PERFORMER (1)", - "id": 5, - "role": "Other Role, Featuring", - }, - { - "name": "RANDOM", - "id": 8, - "role": "Written-By", - }, - { - "name": "MUSICIAN", - "id": 10, - "role": "Featuring [Uncredited]", - }, + _artist("SOLOIST", id=3, role="Featuring"), + _artist( + "PERFORMER (1)", id=5, role="Other Role, Featuring" + ), + _artist("RANDOM", id=8, role="Written-By"), + _artist("MUSICIAN", id=10, role="Featuring [Uncredited]"), ], }, - "NEW ARTIST, VOCALIST Feat. SOLOIST, PERFORMER, MUSICIAN", + "NEW ARTIST & VOCALIST feat. SOLOIST, PERFORMER, MUSICIAN", ["NEW ARTIST", "VOCALIST", "SOLOIST", "PERFORMER", "MUSICIAN"], ), ], @@ -733,7 +707,7 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): "given_artists,expected_info,config_va_name", [ ( - [{"name": "Various", "id": "1"}], + [_artist("Various")], { "artist": "VARIOUS ARTISTS", "artist_id": "1", From 5523ca94a293fa64cd56659133afded333c9a8e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= Date: Sat, 10 Jan 2026 02:28:18 +0000 Subject: [PATCH 36/40] Document ArtistState --- beetsplug/discogs.py | 61 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index a7206c7d6..017969e27 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -127,7 +127,23 @@ class TracklistInfo(TypedDict): @dataclass class ArtistState: + """Represent Discogs artist credits. + + This object centralizes the plugin's policy for which Discogs artist fields + to prefer (name vs. ANV), how to treat 'Various', how to format join + phrases, and how to separate featured artists. It exposes both per-artist + components and fully joined strings for common tag targets like 'artist' and + 'artist_credit'. + """ + class ValidArtist(NamedTuple): + """A normalized, render-ready artist entry extracted from Discogs data. + + Instances represent the subset of Discogs artist information needed for + tagging, including the join token following the artist and whether the + entry is considered a featured appearance. + """ + id: str name: str credit: str @@ -135,9 +151,14 @@ class ArtistState: is_feat: bool def get_artist(self, property_name: str) -> str: - return getattr(self, property_name) + ( - {",": ", ", "": ""}.get(self.join, f" {self.join} ") - ) + """Return the requested display field with its trailing join token. + + The join token is normalized so commas become ', ' and other join + phrases are surrounded with spaces, producing a single fragment that + can be concatenated to form a full artist string. + """ + join = {",": ", ", "": ""}.get(self.join, f" {self.join} ") + return f"{getattr(self, property_name)}{join}" raw_artists: list[Artist] use_anv: bool @@ -147,25 +168,38 @@ class ArtistState: @property def info(self) -> ArtistInfo: + """Expose the state in the shape expected by downstream tag mapping.""" return {k: getattr(self, k) for k in ArtistInfo.__annotations__} # type: ignore[return-value] def strip_disambiguation(self, text: str) -> str: - """Removes discogs specific disambiguations from a string. - Turns 'Label Name (5)' to 'Label Name' or 'Artist (1) & Another Artist (2)' - to 'Artist & Another Artist'. Does nothing if strip_disambiguation is False.""" + """Strip Discogs disambiguation suffixes from an artist or label string. + + This removes Discogs-specific numeric suffixes like 'Name (5)' and can + be applied to multi-artist strings as well (e.g., 'A (1) & B (2)'). When + the feature is disabled, the input is returned unchanged. + """ if self.should_strip_disambiguation: return DISAMBIGUATION_RE.sub("", text) return text @cached_property def valid_artists(self) -> list[ValidArtist]: + """Build the ordered, filtered list of artists used for rendering. + + The resulting list normalizes Discogs entries by: + - substituting the configured 'Various Artists' name when Discogs uses + 'Various' + - choosing between name and ANV according to plugin settings + - excluding non-empty roles unless they indicate a featured appearance + - capturing join tokens so the original credit formatting is preserved + """ va_name = config["va_name"].as_str() return [ self.ValidArtist( str(a["id"]), self.strip_disambiguation(anv if self.use_anv else name), self.strip_disambiguation(anv if self.use_credit_anv else name), - a["join"], + a["join"].strip(), is_feat, ) for a in self.raw_artists @@ -181,29 +215,42 @@ class ArtistState: @property def artists_ids(self) -> list[str]: + """Return Discogs artist IDs for all valid artists, preserving order.""" return [a.id for a in self.valid_artists] @property def artist_id(self) -> str: + """Return the primary Discogs artist ID.""" return self.artists_ids[0] @property def artists(self) -> list[str]: + """Return the per-artist display names used for the 'artist' field.""" return [a.name for a in self.valid_artists] @property def artists_credit(self) -> list[str]: + """Return the per-artist display names used for the credit field.""" return [a.credit for a in self.valid_artists] @property def artist(self) -> str: + """Return the fully rendered artist string using display names.""" return self.join_artists("name") @property def artist_credit(self) -> str: + """Return the fully rendered artist credit string.""" return self.join_artists("credit") def join_artists(self, property_name: str) -> str: + """Render a single artist string with join phrases and featured artists. + + Non-featured artists are concatenated using their join tokens. Featured + artists are appended after the configured 'featured' marker, preserving + Discogs order while keeping featured credits separate from the main + artist string. + """ non_featured = [a for a in self.valid_artists if not a.is_feat] featured = [a for a in self.valid_artists if a.is_feat] From 2cfd1df3c12c72b711ce47392f3951c1355ce585 Mon Sep 17 00:00:00 2001 From: Henry Oberholtzer Date: Mon, 12 Jan 2026 12:03:36 -0800 Subject: [PATCH 37/40] Split discogs.py into smaller and more workable modules. --- beetsplug/{discogs.py => discogs/__init__.py} | 287 +----------------- beetsplug/discogs/states.py | 232 ++++++++++++++ beetsplug/discogs/types.py | 67 ++++ beetsplug/discogs/utils.py | 47 +++ 4 files changed, 353 insertions(+), 280 deletions(-) rename beetsplug/{discogs.py => discogs/__init__.py} (73%) create mode 100644 beetsplug/discogs/states.py create mode 100644 beetsplug/discogs/types.py create mode 100644 beetsplug/discogs/utils.py diff --git a/beetsplug/discogs.py b/beetsplug/discogs/__init__.py similarity index 73% rename from beetsplug/discogs.py rename to beetsplug/discogs/__init__.py index 017969e27..23e2267df 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs/__init__.py @@ -18,23 +18,18 @@ python3-discogs-client library. from __future__ import annotations -import http.client import json import os import re -import socket import time import traceback -from dataclasses import asdict, dataclass, field -from functools import cache, cached_property +from functools import cache from string import ascii_lowercase -from typing import TYPE_CHECKING, NamedTuple +from typing import TYPE_CHECKING import confuse from discogs_client import Client, Master, Release from discogs_client.exceptions import DiscogsAPIError -from requests.exceptions import ConnectionError -from typing_extensions import NotRequired, TypedDict import beets import beets.ui @@ -43,288 +38,20 @@ from beets.autotag.distance import string_dist from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin +from .states import ArtistState, TracklistState +from .utils import CONNECTION_ERRORS, DISAMBIGUATION_RE, TRACK_INDEX_RE + if TYPE_CHECKING: from collections.abc import Callable, Iterable, Sequence from beets.library import Item + from .types import ReleaseFormat, Track + USER_AGENT = f"beets/{beets.__version__} +https://beets.io/" API_KEY = "rAzVUQYRaoFjeBjyWuWZ" API_SECRET = "plxtUTqoCzwxZpqdPysCwGuBSmZNdZVy" -# Exceptions that discogs_client should really handle but does not. -CONNECTION_ERRORS = ( - ConnectionError, - socket.error, - http.client.HTTPException, - ValueError, # JSON decoding raises a ValueError. - DiscogsAPIError, -) - - -TRACK_INDEX_RE = re.compile( - r""" - (.*?) # medium: everything before medium_index. - (\d*?) # medium_index: a number at the end of - # `position`, except if followed by a subtrack index. - # subtrack_index: can only be matched if medium - # or medium_index have been matched, and can be - ( - (?<=\w)\.[\w]+ # a dot followed by a string (A.1, 2.A) - | (?<=\d)[A-Z]+ # a string that follows a number (1A, B2a) - )? - """, - re.VERBOSE, -) - -DISAMBIGUATION_RE = re.compile(r" \(\d+\)") - - -class ReleaseFormat(TypedDict): - name: str - qty: int - descriptions: list[str] | None - - -class Artist(TypedDict): - name: str - anv: str - join: str - role: str - tracks: str - id: str - resource_url: str - - -class Track(TypedDict): - position: str - type_: str - title: str - duration: str - artists: list[Artist] - extraartists: NotRequired[list[Artist]] - sub_tracks: NotRequired[list[Track]] - - -class ArtistInfo(TypedDict): - artist: str - artists: list[str] - artist_credit: str - artists_credit: list[str] - artist_id: str - artists_ids: list[str] - - -class TracklistInfo(TypedDict): - index: int - index_tracks: dict[int, str] - tracks: list[TrackInfo] - divisions: list[str] - next_divisions: list[str] - mediums: list[str | None] - medium_indices: list[str | None] - - -@dataclass -class ArtistState: - """Represent Discogs artist credits. - - This object centralizes the plugin's policy for which Discogs artist fields - to prefer (name vs. ANV), how to treat 'Various', how to format join - phrases, and how to separate featured artists. It exposes both per-artist - components and fully joined strings for common tag targets like 'artist' and - 'artist_credit'. - """ - - class ValidArtist(NamedTuple): - """A normalized, render-ready artist entry extracted from Discogs data. - - Instances represent the subset of Discogs artist information needed for - tagging, including the join token following the artist and whether the - entry is considered a featured appearance. - """ - - id: str - name: str - credit: str - join: str - is_feat: bool - - def get_artist(self, property_name: str) -> str: - """Return the requested display field with its trailing join token. - - The join token is normalized so commas become ', ' and other join - phrases are surrounded with spaces, producing a single fragment that - can be concatenated to form a full artist string. - """ - join = {",": ", ", "": ""}.get(self.join, f" {self.join} ") - return f"{getattr(self, property_name)}{join}" - - raw_artists: list[Artist] - use_anv: bool - use_credit_anv: bool - featured_string: str - should_strip_disambiguation: bool - - @property - def info(self) -> ArtistInfo: - """Expose the state in the shape expected by downstream tag mapping.""" - return {k: getattr(self, k) for k in ArtistInfo.__annotations__} # type: ignore[return-value] - - def strip_disambiguation(self, text: str) -> str: - """Strip Discogs disambiguation suffixes from an artist or label string. - - This removes Discogs-specific numeric suffixes like 'Name (5)' and can - be applied to multi-artist strings as well (e.g., 'A (1) & B (2)'). When - the feature is disabled, the input is returned unchanged. - """ - if self.should_strip_disambiguation: - return DISAMBIGUATION_RE.sub("", text) - return text - - @cached_property - def valid_artists(self) -> list[ValidArtist]: - """Build the ordered, filtered list of artists used for rendering. - - The resulting list normalizes Discogs entries by: - - substituting the configured 'Various Artists' name when Discogs uses - 'Various' - - choosing between name and ANV according to plugin settings - - excluding non-empty roles unless they indicate a featured appearance - - capturing join tokens so the original credit formatting is preserved - """ - va_name = config["va_name"].as_str() - return [ - self.ValidArtist( - str(a["id"]), - self.strip_disambiguation(anv if self.use_anv else name), - self.strip_disambiguation(anv if self.use_credit_anv else name), - a["join"].strip(), - is_feat, - ) - for a in self.raw_artists - if ( - (name := va_name if a["name"] == "Various" else a["name"]) - and (anv := a["anv"] or name) - and ( - (is_feat := ("featuring" in a["role"].lower())) - or not a["role"] - ) - ) - ] - - @property - def artists_ids(self) -> list[str]: - """Return Discogs artist IDs for all valid artists, preserving order.""" - return [a.id for a in self.valid_artists] - - @property - def artist_id(self) -> str: - """Return the primary Discogs artist ID.""" - return self.artists_ids[0] - - @property - def artists(self) -> list[str]: - """Return the per-artist display names used for the 'artist' field.""" - return [a.name for a in self.valid_artists] - - @property - def artists_credit(self) -> list[str]: - """Return the per-artist display names used for the credit field.""" - return [a.credit for a in self.valid_artists] - - @property - def artist(self) -> str: - """Return the fully rendered artist string using display names.""" - return self.join_artists("name") - - @property - def artist_credit(self) -> str: - """Return the fully rendered artist credit string.""" - return self.join_artists("credit") - - def join_artists(self, property_name: str) -> str: - """Render a single artist string with join phrases and featured artists. - - Non-featured artists are concatenated using their join tokens. Featured - artists are appended after the configured 'featured' marker, preserving - Discogs order while keeping featured credits separate from the main - artist string. - """ - non_featured = [a for a in self.valid_artists if not a.is_feat] - featured = [a for a in self.valid_artists if a.is_feat] - - artist = "".join(a.get_artist(property_name) for a in non_featured) - if featured: - if "feat" not in artist: - artist += f" {self.featured_string} " - - artist += ", ".join(a.get_artist(property_name) for a in featured) - - return artist - - @classmethod - def from_plugin( - cls, - plugin: DiscogsPlugin, - artists: list[Artist], - for_album_artist: bool = False, - ) -> ArtistState: - return cls( - artists, - plugin.config["anv"][ - "album_artist" if for_album_artist else "artist" - ].get(bool), - plugin.config["anv"]["artist_credit"].get(bool), - plugin.config["featured_string"].as_str(), - plugin.config["strip_disambiguation"].get(bool), - ) - - -@dataclass -class TracklistState: - index: int = 0 - index_tracks: dict[int, str] = field(default_factory=dict) - tracks: list[TrackInfo] = field(default_factory=list) - divisions: list[str] = field(default_factory=list) - next_divisions: list[str] = field(default_factory=list) - mediums: list[str | None] = field(default_factory=list) - medium_indices: list[str | None] = field(default_factory=list) - - @property - def info(self) -> TracklistInfo: - return asdict(self) # type: ignore[return-value] - - @classmethod - def build( - cls, - plugin: DiscogsPlugin, - clean_tracklist: list[Track], - albumartistinfo: ArtistState, - ) -> TracklistState: - state = cls() - for track in clean_tracklist: - if track["position"]: - state.index += 1 - if state.next_divisions: - state.divisions += state.next_divisions - state.next_divisions.clear() - track_info, medium, medium_index = plugin.get_track_info( - track, state.index, state.divisions, albumartistinfo - ) - track_info.track_alt = track["position"] - state.tracks.append(track_info) - state.mediums.append(medium or None) - state.medium_indices.append(medium_index or None) - else: - state.next_divisions.append(track["title"]) - try: - state.divisions.pop() - except IndexError: - pass - state.index_tracks[state.index + 1] = track["title"] - return state - class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): diff --git a/beetsplug/discogs/states.py b/beetsplug/discogs/states.py new file mode 100644 index 000000000..265c92c4e --- /dev/null +++ b/beetsplug/discogs/states.py @@ -0,0 +1,232 @@ +# This file is part of beets. +# Copyright 2025, Sarunas Nejus, Henry Oberholtzer. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +"""Dataclasses for managing artist credits and tracklists from Discogs.""" + +from __future__ import annotations + +from dataclasses import asdict, dataclass, field +from functools import cached_property +from typing import TYPE_CHECKING, NamedTuple + +from beets import config + +from .types import Artist, ArtistInfo, Track, TracklistInfo +from .utils import DISAMBIGUATION_RE + +if TYPE_CHECKING: + from beets.autotag.hooks import TrackInfo + + from . import DiscogsPlugin + + +@dataclass +class ArtistState: + """Represent Discogs artist credits. + + This object centralizes the plugin's policy for which Discogs artist fields + to prefer (name vs. ANV), how to treat 'Various', how to format join + phrases, and how to separate featured artists. It exposes both per-artist + components and fully joined strings for common tag targets like 'artist' and + 'artist_credit'. + """ + + class ValidArtist(NamedTuple): + """A normalized, render-ready artist entry extracted from Discogs data. + + Instances represent the subset of Discogs artist information needed for + tagging, including the join token following the artist and whether the + entry is considered a featured appearance. + """ + + id: str + name: str + credit: str + join: str + is_feat: bool + + def get_artist(self, property_name: str) -> str: + """Return the requested display field with its trailing join token. + + The join token is normalized so commas become ', ' and other join + phrases are surrounded with spaces, producing a single fragment that + can be concatenated to form a full artist string. + """ + join = {",": ", ", "": ""}.get(self.join, f" {self.join} ") + return f"{getattr(self, property_name)}{join}" + + raw_artists: list[Artist] + use_anv: bool + use_credit_anv: bool + featured_string: str + should_strip_disambiguation: bool + + @property + def info(self) -> ArtistInfo: + """Expose the state in the shape expected by downstream tag mapping.""" + return {k: getattr(self, k) for k in ArtistInfo.__annotations__} # type: ignore[return-value] + + def strip_disambiguation(self, text: str) -> str: + """Strip Discogs disambiguation suffixes from an artist or label string. + + This removes Discogs-specific numeric suffixes like 'Name (5)' and can + be applied to multi-artist strings as well (e.g., 'A (1) & B (2)'). When + the feature is disabled, the input is returned unchanged. + """ + if self.should_strip_disambiguation: + return DISAMBIGUATION_RE.sub("", text) + return text + + @cached_property + def valid_artists(self) -> list[ValidArtist]: + """Build the ordered, filtered list of artists used for rendering. + + The resulting list normalizes Discogs entries by: + - substituting the configured 'Various Artists' name when Discogs uses + 'Various' + - choosing between name and ANV according to plugin settings + - excluding non-empty roles unless they indicate a featured appearance + - capturing join tokens so the original credit formatting is preserved + """ + va_name = config["va_name"].as_str() + return [ + self.ValidArtist( + str(a["id"]), + self.strip_disambiguation(anv if self.use_anv else name), + self.strip_disambiguation(anv if self.use_credit_anv else name), + a["join"].strip(), + is_feat, + ) + for a in self.raw_artists + if ( + (name := va_name if a["name"] == "Various" else a["name"]) + and (anv := a["anv"] or name) + and ( + (is_feat := ("featuring" in a["role"].lower())) + or not a["role"] + ) + ) + ] + + @property + def artists_ids(self) -> list[str]: + """Return Discogs artist IDs for all valid artists, preserving order.""" + return [a.id for a in self.valid_artists] + + @property + def artist_id(self) -> str: + """Return the primary Discogs artist ID.""" + return self.artists_ids[0] + + @property + def artists(self) -> list[str]: + """Return the per-artist display names used for the 'artist' field.""" + return [a.name for a in self.valid_artists] + + @property + def artists_credit(self) -> list[str]: + """Return the per-artist display names used for the credit field.""" + return [a.credit for a in self.valid_artists] + + @property + def artist(self) -> str: + """Return the fully rendered artist string using display names.""" + return self.join_artists("name") + + @property + def artist_credit(self) -> str: + """Return the fully rendered artist credit string.""" + return self.join_artists("credit") + + def join_artists(self, property_name: str) -> str: + """Render a single artist string with join phrases and featured artists. + + Non-featured artists are concatenated using their join tokens. Featured + artists are appended after the configured 'featured' marker, preserving + Discogs order while keeping featured credits separate from the main + artist string. + """ + non_featured = [a for a in self.valid_artists if not a.is_feat] + featured = [a for a in self.valid_artists if a.is_feat] + + artist = "".join(a.get_artist(property_name) for a in non_featured) + if featured: + if "feat" not in artist: + artist += f" {self.featured_string} " + + artist += ", ".join(a.get_artist(property_name) for a in featured) + + return artist + + @classmethod + def from_plugin( + cls, + plugin: DiscogsPlugin, + artists: list[Artist], + for_album_artist: bool = False, + ) -> ArtistState: + return cls( + artists, + plugin.config["anv"][ + "album_artist" if for_album_artist else "artist" + ].get(bool), + plugin.config["anv"]["artist_credit"].get(bool), + plugin.config["featured_string"].as_str(), + plugin.config["strip_disambiguation"].get(bool), + ) + + +@dataclass +class TracklistState: + index: int = 0 + index_tracks: dict[int, str] = field(default_factory=dict) + tracks: list[TrackInfo] = field(default_factory=list) + divisions: list[str] = field(default_factory=list) + next_divisions: list[str] = field(default_factory=list) + mediums: list[str | None] = field(default_factory=list) + medium_indices: list[str | None] = field(default_factory=list) + + @property + def info(self) -> TracklistInfo: + return asdict(self) # type: ignore[return-value] + + @classmethod + def build( + cls, + plugin: DiscogsPlugin, + clean_tracklist: list[Track], + albumartistinfo: ArtistState, + ) -> TracklistState: + state = cls() + for track in clean_tracklist: + if track["position"]: + state.index += 1 + if state.next_divisions: + state.divisions += state.next_divisions + state.next_divisions.clear() + track_info, medium, medium_index = plugin.get_track_info( + track, state.index, state.divisions, albumartistinfo + ) + track_info.track_alt = track["position"] + state.tracks.append(track_info) + state.mediums.append(medium or None) + state.medium_indices.append(medium_index or None) + else: + state.next_divisions.append(track["title"]) + try: + state.divisions.pop() + except IndexError: + pass + state.index_tracks[state.index + 1] = track["title"] + return state diff --git a/beetsplug/discogs/types.py b/beetsplug/discogs/types.py new file mode 100644 index 000000000..e06f51ed5 --- /dev/null +++ b/beetsplug/discogs/types.py @@ -0,0 +1,67 @@ +# This file is part of beets. +# Copyright 2025, Sarunas Nejus, Henry Oberholtzer. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from typing_extensions import NotRequired, TypedDict + +if TYPE_CHECKING: + from beets.autotag.hooks import TrackInfo + + +class ReleaseFormat(TypedDict): + name: str + qty: int + descriptions: list[str] | None + + +class Artist(TypedDict): + name: str + anv: str + join: str + role: str + tracks: str + id: str + resource_url: str + + +class Track(TypedDict): + position: str + type_: str + title: str + duration: str + artists: list[Artist] + extraartists: NotRequired[list[Artist]] + sub_tracks: NotRequired[list[Track]] + + +class ArtistInfo(TypedDict): + artist: str + artists: list[str] + artist_credit: str + artists_credit: list[str] + artist_id: str + artists_ids: list[str] + + +class TracklistInfo(TypedDict): + index: int + index_tracks: dict[int, str] + tracks: list[TrackInfo] + divisions: list[str] + next_divisions: list[str] + mediums: list[str | None] + medium_indices: list[str | None] diff --git a/beetsplug/discogs/utils.py b/beetsplug/discogs/utils.py new file mode 100644 index 000000000..fdb1f0058 --- /dev/null +++ b/beetsplug/discogs/utils.py @@ -0,0 +1,47 @@ +# This file is part of beets. +# Copyright 2016, Adrian Sampson. +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. +"""Utility resources for the Discogs plugin.""" + +import http.client +import re +import socket + +from discogs_client.exceptions import DiscogsAPIError +from requests.exceptions import ConnectionError + +# Exceptions that discogs_client should really handle but does not. +CONNECTION_ERRORS = ( + ConnectionError, + socket.error, + http.client.HTTPException, + ValueError, # JSON decoding raises a ValueError. + DiscogsAPIError, +) + +DISAMBIGUATION_RE = re.compile(r" \(\d+\)") + +TRACK_INDEX_RE = re.compile( + r""" + (.*?) # medium: everything before medium_index. + (\d*?) # medium_index: a number at the end of + # `position`, except if followed by a subtrack index. + # subtrack_index: can only be matched if medium + # or medium_index have been matched, and can be + ( + (?<=\w)\.[\w]+ # a dot followed by a string (A.1, 2.A) + | (?<=\d)[A-Z]+ # a string that follows a number (1A, B2a) + )? + """, + re.VERBOSE, +) From ff95ce5d2034d829fd9517adee9aac7a9bbb48e9 Mon Sep 17 00:00:00 2001 From: Henry Date: Sun, 18 Jan 2026 18:54:47 -0800 Subject: [PATCH 38/40] Remove utils, rework from_plugin method in ArtistState to from_config --- beetsplug/discogs/__init__.py | 42 +++++++++++++++++++++++++------ beetsplug/discogs/states.py | 22 +++++++++------- beetsplug/discogs/utils.py | 47 ----------------------------------- test/plugins/test_discogs.py | 4 +-- 4 files changed, 50 insertions(+), 65 deletions(-) delete mode 100644 beetsplug/discogs/utils.py diff --git a/beetsplug/discogs/__init__.py b/beetsplug/discogs/__init__.py index 23e2267df..dc88e0f14 100644 --- a/beetsplug/discogs/__init__.py +++ b/beetsplug/discogs/__init__.py @@ -18,9 +18,11 @@ python3-discogs-client library. from __future__ import annotations +import http.client import json import os import re +import socket import time import traceback from functools import cache @@ -30,6 +32,7 @@ from typing import TYPE_CHECKING import confuse from discogs_client import Client, Master, Release from discogs_client.exceptions import DiscogsAPIError +from requests.exceptions import ConnectionError import beets import beets.ui @@ -38,8 +41,7 @@ from beets.autotag.distance import string_dist from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.metadata_plugins import MetadataSourcePlugin -from .states import ArtistState, TracklistState -from .utils import CONNECTION_ERRORS, DISAMBIGUATION_RE, TRACK_INDEX_RE +from .states import DISAMBIGUATION_RE, ArtistState, TracklistState if TYPE_CHECKING: from collections.abc import Callable, Iterable, Sequence @@ -53,6 +55,31 @@ API_KEY = "rAzVUQYRaoFjeBjyWuWZ" API_SECRET = "plxtUTqoCzwxZpqdPysCwGuBSmZNdZVy" +# Exceptions that discogs_client should really handle but does not. +CONNECTION_ERRORS = ( + ConnectionError, + socket.error, + http.client.HTTPException, + ValueError, # JSON decoding raises a ValueError. + DiscogsAPIError, +) + +TRACK_INDEX_RE = re.compile( + r""" + (.*?) # medium: everything before medium_index. + (\d*?) # medium_index: a number at the end of + # `position`, except if followed by a subtrack index. + # subtrack_index: can only be matched if medium + # or medium_index have been matched, and can be + ( + (?<=\w)\.[\w]+ # a dot followed by a string (A.1, 2.A) + | (?<=\d)[A-Z]+ # a string that follows a number (1A, B2a) + )? + """, + re.VERBOSE, +) + + class DiscogsPlugin(MetadataSourcePlugin): def __init__(self): super().__init__() @@ -304,8 +331,8 @@ class DiscogsPlugin(MetadataSourcePlugin): artist_data = [a.data for a in result.artists] # Information for the album artist - albumartist = ArtistState.from_plugin( - self, artist_data, for_album_artist=True + albumartist = ArtistState.from_config( + self.config, artist_data, for_album_artist=True ) album = re.sub(r" +", " ", result.title) @@ -315,7 +342,8 @@ class DiscogsPlugin(MetadataSourcePlugin): # information and leave us with skeleton `Artist` objects that will # each make an API call just to get the same data back. tracks = self.get_tracks( - result.data["tracklist"], ArtistState.from_plugin(self, artist_data) + result.data["tracklist"], + ArtistState.from_config(self.config, artist_data), ) # Extract information for the optional AlbumInfo fields, if possible. @@ -611,8 +639,8 @@ class DiscogsPlugin(MetadataSourcePlugin): length = self.get_track_length(track["duration"]) # If artists are found on the track, we will use those instead - artistinfo = ArtistState.from_plugin( - self, + artistinfo = ArtistState.from_config( + self.config, [ *(track.get("artists") or albumartistinfo.raw_artists), *track.get("extraartists", []), diff --git a/beetsplug/discogs/states.py b/beetsplug/discogs/states.py index 265c92c4e..4f59404ca 100644 --- a/beetsplug/discogs/states.py +++ b/beetsplug/discogs/states.py @@ -16,6 +16,7 @@ from __future__ import annotations +import re from dataclasses import asdict, dataclass, field from functools import cached_property from typing import TYPE_CHECKING, NamedTuple @@ -23,13 +24,16 @@ from typing import TYPE_CHECKING, NamedTuple from beets import config from .types import Artist, ArtistInfo, Track, TracklistInfo -from .utils import DISAMBIGUATION_RE if TYPE_CHECKING: + from confuse import ConfigView + from beets.autotag.hooks import TrackInfo from . import DiscogsPlugin +DISAMBIGUATION_RE = re.compile(r" \(\d+\)") + @dataclass class ArtistState: @@ -170,20 +174,20 @@ class ArtistState: return artist @classmethod - def from_plugin( + def from_config( cls, - plugin: DiscogsPlugin, + config: ConfigView, artists: list[Artist], for_album_artist: bool = False, ) -> ArtistState: return cls( artists, - plugin.config["anv"][ - "album_artist" if for_album_artist else "artist" - ].get(bool), - plugin.config["anv"]["artist_credit"].get(bool), - plugin.config["featured_string"].as_str(), - plugin.config["strip_disambiguation"].get(bool), + config["anv"]["album_artist" if for_album_artist else "artist"].get( + bool + ), + config["anv"]["artist_credit"].get(bool), + config["featured_string"].as_str(), + config["strip_disambiguation"].get(bool), ) diff --git a/beetsplug/discogs/utils.py b/beetsplug/discogs/utils.py deleted file mode 100644 index fdb1f0058..000000000 --- a/beetsplug/discogs/utils.py +++ /dev/null @@ -1,47 +0,0 @@ -# This file is part of beets. -# Copyright 2016, Adrian Sampson. -# -# Permission is hereby granted, free of charge, to any person obtaining -# a copy of this software and associated documentation files (the -# "Software"), to deal in the Software without restriction, including -# without limitation the rights to use, copy, modify, merge, publish, -# distribute, sublicense, and/or sell copies of the Software, and to -# permit persons to whom the Software is furnished to do so, subject to -# the following conditions: -# -# The above copyright notice and this permission notice shall be -# included in all copies or substantial portions of the Software. -"""Utility resources for the Discogs plugin.""" - -import http.client -import re -import socket - -from discogs_client.exceptions import DiscogsAPIError -from requests.exceptions import ConnectionError - -# Exceptions that discogs_client should really handle but does not. -CONNECTION_ERRORS = ( - ConnectionError, - socket.error, - http.client.HTTPException, - ValueError, # JSON decoding raises a ValueError. - DiscogsAPIError, -) - -DISAMBIGUATION_RE = re.compile(r" \(\d+\)") - -TRACK_INDEX_RE = re.compile( - r""" - (.*?) # medium: everything before medium_index. - (\d*?) # medium_index: a number at the end of - # `position`, except if followed by a subtrack index. - # subtrack_index: can only be matched if medium - # or medium_index have been matched, and can be - ( - (?<=\w)\.[\w]+ # a dot followed by a string (A.1, 2.A) - | (?<=\d)[A-Z]+ # a string that follows a number (1A, B2a) - )? - """, - re.VERBOSE, -) diff --git a/test/plugins/test_discogs.py b/test/plugins/test_discogs.py index 66cbe9371..15d47db6c 100644 --- a/test/plugins/test_discogs.py +++ b/test/plugins/test_discogs.py @@ -674,7 +674,7 @@ def test_parse_featured_artists(track, expected_artist, expected_artists): """Tests the plugins ability to parse a featured artist. Ignores artists that are not listed as featured.""" plugin = DiscogsPlugin() - artistinfo = ArtistState.from_plugin(plugin, [_artist("ARTIST")]) + artistinfo = ArtistState.from_config(plugin.config, [_artist("ARTIST")]) t, _, _ = plugin.get_track_info(track, 1, 1, artistinfo) assert t.artist == expected_artist assert t.artists == expected_artists @@ -724,7 +724,7 @@ def test_get_media_and_albumtype(formats, expected_media, expected_albumtype): def test_va_buildartistinfo(given_artists, expected_info, config_va_name): config["va_name"] = config_va_name assert ( - ArtistState.from_plugin(DiscogsPlugin(), given_artists).info + ArtistState.from_config(DiscogsPlugin().config, given_artists).info == expected_info ) From 9b1bd5df7afacc88625c6c3d2d5ff71da93cff14 Mon Sep 17 00:00:00 2001 From: Henry Date: Mon, 19 Jan 2026 12:46:22 -0800 Subject: [PATCH 39/40] Adjust type annotation, rebase. --- beetsplug/discogs/states.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs/states.py b/beetsplug/discogs/states.py index 4f59404ca..2a8173ba5 100644 --- a/beetsplug/discogs/states.py +++ b/beetsplug/discogs/states.py @@ -23,7 +23,7 @@ from typing import TYPE_CHECKING, NamedTuple from beets import config -from .types import Artist, ArtistInfo, Track, TracklistInfo +from .types import ArtistInfo if TYPE_CHECKING: from confuse import ConfigView @@ -31,6 +31,7 @@ if TYPE_CHECKING: from beets.autotag.hooks import TrackInfo from . import DiscogsPlugin + from .types import Artist, Track, TracklistInfo DISAMBIGUATION_RE = re.compile(r" \(\d+\)") From 958b36b29847ff4e9c171f5cf75028c3feeec29a Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Sun, 3 Aug 2025 16:38:01 -0700 Subject: [PATCH 40/40] fish: complete files in more places --- beetsplug/fish.py | 12 ++++++------ docs/changelog.rst | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/beetsplug/fish.py b/beetsplug/fish.py index b1518f1c4..82e035eb4 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -183,16 +183,16 @@ def get_basic_beet_options(): BL_NEED2.format("-l format-item", "-f -d 'print with custom format'") + BL_NEED2.format("-l format-album", "-f -d 'print with custom format'") + BL_NEED2.format( - "-s l -l library", "-f -r -d 'library database file to use'" + "-s l -l library", "-F -r -d 'library database file to use'" ) + BL_NEED2.format( - "-s d -l directory", "-f -r -d 'destination music directory'" + "-s d -l directory", "-F -r -d 'destination music directory'" ) + BL_NEED2.format( "-s v -l verbose", "-f -d 'print debugging information'" ) + BL_NEED2.format( - "-s c -l config", "-f -r -d 'path to configuration file'" + "-s c -l config", "-F -r -d 'path to configuration file'" ) + BL_NEED2.format( "-s h -l help", "-f -d 'print this help message and exit'" @@ -216,7 +216,7 @@ def get_subcommands(cmd_name_and_help, nobasicfields, extravalues): word += BL_USE3.format( cmdname, f"-a {wrap('$FIELDS')}", - f"-f -d {wrap('fieldname')}", + f"-d {wrap('fieldname')}", ) if extravalues: @@ -270,7 +270,7 @@ def get_all_commands(beetcmds): word += " ".join( BL_USE3.format( name, - f"{cmd_need_arg}{cmd_s}{cmd_l} -f {cmd_arglist}", + f"{cmd_need_arg}{cmd_s}{cmd_l} {cmd_arglist}", cmd_helpstr, ).split() ) @@ -278,7 +278,7 @@ def get_all_commands(beetcmds): word = word + BL_USE3.format( name, - "-s h -l help -f", + "-s h -l help", f"-d {wrap('print help')}", ) return word diff --git a/docs/changelog.rst b/docs/changelog.rst index c87f1eaf4..d59c7ba1f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -50,6 +50,8 @@ New features: the ``clearart_on_import`` config option. Also, ``beet clearart`` is only going to update the files matching the query and with an embedded art, leaving untouched the files without. +- :doc:`plugins/fish`: Filenames are now completed in more places, like after + ``beet import``. Bug fixes: