From 09ef565cfcb3d962f8693fd5ba08922a1cdd60db Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Sat, 19 Apr 2014 16:22:34 +0200 Subject: [PATCH] Make _infer_album_fields a method --- beets/importer.py | 98 +++++++++++++++++++++---------------------- test/test_importer.py | 21 ++++------ 2 files changed, 56 insertions(+), 63 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index b9b3b9e3a..49042111e 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -58,56 +58,6 @@ class ImportAbort(Exception): # Utilities. -def _infer_album_fields(task): - """Given an album and an associated import task, massage the - album-level metadata. This ensures that the album artist is set - and that the "compilation" flag is set automatically. - """ - assert task.is_album - assert task.items - - changes = {} - - if task.choice_flag == action.ASIS: - # Taking metadata "as-is". Guess whether this album is VA. - plur_albumartist, freq = util.plurality( - [i.albumartist or i.artist for i in task.items] - ) - if freq == len(task.items) or \ - (freq > 1 and - float(freq) / len(task.items) >= SINGLE_ARTIST_THRESH): - # Single-artist album. - changes['albumartist'] = plur_albumartist - changes['comp'] = False - else: - # VA. - changes['albumartist'] = VARIOUS_ARTISTS - changes['comp'] = True - - elif task.choice_flag == action.APPLY: - # Applying autotagged metadata. Just get AA from the first - # item. - for item in task.items: - if item is not None: - first_item = item - break - else: - assert False, "all items are None" - if not first_item.albumartist: - changes['albumartist'] = first_item.artist - if not first_item.mb_albumartistid: - changes['mb_albumartistid'] = first_item.mb_artistid - - else: - assert False - - # Apply new metadata. - for item in task.items: - if item is not None: - for k, v in changes.iteritems(): - setattr(item, k, v) - - def _open_state(): """Reads the state file, returning a dictionary.""" try: @@ -503,6 +453,49 @@ class ImportTask(object): duplicates.append(album) return duplicates + def infer_album_fields(self): + """Make the some album fields equal across `self.items` + """ + changes = {} + + if self.choice_flag == action.ASIS: + # Taking metadata "as-is". Guess whether this album is VA. + plur_albumartist, freq = util.plurality( + [i.albumartist or i.artist for i in self.items] + ) + if freq == len(self.items) or \ + (freq > 1 and + float(freq) / len(self.items) >= SINGLE_ARTIST_THRESH): + # Single-artist album. + changes['albumartist'] = plur_albumartist + changes['comp'] = False + else: + # VA. + changes['albumartist'] = VARIOUS_ARTISTS + changes['comp'] = True + + elif self.choice_flag == action.APPLY: + # Applying autotagged metadata. Just get AA from the first + # item. + # FIXME this is overly complicated. Can we assume that + # `self.items` contains only elements that are not none and + # at least one of them? + for item in self.items: + if item is not None: + first_item = item + break + else: + assert False, "all items are None" + if not first_item.albumartist: + changes['albumartist'] = first_item.artist + if not first_item.mb_albumartistid: + changes['mb_albumartistid'] = first_item.mb_artistid + + # Apply new metadata. + for item in self.items: + if item is not None: + item.update(changes) + # Utilities. def prune(self, filename): @@ -586,6 +579,9 @@ class SingletonImportTask(ImportTask): found_items.append(other_item) return found_items + def infer_album_fields(self): + raise NotImplementedError + # FIXME The inheritance relationships are inverted. This is why there # are so many methods which pass. We should introduce a new @@ -967,7 +963,7 @@ def apply_choices(session): # Infer album-level fields. if task.is_album: - _infer_album_fields(task) + task.infer_album_fields() # Find existing item entries that these are replacing (for # re-imports). Old album structures are automatically cleaned up diff --git a/test/test_importer.py b/test/test_importer.py index 50a8c1d3b..bf37b21b4 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -867,12 +867,9 @@ class InferAlbumDataTest(_common.TestCase): items=self.items) self.task.set_null_candidates() - def _infer(self): - importer._infer_album_fields(self.task) - def test_asis_homogenous_single_artist(self): self.task.set_choice(importer.action.ASIS) - self._infer() + self.task.infer_album_fields() self.assertFalse(self.items[0].comp) self.assertEqual(self.items[0].albumartist, self.items[2].artist) @@ -881,7 +878,7 @@ class InferAlbumDataTest(_common.TestCase): self.items[1].artist = 'some other artist' self.task.set_choice(importer.action.ASIS) - self._infer() + self.task.infer_album_fields() self.assertTrue(self.items[0].comp) self.assertEqual(self.items[0].albumartist, 'Various Artists') @@ -891,7 +888,7 @@ class InferAlbumDataTest(_common.TestCase): self.items[1].artist = 'some other artist' self.task.set_choice(importer.action.ASIS) - self._infer() + self.task.infer_album_fields() for item in self.items: self.assertTrue(item.comp) @@ -901,7 +898,7 @@ class InferAlbumDataTest(_common.TestCase): self.items[0].artist = 'another artist' self.task.set_choice(importer.action.ASIS) - self._infer() + self.task.infer_album_fields() self.assertFalse(self.items[0].comp) self.assertEqual(self.items[0].albumartist, self.items[2].artist) @@ -914,7 +911,7 @@ class InferAlbumDataTest(_common.TestCase): item.mb_albumartistid = 'some album artist id' self.task.set_choice(importer.action.ASIS) - self._infer() + self.task.infer_album_fields() self.assertEqual(self.items[0].albumartist, 'some album artist') @@ -924,7 +921,7 @@ class InferAlbumDataTest(_common.TestCase): def test_apply_gets_artist_and_id(self): self.task.set_choice(AlbumMatch(0, None, {}, set(), set())) # APPLY - self._infer() + self.task.infer_album_fields() self.assertEqual(self.items[0].albumartist, self.items[0].artist) self.assertEqual(self.items[0].mb_albumartistid, @@ -936,7 +933,7 @@ class InferAlbumDataTest(_common.TestCase): item.mb_albumartistid = 'some album artist id' self.task.set_choice(AlbumMatch(0, None, {}, set(), set())) # APPLY - self._infer() + self.task.infer_album_fields() self.assertEqual(self.items[0].albumartist, 'some album artist') @@ -947,13 +944,13 @@ class InferAlbumDataTest(_common.TestCase): self.items = [self.items[0]] self.task.items = self.items self.task.set_choice(importer.action.ASIS) - self._infer() + self.task.infer_album_fields() self.assertFalse(self.items[0].comp) def test_first_item_null_apply(self): self.items[0] = None self.task.set_choice(AlbumMatch(0, None, {}, set(), set())) # APPLY - self._infer() + self.task.infer_album_fields() self.assertFalse(self.items[1].comp) self.assertEqual(self.items[1].albumartist, self.items[2].artist)