From d9d2ddac2366bf30faaed55e1d9172e4d2d0d7c2 Mon Sep 17 00:00:00 2001 From: Tai Lee Date: Mon, 4 Feb 2013 17:04:12 +1100 Subject: [PATCH] Detect false positive markers in root/parent directories. For example, catalogue numbers like "[REACT217]". This shouldn't bypass the nested multi-disc detection and automatically include all subdirs. Do nested multi-disc detection first, so that `collapse_pat` is only set for flattened albums, and we can skip the ancestry check on subsequent folders. --- beets/autotag/__init__.py | 23 ++++++++-------- test/test_autotag.py | 56 +++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index 421dd2460..be0181c79 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -67,7 +67,8 @@ def albums_in_dir(path): # and add the current directory. If so, just add the directory # and move on to the next directory. If not, stop collapsing. if collapse_paths: - if collapse_paths[0] in ancestry(root) or (collapse_pat and + if (not collapse_pat and collapse_paths[0] in ancestry(root)) or ( + collapse_pat and \ collapse_pat.match(os.path.basename(root))): # Still collapsing. collapse_paths.append(root) @@ -88,19 +89,10 @@ def albums_in_dir(path): start_collapsing = False for marker in MULTIDISC_MARKERS: marker_pat = re.compile(MULTIDISC_PAT_FMT % marker, re.I) - - # Is this directory the first in a flattened multi-disc album? match = marker_pat.match(os.path.basename(root)) - if match: - start_collapsing = True - # Set the current pattern to match directories with the same - # prefix as this one, followed by a digit. - collapse_pat = re.compile(r'^%s\d' % - re.escape(match.group(1)), re.I) - break # Is this directory the root of a nested multi-disc album? - elif dirs and not items: + if dirs and not items: # Check whether all subdirectories have the same prefix. start_collapsing = True subdir_pat = None @@ -126,6 +118,15 @@ def albums_in_dir(path): if start_collapsing: break + # Is this directory the first in a flattened multi-disc album? + elif match: + start_collapsing = True + # Set the current pattern to match directories with the same + # prefix as this one, followed by a digit. + collapse_pat = re.compile(r'^%s\d' % + re.escape(match.group(1)), re.I) + break + # If either of the above heuristics indicated that this is the # beginning of a multi-disc album, initialize the collapsed # directory and item lists and check the next directory. diff --git a/test/test_autotag.py b/test/test_autotag.py index 8132314bf..8c9c61921 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -322,32 +322,34 @@ class MultiDiscAlbumsInDirTest(unittest.TestCase): os.mkdir(self.base) self.dirs = [ - # Nested album with non-consecutive disc numbers and a subtitle. - os.path.join(self.base, 'album1'), - os.path.join(self.base, 'album1', 'cd 1'), - os.path.join(self.base, 'album1', 'cd 3 - bonus'), + # Nested album, multiple subdirs. + # Also, false positive marker in root dir, and subtitle for disc 3. + os.path.join(self.base, 'ABCD1234'), + os.path.join(self.base, 'ABCD1234', 'cd 1'), + os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus'), - # Nested album with a single disc and punctuation. - os.path.join(self.base, 'album2'), - os.path.join(self.base, 'album2', 'cd _ 1'), + # Nested album, single subdir. + # Also, punctuation between marker and disc number. + os.path.join(self.base, 'album'), + os.path.join(self.base, 'album', 'cd _ 1'), - # Flattened album with case typo. - os.path.join(self.base, 'artist'), - os.path.join(self.base, 'artist', 'CAT disc 1'), - os.path.join(self.base, 'artist', 'Cat disc 2'), + # Flattened album, case typo. + # Also, false positive marker in parent dir. + os.path.join(self.base, 'artist [CD5]'), + os.path.join(self.base, 'artist [CD5]', 'CAT disc 1'), + os.path.join(self.base, 'artist [CD5]', 'CAt disc 2'), - # Prevent "CAT" being collapsed as a nested multi-disc album, - # and test case insensitive sorting. - os.path.join(self.base, 'artist', 'CATS'), + # Single disc album, sorted between CAT discs. + os.path.join(self.base, 'artist [CD5]', 'CATS'), ] self.files = [ - os.path.join(self.base, 'album1', 'cd 1', 'song1.mp3'), - os.path.join(self.base, 'album1', 'cd 3 - bonus', 'song2.mp3'), - os.path.join(self.base, 'album1', 'cd 3 - bonus', 'song3.mp3'), - os.path.join(self.base, 'album2', 'cd _ 1', 'song4.mp3'), - os.path.join(self.base, 'artist', 'CAT disc 1', 'song5.mp3'), - os.path.join(self.base, 'artist', 'Cat disc 2', 'song6.mp3'), - os.path.join(self.base, 'artist', 'CATS', 'song7.mp3'), + os.path.join(self.base, 'ABCD1234', 'cd 1', 'song1.mp3'), + os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song2.mp3'), + os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song3.mp3'), + os.path.join(self.base, 'album', 'cd _ 1', 'song4.mp3'), + os.path.join(self.base, 'artist [CD5]', 'CAT disc 1', 'song5.mp3'), + os.path.join(self.base, 'artist [CD5]', 'CAt disc 2', 'song6.mp3'), + os.path.join(self.base, 'artist [CD5]', 'CATS', 'song7.mp3'), ] for path in self.dirs: @@ -358,30 +360,26 @@ class MultiDiscAlbumsInDirTest(unittest.TestCase): def tearDown(self): shutil.rmtree(self.base) - def test_coalesce_nested_album_dirs(self): - # Nested album with non-consecutive disc numbers. + def test_coalesce_nested_album_multiple_subdirs(self): albums = list(autotag.albums_in_dir(self.base)) self.assertEquals(len(albums), 4) root, items = albums[0] self.assertEquals(root, self.dirs[0:3]) self.assertEquals(len(items), 3) - def test_coalesce_single_subdirectory(self): - # Nested album with a single disc and punctuation. + def test_coalesce_nested_album_single_subdir(self): albums = list(autotag.albums_in_dir(self.base)) root, items = albums[1] self.assertEquals(root, self.dirs[3:5]) self.assertEquals(len(items), 1) - def test_coalesce_case_insensitive(self): - # Flattened album with case typo. + def test_coalesce_flattened_album_case_typo(self): albums = list(autotag.albums_in_dir(self.base)) root, items = albums[2] self.assertEquals(root, self.dirs[6:8]) self.assertEquals(len(items), 2) - def test_non_multi_disc_album(self): - # Non-multi-disc album (sorted in between "cat" album). + def test_single_disc_album(self): albums = list(autotag.albums_in_dir(self.base)) root, items = albums[3] self.assertEquals(root, self.dirs[8:])