From 4d4113e3a4a157c60e81b4075fec7310547880e3 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:15:09 -0500 Subject: [PATCH 1/5] Refactor ftintitle to extract the code to find the featured artist This removes the code that extracts the featured artist from the original artist field from the ft_in_title function and puts it into its own. It also adds a custom ArtistNotFoundException that find_feat_part will throw if the album artist is not in the artist field. This allows us to easily test the results of finding a featured artist in the artist sting, and thus adds tests for the usual test cases that the split_on_feat gets tested for. --- beetsplug/ftintitle.py | 56 ++++++++++++++++++++++++++++-------------- test/test_ftintitle.py | 15 +++++++++++ 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index f28a1661c..bfeaf734d 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -23,6 +23,11 @@ import re log = logging.getLogger('beets') +class ArtistNotFoundException(Exception): + def __init__(self, value): + self.value = value + def __str__(self): + return repr(self.value) def split_on_feat(artist): """Given an artist string, split the "main" artist from any artist @@ -67,6 +72,34 @@ def update_metadata(item, feat_part, drop_feat, loglevel=logging.DEBUG): item.title = new_title +def find_feat_part(artist, albumartist): + """Attempt to find featured artists in the item's artist fields and + return the results. Returns None if no featured artist found. + """ + feat_part = None + + # Look for the album artist in the artist field. If it's not + # present, give up. + albumartist_split = artist.split(albumartist, 1) + if len(albumartist_split) <= 1: + raise ArtistNotFoundException('album artist not present in artist') + + # If the last element of the split (the right-hand side of the + # album artist) is nonempty, then it probably contains the + # featured artist. + elif albumartist_split[-1] != '': + # Extract the featured artist from the right-hand side. + _, feat_part = split_on_feat(albumartist_split[-1]) + + # Otherwise, if there's nothing on the right-hand side, look for a + # featuring artist on the left-hand side. + else: + lhs, rhs = split_on_feat(albumartist_split[0]) + if rhs: + feat_part = lhs + + return feat_part + def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): """Look for featured artists in the item's artist fields and move them to the title. @@ -80,28 +113,13 @@ def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): _, featured = split_on_feat(artist) if featured and albumartist != artist and albumartist: log.log(loglevel, displayable_path(item.path)) - feat_part = None - # Look for the album artist in the artist field. If it's not - # present, give up. - albumartist_split = artist.split(albumartist, 1) - if len(albumartist_split) <= 1: + # Attempt to find the featured artist + try: + feat_part = find_feat_part(artist, albumartist) + except ArtistNotFoundException: log.log(loglevel, 'album artist not present in artist') - # If the last element of the split (the right-hand side of the - # album artist) is nonempty, then it probably contains the - # featured artist. - elif albumartist_split[-1] != '': - # Extract the featured artist from the right-hand side. - _, feat_part = split_on_feat(albumartist_split[-1]) - - # Otherwise, if there's nothing on the right-hand side, look for a - # featuring artist on the left-hand side. - else: - lhs, rhs = split_on_feat(albumartist_split[0]) - if rhs: - feat_part = lhs - # If we have a featuring artist, move it to the title. if feat_part: update_metadata(item, feat_part, drop_feat, loglevel) diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index 77e416c5a..cffa3332a 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -23,6 +23,21 @@ class FtInTitlePluginTest(unittest.TestCase): """Set up configuration""" ftintitle.FtInTitlePlugin() + def test_find_feat_part(self): + test_cases = [ + {'artist': 'Alice ft. Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice feat Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice featuring Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice & Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice and Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice With Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, + {'artist': 'Alice defeat Bob', 'album_artist': 'Alice', 'feat_part': None}, + ] + + for test_case in test_cases: + feat_part = ftintitle.find_feat_part(test_case['artist'], test_case['album_artist']) + self.assertEqual(feat_part, test_case['feat_part']) + def test_split_on_feat(self): parts = ftintitle.split_on_feat('Alice ft. Bob') self.assertEqual(parts, ('Alice', 'Bob')) From a70820d8a1fc2d16fac2cd89b73a3b86d85ea931 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:20:29 -0500 Subject: [PATCH 2/5] Fix a bug in ftintitle with the order of album artist There was a bug in the find_feat_part function that would cause it to fail if the album artist was the second part of the featured string. For example, if the Artist field was Alice & Bob, and the Album Artist field was Bob it would return None due to the order. This fixes that and adds test cases to ensure it doesn't return. --- beetsplug/ftintitle.py | 2 +- test/test_ftintitle.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index bfeaf734d..d332b0c08 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -95,7 +95,7 @@ def find_feat_part(artist, albumartist): # featuring artist on the left-hand side. else: lhs, rhs = split_on_feat(albumartist_split[0]) - if rhs: + if lhs: feat_part = lhs return feat_part diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index cffa3332a..fa51dfc14 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -32,6 +32,8 @@ class FtInTitlePluginTest(unittest.TestCase): {'artist': 'Alice and Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, {'artist': 'Alice With Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, {'artist': 'Alice defeat Bob', 'album_artist': 'Alice', 'feat_part': None}, + {'artist': 'Alice & Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, + {'artist': 'Alice ft. Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, ] for test_case in test_cases: From 8f4181843445023fd9fb20426a63d084f59ceccb Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 20:58:59 -0500 Subject: [PATCH 3/5] Fix formatting to pass flake8 tests --- beetsplug/ftintitle.py | 4 +++ test/test_ftintitle.py | 59 +++++++++++++++++++++++++++++++++++------- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index d332b0c08..fbeb7bc27 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -23,12 +23,15 @@ import re log = logging.getLogger('beets') + class ArtistNotFoundException(Exception): def __init__(self, value): self.value = value + def __str__(self): return repr(self.value) + def split_on_feat(artist): """Given an artist string, split the "main" artist from any artist on the right-hand side of a string like "feat". Return the main @@ -100,6 +103,7 @@ def find_feat_part(artist, albumartist): return feat_part + def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): """Look for featured artists in the item's artist fields and move them to the title. diff --git a/test/test_ftintitle.py b/test/test_ftintitle.py index fa51dfc14..249569566 100644 --- a/test/test_ftintitle.py +++ b/test/test_ftintitle.py @@ -25,19 +25,58 @@ class FtInTitlePluginTest(unittest.TestCase): def test_find_feat_part(self): test_cases = [ - {'artist': 'Alice ft. Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice feat Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice featuring Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice & Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice and Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice With Bob', 'album_artist': 'Alice', 'feat_part': 'Bob'}, - {'artist': 'Alice defeat Bob', 'album_artist': 'Alice', 'feat_part': None}, - {'artist': 'Alice & Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, - {'artist': 'Alice ft. Bob', 'album_artist': 'Bob', 'feat_part': 'Alice'}, + { + 'artist': 'Alice ft. Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice feat Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice featuring Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice & Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice and Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice With Bob', + 'album_artist': 'Alice', + 'feat_part': 'Bob' + }, + { + 'artist': 'Alice defeat Bob', + 'album_artist': 'Alice', + 'feat_part': None + }, + { + 'artist': 'Alice & Bob', + 'album_artist': 'Bob', + 'feat_part': 'Alice' + }, + { + 'artist': 'Alice ft. Bob', + 'album_artist': 'Bob', + 'feat_part': 'Alice' + }, ] for test_case in test_cases: - feat_part = ftintitle.find_feat_part(test_case['artist'], test_case['album_artist']) + feat_part = ftintitle.find_feat_part( + test_case['artist'], + test_case['album_artist'] + ) self.assertEqual(feat_part, test_case['feat_part']) def test_split_on_feat(self): From e71c464c141c51e65807d0ca19e50d28caf5776e Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sun, 28 Dec 2014 21:27:58 -0500 Subject: [PATCH 4/5] Initialize feat_part as None --- beetsplug/ftintitle.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index fbeb7bc27..47d918f5f 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -118,6 +118,8 @@ def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): if featured and albumartist != artist and albumartist: log.log(loglevel, displayable_path(item.path)) + feat_part = None + # Attempt to find the featured artist try: feat_part = find_feat_part(artist, albumartist) From 8cd3d0059fe7fbe655c346cd801d9fe3acbd1612 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Mon, 29 Dec 2014 11:10:43 -0500 Subject: [PATCH 5/5] Remove ArtistNotFoundException in favor of returning None for simplicity --- beetsplug/ftintitle.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 47d918f5f..c241e1f08 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -24,14 +24,6 @@ import re log = logging.getLogger('beets') -class ArtistNotFoundException(Exception): - def __init__(self, value): - self.value = value - - def __str__(self): - return repr(self.value) - - def split_on_feat(artist): """Given an artist string, split the "main" artist from any artist on the right-hand side of a string like "feat". Return the main @@ -85,7 +77,7 @@ def find_feat_part(artist, albumartist): # present, give up. albumartist_split = artist.split(albumartist, 1) if len(albumartist_split) <= 1: - raise ArtistNotFoundException('album artist not present in artist') + return feat_part # If the last element of the split (the right-hand side of the # album artist) is nonempty, then it probably contains the @@ -121,10 +113,7 @@ def ft_in_title(item, drop_feat, loglevel=logging.DEBUG): feat_part = None # Attempt to find the featured artist - try: - feat_part = find_feat_part(artist, albumartist) - except ArtistNotFoundException: - log.log(loglevel, 'album artist not present in artist') + feat_part = find_feat_part(artist, albumartist) # If we have a featuring artist, move it to the title. if feat_part: