-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ftintitle: Fix false positives in "feat. X" detection in title #5442
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
The unit tests for the `ftintitle.contains_feat` function are now split up for artist and title matching.
snejus
requested changes
Sep 28, 2024
Add references to the ftintitle plugin for the bug fixes beetbox#5441 and beetbox#5436 that are related to this 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.
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.
snejus
requested changes
Oct 1, 2024
Remove redundant unit tests for the `ftintitle.cotains_feat` function
snejus
approved these changes
Oct 1, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for patience, all looks great!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #5441
This small change explicitly passes the
for_artist
keyword to theplugins.feat_tokens
function that constructs the regex for matching existing "feat. X" parts in song titles.Previously, it was not passed and set to the default (
True
), which caused using the pattern intended for the artist field to also be used for the title field.This caused some false positives as shown in #5441
To Do