From c66eb1044549cc3ebd3d1ca98dde899ea910c93c Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Thu, 26 Sep 2024 17:20:12 -0400 Subject: [PATCH 1/7] Fix false positives in "feat. X" detection in ftintitle The old version of the `ftintitle.contains_feat` function could lead to false positives by matching words like "and" and "with" in the title, even if there was no "feat. X" part. With this commit, the `for_artist` keyword is explicitly passed to the `plugins.feat_tokens` function to disable these matches when matching a title (and not an artist). --- beetsplug/ftintitle.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 5bc018fe1..2c4634002 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -35,9 +35,15 @@ def split_on_feat(artist): return tuple(parts) -def contains_feat(title): +def contains_feat(title, for_artist=True): """Determine whether the title contains a "featured" marker.""" - return bool(re.search(plugins.feat_tokens(), title, flags=re.IGNORECASE)) + return bool( + re.search( + plugins.feat_tokens(for_artist=for_artist), + title, + flags=re.IGNORECASE, + ) + ) def find_feat_part(artist, albumartist): @@ -143,7 +149,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # Only update the title if it does not already contain a featured # artist and if we do not drop featuring information. - if not drop_feat and not contains_feat(item.title): + if not drop_feat and not contains_feat(item.title, for_artist=False): feat_format = self.config["format"].as_str() new_format = feat_format.format(feat_part) new_title = f"{item.title} {new_format}" From 6cb2e5926bdbf205277b03e1c55e9b97db997cea Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Thu, 26 Sep 2024 17:22:25 -0400 Subject: [PATCH 2/7] Add unit tests for separate "feat. X" detection The unit tests for the `ftintitle.contains_feat` function are now split up for artist and title matching. --- test/plugins/test_ftintitle.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 9e8f14fe1..6347d814b 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -175,7 +175,7 @@ class FtInTitlePluginTest(unittest.TestCase): parts = ftintitle.split_on_feat("Alice defeat Bob") assert parts == ("Alice defeat Bob", None) - def test_contains_feat(self): + def test_contains_feat_artist(self): assert ftintitle.contains_feat("Alice ft. Bob") assert ftintitle.contains_feat("Alice feat. Bob") assert ftintitle.contains_feat("Alice feat Bob") @@ -190,3 +190,16 @@ class FtInTitlePluginTest(unittest.TestCase): assert not ftintitle.contains_feat("Alice defeat Bob") assert not ftintitle.contains_feat("Aliceft.Bob") assert not ftintitle.contains_feat("Alice (defeat Bob)") + + def test_contains_feat_title(self): + assert ftintitle.contains_feat( + "Live and Let Go (feat. Alice)", for_artist=False + ) + assert ftintitle.contains_feat( + "Live and Let Go [feat. Alice]", for_artist=False + ) + assert ftintitle.contains_feat( + "Live and Let Go feat. Alice", for_artist=False + ) + assert not ftintitle.contains_feat("Live and Let Go", for_artist=False) + assert not ftintitle.contains_feat("Come With Me", for_artist=False) From 85fc8e50cfa6a02f0c3b093bb4bab30d513dea86 Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Thu, 26 Sep 2024 17:24:47 -0400 Subject: [PATCH 3/7] Add changelog about fixing false positives in "feat. X" detection --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 33a4b5f94..9ebd77d67 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,6 +26,8 @@ New features: Bug fixes: +* The detection of a "feat. X" part in a song title does not produce any false + positives caused by words like "and" or "with" anymore. :bug:`5441` * The detection of a "feat. X" part now also matches such parts if they are in parentheses or brackets. :bug:`5436` * Improve naming of temporary files by separating the random part with the file extension. From 71b5a9651c1e1590a1ccdf5b67e1c3894f5f3255 Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Mon, 30 Sep 2024 10:20:29 -0500 Subject: [PATCH 4/7] Add reference to ftintitle plugin in changelog Add references to the ftintitle plugin for the bug fixes #5441 and #5436 that are related to this plugin. --- docs/changelog.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 8385a3b85..a9ed03e42 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,9 +26,9 @@ New features: Bug fixes: -* The detection of a "feat. X" part in a song title does not produce any false +* :doc:`plugins/ftintitle`: The detection of a "feat. X" part in a song title does not produce any false positives caused by words like "and" or "with" anymore. :bug:`5441` -* The detection of a "feat. X" part now also matches such parts if they are in +* :doc:`plugins/ftintitle`: The detection of a "feat. X" part now also matches such parts if they are in parentheses or brackets. :bug:`5436` * Improve naming of temporary files by separating the random part with the file extension. * Fix the ``auto`` value for the :ref:`reflink` config option. From ed627c031c1223e45324a30fdfce0be5aba30bcd Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Mon, 30 Sep 2024 10:24:09 -0500 Subject: [PATCH 5/7] Hardcode for_artist keyword in ftintitle plugin Remove the optional `for_artist` keyword in the `ftintitle.contains_feat` function and hardcode it to False, since it is always used like this in the ftintitle plugin. --- beetsplug/ftintitle.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/ftintitle.py b/beetsplug/ftintitle.py index 2c4634002..e4f51fd10 100644 --- a/beetsplug/ftintitle.py +++ b/beetsplug/ftintitle.py @@ -35,11 +35,11 @@ def split_on_feat(artist): return tuple(parts) -def contains_feat(title, for_artist=True): +def contains_feat(title): """Determine whether the title contains a "featured" marker.""" return bool( re.search( - plugins.feat_tokens(for_artist=for_artist), + plugins.feat_tokens(for_artist=False), title, flags=re.IGNORECASE, ) @@ -149,7 +149,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin): # Only update the title if it does not already contain a featured # artist and if we do not drop featuring information. - if not drop_feat and not contains_feat(item.title, for_artist=False): + if not drop_feat and not contains_feat(item.title): feat_format = self.config["format"].as_str() new_format = feat_format.format(feat_part) new_title = f"{item.title} {new_format}" From 669307c91cec8b15610bbbcafea29de3d96c23e7 Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Mon, 30 Sep 2024 10:28:23 -0500 Subject: [PATCH 6/7] Update ftintitle.contains_feat unit tests Since the `for_artist` keyword has been removed from `ftintitle.contains_feat`, the unit tests need to be updated. This includes the deletion of the test cases that test the `for_artist=True` delimiters. --- test/plugins/test_ftintitle.py | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 6347d814b..5568c94b1 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -175,14 +175,11 @@ class FtInTitlePluginTest(unittest.TestCase): parts = ftintitle.split_on_feat("Alice defeat Bob") assert parts == ("Alice defeat Bob", None) - def test_contains_feat_artist(self): + def test_contains_feat(self): assert ftintitle.contains_feat("Alice ft. Bob") assert ftintitle.contains_feat("Alice feat. Bob") assert ftintitle.contains_feat("Alice feat Bob") assert ftintitle.contains_feat("Alice featuring Bob") - assert ftintitle.contains_feat("Alice & Bob") - assert ftintitle.contains_feat("Alice and Bob") - assert ftintitle.contains_feat("Alice With Bob") assert ftintitle.contains_feat("Alice (ft. Bob)") assert ftintitle.contains_feat("Alice (feat. Bob)") assert ftintitle.contains_feat("Alice [ft. Bob]") @@ -190,16 +187,8 @@ class FtInTitlePluginTest(unittest.TestCase): assert not ftintitle.contains_feat("Alice defeat Bob") assert not ftintitle.contains_feat("Aliceft.Bob") assert not ftintitle.contains_feat("Alice (defeat Bob)") - - def test_contains_feat_title(self): - assert ftintitle.contains_feat( - "Live and Let Go (feat. Alice)", for_artist=False - ) - assert ftintitle.contains_feat( - "Live and Let Go [feat. Alice]", for_artist=False - ) - assert ftintitle.contains_feat( - "Live and Let Go feat. Alice", for_artist=False - ) - assert not ftintitle.contains_feat("Live and Let Go", for_artist=False) - assert not ftintitle.contains_feat("Come With Me", for_artist=False) + assert ftintitle.contains_feat("Live and Let Go (feat. Alice)") + assert ftintitle.contains_feat("Live and Let Go [feat. Alice]") + assert ftintitle.contains_feat("Live and Let Go feat. Alice") + assert not ftintitle.contains_feat("Live and Let Go") + assert not ftintitle.contains_feat("Come With Me") From 37879d0b18a0d9c9f85c8c58eda1755f521cb524 Mon Sep 17 00:00:00 2001 From: Karl Besser Date: Tue, 1 Oct 2024 15:55:38 -0500 Subject: [PATCH 7/7] Remove redundant unit tests for ftintitle plugin Remove redundant unit tests for the `ftintitle.cotains_feat` function --- test/plugins/test_ftintitle.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/plugins/test_ftintitle.py b/test/plugins/test_ftintitle.py index 5568c94b1..1dbe4a727 100644 --- a/test/plugins/test_ftintitle.py +++ b/test/plugins/test_ftintitle.py @@ -187,8 +187,5 @@ class FtInTitlePluginTest(unittest.TestCase): assert not ftintitle.contains_feat("Alice defeat Bob") assert not ftintitle.contains_feat("Aliceft.Bob") assert not ftintitle.contains_feat("Alice (defeat Bob)") - assert ftintitle.contains_feat("Live and Let Go (feat. Alice)") - assert ftintitle.contains_feat("Live and Let Go [feat. Alice]") - assert ftintitle.contains_feat("Live and Let Go feat. Alice") assert not ftintitle.contains_feat("Live and Let Go") assert not ftintitle.contains_feat("Come With Me")