From c54703a52441e7f250538f559fc638b21c4d248b Mon Sep 17 00:00:00 2001 From: Ari Bradshaw Date: Mon, 17 Aug 2026 07:38:29 -0700 Subject: [PATCH] refactor: replace Likelies with frozen dataclass (#6920) --- beets/autotag/source.py | 16 ++++++++- beets/importer/tasks.py | 6 +++- beets/util/__init__.py | 24 ++++++++++--- docs/changelog.rst | 2 ++ test/autotag/test_source.py | 69 +++++++++++++++++++++++++++++++++++++ test/test_importer.py | 32 +++++++++++++++++ test/test_util.py | 10 +++--- 7 files changed, 148 insertions(+), 11 deletions(-) create mode 100644 test/autotag/test_source.py diff --git a/beets/autotag/source.py b/beets/autotag/source.py index 08cd1c1864..1019c43987 100644 --- a/beets/autotag/source.py +++ b/beets/autotag/source.py @@ -50,7 +50,21 @@ def from_item(cls, item: Item) -> Source: type="track", artist=item.artist, name=item.title, - data=Likelies(item), + data=Likelies( + artist=item.artist, + album=item.album, + albumartist=item.albumartist, + year=item.year, + disctotal=item.disctotal, + mb_albumid=item.mb_albumid, + label=item.label, + barcode=item.barcode, + catalognum=item.catalognum, + country=item.country, + media=item.media, + albumdisambig=item.albumdisambig, + data_source=item.get("data_source"), + ), items=[item], id=item.mb_trackid, id_consensus=True, diff --git a/beets/importer/tasks.py b/beets/importer/tasks.py index 832c29272f..8eb5a40b1d 100644 --- a/beets/importer/tasks.py +++ b/beets/importer/tasks.py @@ -7,6 +7,8 @@ import time from collections import defaultdict from collections.abc import Callable +from copy import deepcopy +from dataclasses import asdict from functools import cached_property from tempfile import mkdtemp from typing import TYPE_CHECKING, Any, AnyStr @@ -320,7 +322,9 @@ def chosen_info(self) -> dict[str, Any]: or APPLY (in which case the data comes from the choice). """ if self.choice_flag in (Action.ASIS, Action.RETAG): - return self.source.data.copy() + if self.is_album: + return asdict(self.source.data) + return deepcopy(dict(self.items[0])) if self.choice_flag is Action.APPLY and self.match: return self.match.info.copy() assert False diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 159fbc3fdd..d0ac125ab9 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -19,6 +19,7 @@ from collections.abc import Sequence from contextlib import suppress from copy import deepcopy +from dataclasses import dataclass from enum import Enum from functools import cache, cached_property from importlib import import_module @@ -839,7 +840,21 @@ def get_most_common_tags(items: Sequence[Item]) -> Likelies: if len({i.albumartist for i in items}) == 1 and likelies["albumartist"]: likelies["artist"] = likelies["albumartist"] - return Likelies(likelies) + return Likelies( + artist=likelies["artist"], + album=likelies["album"], + albumartist=likelies["albumartist"], + year=likelies["year"], + disctotal=likelies["disctotal"], + mb_albumid=likelies["mb_albumid"], + label=likelies["label"], + barcode=likelies["barcode"], + catalognum=likelies["catalognum"], + country=likelies["country"], + media=likelies["media"], + albumdisambig=likelies["albumdisambig"], + data_source=likelies["data_source"], + ) # stdout and stderr as bytes @@ -1247,8 +1262,9 @@ def __hash__(self) -> int: # type: ignore[override] return id(self) -class Likelies(AttrDict[Any]): - """A dictionary of the most common tags in a list of items.""" +@dataclass(frozen=True) +class Likelies: + """The most common tags in a list of items.""" artist: str album: str @@ -1262,4 +1278,4 @@ class Likelies(AttrDict[Any]): country: str media: str albumdisambig: str - data_source: str + data_source: str | None diff --git a/docs/changelog.rst b/docs/changelog.rst index 170708fdc3..a419eda7e8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -75,6 +75,8 @@ Bug fixes Other changes ~~~~~~~~~~~~~ +- Replace the internal ``Likelies`` dictionary with a frozen dataclass whose + fields explicitly describe the metadata used for autotagging. :bug:`6920` - :doc:`plugins/bpd`: Replace the bundled Bluelet scheduler with Python's standard ``asyncio`` event loop. diff --git a/test/autotag/test_source.py b/test/autotag/test_source.py new file mode 100644 index 0000000000..e97e6c7e50 --- /dev/null +++ b/test/autotag/test_source.py @@ -0,0 +1,69 @@ +from dataclasses import FrozenInstanceError, asdict + +import pytest + +from beets.autotag import Source +from beets.library import Item +from beets.util import Likelies + + +def test_album_source_uses_frozen_likelies(): + items = [ + Item( + artist="track artist", + album="album", + albumartist="album artist", + year=2024, + ), + Item( + artist="another artist", + album="album", + albumartist="album artist", + year=2024, + ), + ] + + source = Source.from_items(items) + + assert isinstance(source.data, Likelies) + assert source.data.artist == "album artist" + assert source.data.album == "album" + assert source.data.year == 2024 + with pytest.raises(FrozenInstanceError): + setattr(source.data, "artist", "changed") + + +def test_singleton_source_constructs_fixed_metadata_fields(): + item = Item( + artist="track artist", + title="title", + album="album", + albumartist="album artist", + mb_trackid="track id", + mb_albumid="album id", + data_source="MusicBrainz", + custom_field="not source metadata", + ) + + source = Source.from_item(item) + + assert source.artist == "track artist" + assert source.name == "title" + assert source.id == "track id" + assert source.data.artist == "track artist" + assert source.data.albumartist == "album artist" + assert source.data.mb_albumid == "album id" + assert source.data.data_source == "MusicBrainz" + assert not hasattr(source.data, "title") + assert not hasattr(source.data, "custom_field") + + +def test_likelies_converts_to_detached_dictionary(): + source = Source.from_item(Item(artist="artist", title="title")) + + metadata = asdict(source.data) + metadata["artist"] = "changed" + + assert isinstance(metadata, dict) + assert metadata["album"] == "" + assert source.data.artist == "artist" diff --git a/test/test_importer.py b/test/test_importer.py index 7723535795..61885063b2 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -49,6 +49,38 @@ from beets.util.extension import remux_mpeglayer3_wav +class TestChosenInfo: + def test_album_metadata_is_a_detached_dictionary(self): + item = Item(artist="artist", album="album") + task = importer.ImportTask(None, [], [item]) + task.set_choice(importer.Action.ASIS) + + info = task.chosen_info() + + assert isinstance(info, dict) + assert info["artist"] == "artist" + info["artist"] = "changed" + assert task.source.data.artist == "artist" + + def test_singleton_metadata_preserves_all_item_fields(self): + item = Item( + artist="artist", + title="title", + genres=["Rock"], + custom_field="custom value", + ) + task = importer.SingletonImportTask(None, item) + task.set_choice(importer.Action.ASIS) + + info = task.chosen_info() + + assert isinstance(info, dict) + assert info["title"] == "title" + assert info["custom_field"] == "custom value" + info["genres"].append("Jazz") + assert item.genres == ["Rock"] + + class PathsMixin: import_media: list[MediaFile] diff --git a/test/test_util.py b/test/test_util.py index 64b0f8497e..8ae7bd12eb 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -232,12 +232,12 @@ def test_get_most_common_tags(self): likelies = util.get_most_common_tags(items) - assert likelies["albumartist"] == "aartist" - assert likelies["album"] == "album" + assert likelies.albumartist == "aartist" + assert likelies.album == "album" # albumartist consensus overrides artist - assert likelies["artist"] == "aartist" - assert likelies["label"] == "label 1" - assert likelies["year"] == 0 + assert likelies.artist == "aartist" + assert likelies.label == "label 1" + assert likelies.year == 0 class HelperTest(unittest.TestCase):