diff --git a/beets/autotag/match.py b/beets/autotag/match.py index db4a35b13..a7121fd34 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -127,15 +127,21 @@ def assign_items( objects. These "extra" objects occur when there is an unequal number of objects of the two types. """ + log.debug("Computing track assignment...") # Construct the cost matrix. costs = [[float(track_distance(i, t)) for t in tracks] for i in items] - # Find a minimum-cost bipartite matching. - log.debug("Computing track assignment...") - cost, _, assigned_idxs = lap.lapjv(np.array(costs), extend_cost=True) + # Assign items to tracks + _, _, assigned_item_idxs = lap.lapjv(np.array(costs), extend_cost=True) log.debug("...done.") - # Produce the output matching. - mapping = {items[i]: tracks[t] for (t, i) in enumerate(assigned_idxs)} + # Each item in `assigned_item_idxs` list corresponds to a track in the + # `tracks` list. Each value is either an index into the assigned item in + # `items` list, or -1 if that track has no match. + mapping = { + items[iidx]: t + for iidx, t in zip(assigned_item_idxs, tracks) + if iidx != -1 + } extra_items = list(set(items) - mapping.keys()) extra_items.sort(key=lambda i: (i.disc, i.track, i.title)) extra_tracks = list(set(tracks) - set(mapping.values())) diff --git a/beets/test/helper.py b/beets/test/helper.py index 124063d76..4effa47f8 100644 --- a/beets/test/helper.py +++ b/beets/test/helper.py @@ -48,7 +48,7 @@ from mediafile import Image, MediaFile import beets import beets.plugins -from beets import autotag, config, importer, logging, util +from beets import autotag, importer, logging, util from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.importer import ImportSession from beets.library import Album, Item, Library @@ -153,12 +153,27 @@ def check_reflink_support(path: str) -> bool: return reflink.supported_at(path) +class ConfigMixin: + @cached_property + def config(self) -> beets.IncludeLazyConfig: + """Base beets configuration for tests.""" + config = beets.config + config.sources = [] + config.read(user=False, defaults=True) + + config["plugins"] = [] + config["verbose"] = 1 + config["ui"]["color"] = False + config["threaded"] = False + return config + + NEEDS_REFLINK = unittest.skipUnless( check_reflink_support(gettempdir()), "no reflink support for libdir" ) -class TestHelper(_common.Assertions): +class TestHelper(_common.Assertions, ConfigMixin): """Helper mixin for high-level cli and plugin tests. This mixin provides methods to isolate beets' global state provide @@ -184,8 +199,6 @@ class TestHelper(_common.Assertions): - ``libdir`` Path to a subfolder of ``temp_dir``, containing the library's media files. Same as ``config['directory']``. - - ``config`` The global configuration used by beets. - - ``lib`` Library instance created with the settings from ``config``. @@ -202,15 +215,6 @@ class TestHelper(_common.Assertions): ) self.env_patcher.start() - self.config = beets.config - self.config.sources = [] - self.config.read(user=False, defaults=True) - - self.config["plugins"] = [] - self.config["verbose"] = 1 - self.config["ui"]["color"] = False - self.config["threaded"] = False - self.libdir = os.path.join(self.temp_dir, b"libdir") os.mkdir(syspath(self.libdir)) self.config["directory"] = os.fsdecode(self.libdir) @@ -229,8 +233,6 @@ class TestHelper(_common.Assertions): self.io.restore() self.lib._close() self.remove_temp_dir() - beets.config.clear() - beets.config._materialized = False # Library fixtures methods @@ -452,7 +454,7 @@ class ItemInDBTestCase(BeetsTestCase): self.i = _common.item(self.lib) -class PluginMixin: +class PluginMixin(ConfigMixin): plugin: ClassVar[str] preload_plugin: ClassVar[bool] = True @@ -473,7 +475,7 @@ class PluginMixin: """ # FIXME this should eventually be handled by a plugin manager plugins = (self.plugin,) if hasattr(self, "plugin") else plugins - beets.config["plugins"] = plugins + self.config["plugins"] = plugins beets.plugins.load_plugins(plugins) beets.plugins.find_plugins() @@ -494,7 +496,7 @@ class PluginMixin: # FIXME this should eventually be handled by a plugin manager for plugin_class in beets.plugins._instances: plugin_class.listeners = None - beets.config["plugins"] = [] + self.config["plugins"] = [] beets.plugins._classes = set() beets.plugins._instances = {} Item._types = getattr(Item, "_original_types", {}) @@ -504,7 +506,7 @@ class PluginMixin: @contextmanager def configure_plugin(self, config: Any): - beets.config[self.plugin].set(config) + self.config[self.plugin].set(config) self.load_plugins(self.plugin) yield @@ -624,7 +626,7 @@ class ImportHelper(TestHelper): def setup_importer( self, import_dir: bytes | None = None, **kwargs ) -> ImportSession: - config["import"].set_args({**self.default_import_config, **kwargs}) + self.config["import"].set_args({**self.default_import_config, **kwargs}) self.importer = self._get_import_session(import_dir or self.import_dir) return self.importer diff --git a/test/test_autotag.py b/test/test_autotag.py index e131a6be1..7f8ed3d2e 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -23,7 +23,7 @@ from beets import autotag, config from beets.autotag import AlbumInfo, TrackInfo, correct_list_fields, match from beets.autotag.hooks import Distance, string_dist from beets.library import Item -from beets.test.helper import BeetsTestCase +from beets.test.helper import BeetsTestCase, ConfigMixin from beets.util import plurality @@ -498,85 +498,46 @@ class AlbumDistanceTest(BeetsTestCase): assert dist == 0 -class AssignmentTest(unittest.TestCase): - def item(self, title, track): - return Item( - title=title, - track=track, - mb_trackid="", - mb_albumid="", - mb_artistid="", - ) +class TestAssignment(ConfigMixin): + A = "one" + B = "two" + C = "three" - def test_reorder_when_track_numbers_incorrect(self): - items = [] - items.append(self.item("one", 1)) - items.append(self.item("three", 2)) - items.append(self.item("two", 3)) - trackinfo = [] - trackinfo.append(TrackInfo(title="one")) - trackinfo.append(TrackInfo(title="two")) - trackinfo.append(TrackInfo(title="three")) - mapping, extra_items, extra_tracks = match.assign_items( - items, trackinfo - ) - assert extra_items == [] - assert extra_tracks == [] - assert mapping == { - items[0]: trackinfo[0], - items[1]: trackinfo[2], - items[2]: trackinfo[1], - } + @pytest.fixture(autouse=True) + def _setup_config(self): + self.config["match"]["track_length_grace"] = 10 + self.config["match"]["track_length_max"] = 30 - def test_order_works_with_invalid_track_numbers(self): - items = [] - items.append(self.item("one", 1)) - items.append(self.item("three", 1)) - items.append(self.item("two", 1)) - trackinfo = [] - trackinfo.append(TrackInfo(title="one")) - trackinfo.append(TrackInfo(title="two")) - trackinfo.append(TrackInfo(title="three")) - mapping, extra_items, extra_tracks = match.assign_items( - items, trackinfo - ) - assert extra_items == [] - assert extra_tracks == [] - assert mapping == { - items[0]: trackinfo[0], - items[1]: trackinfo[2], - items[2]: trackinfo[1], - } + @pytest.mark.parametrize( + # 'expected' is a tuple of expected (mapping, extra_items, extra_tracks) + "item_titles, track_titles, expected", + [ + # items ordering gets corrected + ([A, C, B], [A, B, C], ({A: A, B: B, C: C}, [], [])), + # unmatched tracks are returned as 'extra_tracks' + # the first track is unmatched + ([B, C], [A, B, C], ({B: B, C: C}, [], [A])), + # the middle track is unmatched + ([A, C], [A, B, C], ({A: A, C: C}, [], [B])), + # the last track is unmatched + ([A, B], [A, B, C], ({A: A, B: B}, [], [C])), + # unmatched items are returned as 'extra_items' + ([A, C, B], [A, C], ({A: A, C: C}, [B], [])), + ], + ) + def test_assign_tracks(self, item_titles, track_titles, expected): + expected_mapping, expected_extra_items, expected_extra_tracks = expected - def test_order_works_with_missing_tracks(self): - items = [] - items.append(self.item("one", 1)) - items.append(self.item("three", 3)) - trackinfo = [] - trackinfo.append(TrackInfo(title="one")) - trackinfo.append(TrackInfo(title="two")) - trackinfo.append(TrackInfo(title="three")) - mapping, extra_items, extra_tracks = match.assign_items( - items, trackinfo - ) - assert extra_items == [] - assert extra_tracks == [trackinfo[1]] - assert mapping == {items[0]: trackinfo[0], items[1]: trackinfo[2]} + items = [Item(title=title) for title in item_titles] + tracks = [TrackInfo(title=title) for title in track_titles] - def test_order_works_with_extra_tracks(self): - items = [] - items.append(self.item("one", 1)) - items.append(self.item("two", 2)) - items.append(self.item("three", 3)) - trackinfo = [] - trackinfo.append(TrackInfo(title="one")) - trackinfo.append(TrackInfo(title="three")) - mapping, extra_items, extra_tracks = match.assign_items( - items, trackinfo - ) - assert extra_items == [items[1]] - assert extra_tracks == [] - assert mapping == {items[0]: trackinfo[0], items[2]: trackinfo[1]} + mapping, extra_items, extra_tracks = match.assign_items(items, tracks) + + assert ( + {i.title: t.title for i, t in mapping.items()}, + [i.title for i in extra_items], + [t.title for t in extra_tracks], + ) == (expected_mapping, expected_extra_items, expected_extra_tracks) def test_order_works_when_track_names_are_entirely_wrong(self): # A real-world test case contributed by a user. @@ -587,9 +548,6 @@ class AssignmentTest(unittest.TestCase): title=f"ben harper - Burn to Shine {i}", track=i, length=length, - mb_trackid="", - mb_albumid="", - mb_artistid="", ) items = [] @@ -623,13 +581,9 @@ class AssignmentTest(unittest.TestCase): trackinfo.append(info(11, "Beloved One", 243.733)) trackinfo.append(info(12, "In the Lord's Arms", 186.13300000000001)) - mapping, extra_items, extra_tracks = match.assign_items( - items, trackinfo - ) - assert extra_items == [] - assert extra_tracks == [] - for item, info in mapping.items(): - assert items.index(item) == trackinfo.index(info) + expected = dict(zip(items, trackinfo)), [], [] + + assert match.assign_items(items, trackinfo) == expected class ApplyTestUtil: