diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index 4dcc27b64..2b8adf986 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -72,7 +72,8 @@ def albums_in_dir(path, ignore=()): else: # Collapse finished. Yield the collapsed directory and # proceed to process the current one. - yield collapse_root, collapse_items + if collapse_items: + yield collapse_root, collapse_items collapse_root = collapse_items = None # Does the current directory look like a multi-disc album? If @@ -98,7 +99,7 @@ def albums_in_dir(path, ignore=()): yield root, items # Clear out any unfinished collapse. - if collapse_root is not None: + if collapse_root is not None and collapse_items: yield collapse_root, collapse_items def apply_item_metadata(item, track_info): diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 67dd61029..198fc8baf 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -328,13 +328,17 @@ def levenshtein(s1, s2): def plurality(objs): """Given a sequence of comparable objects, returns the object that - is most common in the set and the frequency of that object. + is most common in the set and the frequency of that object. The + sequence must contain at least one object. """ # Calculate frequencies. freqs = defaultdict(int) for obj in objs: freqs[obj] += 1 + if not freqs: + raise ValueError('sequence must be non-empty') + # Find object with maximum frequency. max_freq = 0 res = None diff --git a/test/test_autotag.py b/test/test_autotag.py index 9ff0ceebf..82406c200 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -46,6 +46,10 @@ class PluralityTest(unittest.TestCase): self.assert_(obj in (1, 2)) self.assertEqual(freq, 2) + def test_plurality_empty_sequence_raises_error(self): + with self.assertRaises(ValueError): + plurality([]) + def test_current_metadata_finds_pluralities(self): items = [Item({'artist': 'The Beetles', 'album': 'The White Album'}), Item({'artist': 'The Beatles', 'album': 'The White Album'}), @@ -201,21 +205,27 @@ class MultiDiscAlbumsInDirTest(unittest.TestCase): def setUp(self): self.base = os.path.abspath(os.path.join(_common.RSRC, 'tempdir')) os.mkdir(self.base) - - os.mkdir(os.path.join(self.base, 'album1')) - os.mkdir(os.path.join(self.base, 'album1', 'disc 1')) - os.mkdir(os.path.join(self.base, 'album1', 'disc 2')) - os.mkdir(os.path.join(self.base, 'dir2')) - os.mkdir(os.path.join(self.base, 'dir2', 'disc 1')) - os.mkdir(os.path.join(self.base, 'dir2', 'something')) - - _mkmp3(os.path.join(self.base, 'album1', 'disc 1', 'song1.mp3')) - _mkmp3(os.path.join(self.base, 'album1', 'disc 2', 'song2.mp3')) - _mkmp3(os.path.join(self.base, 'album1', 'disc 2', 'song3.mp3')) + self.dirs = [ + os.path.join(self.base, 'album1'), + os.path.join(self.base, 'album1', 'disc 1'), + os.path.join(self.base, 'album1', 'disc 2'), + os.path.join(self.base, 'dir2'), + os.path.join(self.base, 'dir2', 'disc 1'), + os.path.join(self.base, 'dir2', 'something'), + ] + self.files = [ + os.path.join(self.base, 'album1', 'disc 1', 'song1.mp3'), + os.path.join(self.base, 'album1', 'disc 2', 'song2.mp3'), + os.path.join(self.base, 'album1', 'disc 2', 'song3.mp3'), + os.path.join(self.base, 'dir2', 'disc 1', 'song4.mp3'), + os.path.join(self.base, 'dir2', 'something', 'song5.mp3'), + ] - _mkmp3(os.path.join(self.base, 'dir2', 'disc 1', 'song4.mp3')) - _mkmp3(os.path.join(self.base, 'dir2', 'something', 'song5.mp3')) + for path in self.dirs: + os.mkdir(path) + for path in self.files: + _mkmp3(path) def tearDown(self): shutil.rmtree(self.base) @@ -234,6 +244,14 @@ class MultiDiscAlbumsInDirTest(unittest.TestCase): root, items = albums[2] self.assertEquals(root, os.path.join(self.base, 'dir2', 'something')) + def test_do_not_yield_empty_album(self): + # Remove all the MP3s. + for path in self.files: + os.remove(path) + + albums = list(autotag.albums_in_dir(self.base)) + self.assertEquals(len(albums), 0) + class OrderingTest(unittest.TestCase): def item(self, title, track): return Item({