mirror of
https://github.com/beetbox/beets.git
synced 2025-12-09 18:12:19 +01:00
feat(FtInTitle): Support tracks by artists != album artist (#5943)
I have different expectations around this [FtInTitle test case](fcc9341360/test/plugins/test_ftintitle.py (L147-L151)), which represents a song on an album by a **guest artist featuring a third artist**: ``` { "artist": "Alice ft. Carol", "album_artist": "Bob", "feat_part": None, }, ``` If this were Alice's album, the plugin would process the track, moving `Carol` to the `feat_part` and changing the `artist` to `Alice`. But because it's Bob's album, nothing happens. I don't want `Alice ft. Carol` as an artist on Bob's album any more than I want it on Alice's though 😭 This may be a bit of a corner case but it does happen reasonably often in the wild: ``` Album: Flying Lotus - ASH (OST) (2025) Track 26: Kuedo feat. Miguel Atwood-Ferguson - WHAT'S WRONG PEACH? ``` More commonly, this case applies to __compilation albums which use `Various Artists` as an albumartist__. Processing doesn't occur on these tracks in the current implementation because of how [`find_feat_part()`](fcc9341360/beetsplug/ftintitle.py (L57-L82)) kicks off: ``` # 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: return None ``` As best I can tell, the code is setup this way to enable parsing of more complex cases , like a song by `Hall & Oates` on an Oates album, which is pretty clever. But giving up in cases where albumartist isn't in the artist field seems premature when a reasonable parsing method exists that doesn't require it. So this PR proposes modifications to code and tests that enables FtInTitle to process items whose artist doesn't contain the albumartist. It seems like what someone loading a `FtInTitle` plugin would want by default, but if that's a contentious take then I'm happy to put it behind a config flag. Thanks for your consideration!
This commit is contained in:
commit
a12ca093cb
3 changed files with 261 additions and 185 deletions
|
|
@ -26,14 +26,16 @@ if TYPE_CHECKING:
|
|||
from beets.library import Item
|
||||
|
||||
|
||||
def split_on_feat(artist: str) -> tuple[str, str | None]:
|
||||
def split_on_feat(
|
||||
artist: str, for_artist: bool = True
|
||||
) -> tuple[str, str | None]:
|
||||
"""Given an artist string, split the "main" artist from any artist
|
||||
on the right-hand side of a string like "feat". Return the main
|
||||
artist, which is always a string, and the featuring artist, which
|
||||
may be a string or None if none is present.
|
||||
"""
|
||||
# split on the first "feat".
|
||||
regex = re.compile(plugins.feat_tokens(), re.IGNORECASE)
|
||||
regex = re.compile(plugins.feat_tokens(for_artist), re.IGNORECASE)
|
||||
parts = tuple(s.strip() for s in regex.split(artist, 1))
|
||||
if len(parts) == 1:
|
||||
return parts[0], None
|
||||
|
|
@ -53,32 +55,35 @@ def contains_feat(title: str) -> bool:
|
|||
)
|
||||
|
||||
|
||||
def find_feat_part(artist: str, albumartist: str) -> str | None:
|
||||
def find_feat_part(artist: str, albumartist: str | None) -> str | None:
|
||||
"""Attempt to find featured artists in the item's artist fields and
|
||||
return the results. Returns None if no featured artist found.
|
||||
"""
|
||||
# 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:
|
||||
return None
|
||||
# Handle a wider variety of extraction cases if the album artist is
|
||||
# contained within the track artist.
|
||||
if albumartist and albumartist in artist:
|
||||
albumartist_split = artist.split(albumartist, 1)
|
||||
|
||||
# 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])
|
||||
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
|
||||
# featured artist.
|
||||
if albumartist_split[1] != "":
|
||||
# Extract the featured artist from the right-hand side.
|
||||
_, feat_part = split_on_feat(albumartist_split[1])
|
||||
return feat_part
|
||||
|
||||
# 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 lhs:
|
||||
return lhs
|
||||
# Otherwise, if there's nothing on the right-hand side,
|
||||
# look for a featuring artist on the left-hand side.
|
||||
else:
|
||||
lhs, _ = split_on_feat(albumartist_split[0])
|
||||
if lhs:
|
||||
return lhs
|
||||
|
||||
return None
|
||||
# Fall back to conservative handling of the track artist without relying
|
||||
# on albumartist, which covers compilations using a 'Various Artists'
|
||||
# albumartist and album tracks by a guest artist featuring a third artist.
|
||||
_, feat_part = split_on_feat(artist, False)
|
||||
return feat_part
|
||||
|
||||
|
||||
class FtInTitlePlugin(plugins.BeetsPlugin):
|
||||
|
|
@ -153,8 +158,9 @@ class FtInTitlePlugin(plugins.BeetsPlugin):
|
|||
"artist: {.artist} (Not changing due to keep_in_artist)", item
|
||||
)
|
||||
else:
|
||||
self._log.info("artist: {0.artist} -> {0.albumartist}", item)
|
||||
item.artist = item.albumartist
|
||||
track_artist, _ = split_on_feat(item.artist)
|
||||
self._log.info("artist: {0.artist} -> {1}", item, track_artist)
|
||||
item.artist = track_artist
|
||||
|
||||
if item.artist_sort:
|
||||
# Just strip the featured artist from the sort name.
|
||||
|
|
@ -187,7 +193,7 @@ class FtInTitlePlugin(plugins.BeetsPlugin):
|
|||
# Check whether there is a featured artist on this track and the
|
||||
# artist field does not exactly match the album artist field. In
|
||||
# that case, we attempt to move the featured artist to the title.
|
||||
if not albumartist or albumartist == artist:
|
||||
if albumartist and artist == albumartist:
|
||||
return False
|
||||
|
||||
_, featured = split_on_feat(artist)
|
||||
|
|
|
|||
|
|
@ -161,6 +161,9 @@ Other changes:
|
|||
Autogenerated API references are now located in the ``docs/api`` subdirectory.
|
||||
- :doc:`/plugins/substitute`: Fix rST formatting for example cases so that each
|
||||
case is shown on separate lines.
|
||||
- :doc:`/plugins/ftintitle`: Process items whose albumartist is not contained in
|
||||
the artist field, including compilations using Various Artists as an
|
||||
albumartist and album tracks by guest artists featuring a third artist.
|
||||
- Refactored library.py file by splitting it into multiple modules within the
|
||||
beets/library directory.
|
||||
- Added a test to check that all plugins can be imported without errors.
|
||||
|
|
|
|||
|
|
@ -14,8 +14,11 @@
|
|||
|
||||
"""Tests for the 'ftintitle' plugin."""
|
||||
|
||||
import unittest
|
||||
from typing import Dict, Generator, Optional, Tuple, Union
|
||||
|
||||
import pytest
|
||||
|
||||
from beets.library.models import Item
|
||||
from beets.test.helper import PluginTestCase
|
||||
from beetsplug import ftintitle
|
||||
|
||||
|
|
@ -23,169 +26,233 @@ from beetsplug import ftintitle
|
|||
class FtInTitlePluginFunctional(PluginTestCase):
|
||||
plugin = "ftintitle"
|
||||
|
||||
def _ft_add_item(self, path, artist, title, aartist):
|
||||
return self.add_item(
|
||||
path=path,
|
||||
artist=artist,
|
||||
artist_sort=artist,
|
||||
title=title,
|
||||
albumartist=aartist,
|
||||
)
|
||||
|
||||
def _ft_set_config(
|
||||
self, ftformat, drop=False, auto=True, keep_in_artist=False
|
||||
):
|
||||
self.config["ftintitle"]["format"] = ftformat
|
||||
self.config["ftintitle"]["drop"] = drop
|
||||
self.config["ftintitle"]["auto"] = auto
|
||||
self.config["ftintitle"]["keep_in_artist"] = keep_in_artist
|
||||
|
||||
def test_functional_drop(self):
|
||||
item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice")
|
||||
self.run_command("ftintitle", "-d")
|
||||
item.load()
|
||||
assert item["artist"] == "Alice"
|
||||
assert item["title"] == "Song 1"
|
||||
|
||||
def test_functional_not_found(self):
|
||||
item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "George")
|
||||
self.run_command("ftintitle", "-d")
|
||||
item.load()
|
||||
# item should be unchanged
|
||||
assert item["artist"] == "Alice ft Bob"
|
||||
assert item["title"] == "Song 1"
|
||||
|
||||
def test_functional_custom_format(self):
|
||||
self._ft_set_config("feat. {}")
|
||||
item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice")
|
||||
self.run_command("ftintitle")
|
||||
item.load()
|
||||
assert item["artist"] == "Alice"
|
||||
assert item["title"] == "Song 1 feat. Bob"
|
||||
|
||||
self._ft_set_config("featuring {}")
|
||||
item = self._ft_add_item("/", "Alice feat. Bob", "Song 1", "Alice")
|
||||
self.run_command("ftintitle")
|
||||
item.load()
|
||||
assert item["artist"] == "Alice"
|
||||
assert item["title"] == "Song 1 featuring Bob"
|
||||
|
||||
self._ft_set_config("with {}")
|
||||
item = self._ft_add_item("/", "Alice feat Bob", "Song 1", "Alice")
|
||||
self.run_command("ftintitle")
|
||||
item.load()
|
||||
assert item["artist"] == "Alice"
|
||||
assert item["title"] == "Song 1 with Bob"
|
||||
|
||||
def test_functional_keep_in_artist(self):
|
||||
self._ft_set_config("feat. {}", keep_in_artist=True)
|
||||
item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice")
|
||||
self.run_command("ftintitle")
|
||||
item.load()
|
||||
assert item["artist"] == "Alice ft Bob"
|
||||
assert item["title"] == "Song 1 feat. Bob"
|
||||
|
||||
item = self._ft_add_item("/", "Alice ft Bob", "Song 1", "Alice")
|
||||
self.run_command("ftintitle", "-d")
|
||||
item.load()
|
||||
assert item["artist"] == "Alice ft Bob"
|
||||
assert item["title"] == "Song 1"
|
||||
@pytest.fixture
|
||||
def env() -> Generator[FtInTitlePluginFunctional, None, None]:
|
||||
case = FtInTitlePluginFunctional(methodName="runTest")
|
||||
case.setUp()
|
||||
try:
|
||||
yield case
|
||||
finally:
|
||||
case.tearDown()
|
||||
|
||||
|
||||
class FtInTitlePluginTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
"""Set up configuration"""
|
||||
ftintitle.FtInTitlePlugin()
|
||||
def set_config(
|
||||
env: FtInTitlePluginFunctional, cfg: Optional[Dict[str, Union[str, bool]]]
|
||||
) -> None:
|
||||
cfg = {} if cfg is None else cfg
|
||||
defaults = {
|
||||
"drop": False,
|
||||
"auto": True,
|
||||
"keep_in_artist": False,
|
||||
}
|
||||
env.config["ftintitle"].set(defaults)
|
||||
env.config["ftintitle"].set(cfg)
|
||||
|
||||
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. Carol",
|
||||
"album_artist": "Bob",
|
||||
"feat_part": None,
|
||||
},
|
||||
]
|
||||
|
||||
for test_case in test_cases:
|
||||
feat_part = ftintitle.find_feat_part(
|
||||
test_case["artist"], test_case["album_artist"]
|
||||
)
|
||||
assert feat_part == test_case["feat_part"]
|
||||
def add_item(
|
||||
env: FtInTitlePluginFunctional,
|
||||
path: str,
|
||||
artist: str,
|
||||
title: str,
|
||||
albumartist: Optional[str],
|
||||
) -> Item:
|
||||
return env.add_item(
|
||||
path=path,
|
||||
artist=artist,
|
||||
artist_sort=artist,
|
||||
title=title,
|
||||
albumartist=albumartist,
|
||||
)
|
||||
|
||||
def test_split_on_feat(self):
|
||||
parts = ftintitle.split_on_feat("Alice ft. Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice feat Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice feat. Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice featuring Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice & Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice and Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice With Bob")
|
||||
assert parts == ("Alice", "Bob")
|
||||
parts = ftintitle.split_on_feat("Alice defeat Bob")
|
||||
assert parts == ("Alice defeat Bob", None)
|
||||
|
||||
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 (ft. Bob)")
|
||||
assert ftintitle.contains_feat("Alice (feat. Bob)")
|
||||
assert ftintitle.contains_feat("Alice [ft. Bob]")
|
||||
assert ftintitle.contains_feat("Alice [feat. Bob]")
|
||||
assert not ftintitle.contains_feat("Alice defeat Bob")
|
||||
assert not ftintitle.contains_feat("Aliceft.Bob")
|
||||
assert not ftintitle.contains_feat("Alice (defeat Bob)")
|
||||
assert not ftintitle.contains_feat("Live and Let Go")
|
||||
assert not ftintitle.contains_feat("Come With Me")
|
||||
@pytest.mark.parametrize(
|
||||
"cfg, cmd_args, given, expected",
|
||||
[
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle",),
|
||||
("Alice", "Song 1", "Alice"),
|
||||
("Alice", "Song 1"),
|
||||
id="no-featured-artist",
|
||||
),
|
||||
pytest.param(
|
||||
{"format": "feat {0}"},
|
||||
("ftintitle",),
|
||||
("Alice ft. Bob", "Song 1", None),
|
||||
("Alice", "Song 1 feat Bob"),
|
||||
id="no-albumartist-custom-format",
|
||||
),
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle",),
|
||||
("Alice", "Song 1", None),
|
||||
("Alice", "Song 1"),
|
||||
id="no-albumartist-no-feature",
|
||||
),
|
||||
pytest.param(
|
||||
{"format": "featuring {0}"},
|
||||
("ftintitle",),
|
||||
("Alice ft Bob", "Song 1", "George"),
|
||||
("Alice", "Song 1 featuring Bob"),
|
||||
id="guest-artist-custom-format",
|
||||
),
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle",),
|
||||
("Alice", "Song 1", "George"),
|
||||
("Alice", "Song 1"),
|
||||
id="guest-artist-no-feature",
|
||||
),
|
||||
# ---- drop (-d) variants ----
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle", "-d"),
|
||||
("Alice ft Bob", "Song 1", "Alice"),
|
||||
("Alice", "Song 1"),
|
||||
id="drop-self-ft",
|
||||
),
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle", "-d"),
|
||||
("Alice", "Song 1", "Alice"),
|
||||
("Alice", "Song 1"),
|
||||
id="drop-self-no-ft",
|
||||
),
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle", "-d"),
|
||||
("Alice ft Bob", "Song 1", "George"),
|
||||
("Alice", "Song 1"),
|
||||
id="drop-guest-ft",
|
||||
),
|
||||
pytest.param(
|
||||
None,
|
||||
("ftintitle", "-d"),
|
||||
("Alice", "Song 1", "George"),
|
||||
("Alice", "Song 1"),
|
||||
id="drop-guest-no-ft",
|
||||
),
|
||||
# ---- custom format variants ----
|
||||
pytest.param(
|
||||
{"format": "feat. {}"},
|
||||
("ftintitle",),
|
||||
("Alice ft Bob", "Song 1", "Alice"),
|
||||
("Alice", "Song 1 feat. Bob"),
|
||||
id="custom-format-feat-dot",
|
||||
),
|
||||
pytest.param(
|
||||
{"format": "featuring {}"},
|
||||
("ftintitle",),
|
||||
("Alice feat. Bob", "Song 1", "Alice"),
|
||||
("Alice", "Song 1 featuring Bob"),
|
||||
id="custom-format-featuring",
|
||||
),
|
||||
pytest.param(
|
||||
{"format": "with {}"},
|
||||
("ftintitle",),
|
||||
("Alice feat Bob", "Song 1", "Alice"),
|
||||
("Alice", "Song 1 with Bob"),
|
||||
id="custom-format-with",
|
||||
),
|
||||
# ---- keep_in_artist variants ----
|
||||
pytest.param(
|
||||
{"format": "feat. {}", "keep_in_artist": True},
|
||||
("ftintitle",),
|
||||
("Alice ft Bob", "Song 1", "Alice"),
|
||||
("Alice ft Bob", "Song 1 feat. Bob"),
|
||||
id="keep-in-artist-add-to-title",
|
||||
),
|
||||
pytest.param(
|
||||
{"format": "feat. {}", "keep_in_artist": True},
|
||||
("ftintitle", "-d"),
|
||||
("Alice ft Bob", "Song 1", "Alice"),
|
||||
("Alice ft Bob", "Song 1"),
|
||||
id="keep-in-artist-drop-from-title",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_ftintitle_functional(
|
||||
env: FtInTitlePluginFunctional,
|
||||
cfg: Optional[Dict[str, Union[str, bool]]],
|
||||
cmd_args: Tuple[str, ...],
|
||||
given: Tuple[str, str, Optional[str]],
|
||||
expected: Tuple[str, str],
|
||||
) -> None:
|
||||
set_config(env, cfg)
|
||||
ftintitle.FtInTitlePlugin()
|
||||
|
||||
artist, title, albumartist = given
|
||||
item = add_item(env, "/", artist, title, albumartist)
|
||||
|
||||
env.run_command(*cmd_args)
|
||||
item.load()
|
||||
|
||||
expected_artist, expected_title = expected
|
||||
assert item["artist"] == expected_artist
|
||||
assert item["title"] == expected_title
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"artist,albumartist,expected",
|
||||
[
|
||||
("Alice ft. Bob", "Alice", "Bob"),
|
||||
("Alice feat Bob", "Alice", "Bob"),
|
||||
("Alice featuring Bob", "Alice", "Bob"),
|
||||
("Alice & Bob", "Alice", "Bob"),
|
||||
("Alice and Bob", "Alice", "Bob"),
|
||||
("Alice With Bob", "Alice", "Bob"),
|
||||
("Alice defeat Bob", "Alice", None),
|
||||
("Alice & Bob", "Bob", "Alice"),
|
||||
("Alice ft. Bob", "Bob", "Alice"),
|
||||
("Alice ft. Carol", "Bob", "Carol"),
|
||||
],
|
||||
)
|
||||
def test_find_feat_part(
|
||||
artist: str,
|
||||
albumartist: str,
|
||||
expected: Optional[str],
|
||||
) -> None:
|
||||
assert ftintitle.find_feat_part(artist, albumartist) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"given,expected",
|
||||
[
|
||||
("Alice ft. Bob", ("Alice", "Bob")),
|
||||
("Alice feat Bob", ("Alice", "Bob")),
|
||||
("Alice feat. Bob", ("Alice", "Bob")),
|
||||
("Alice featuring Bob", ("Alice", "Bob")),
|
||||
("Alice & Bob", ("Alice", "Bob")),
|
||||
("Alice and Bob", ("Alice", "Bob")),
|
||||
("Alice With Bob", ("Alice", "Bob")),
|
||||
("Alice defeat Bob", ("Alice defeat Bob", None)),
|
||||
],
|
||||
)
|
||||
def test_split_on_feat(
|
||||
given: str,
|
||||
expected: Tuple[str, Optional[str]],
|
||||
) -> None:
|
||||
assert ftintitle.split_on_feat(given) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"given,expected",
|
||||
[
|
||||
("Alice ft. Bob", True),
|
||||
("Alice feat. Bob", True),
|
||||
("Alice feat Bob", True),
|
||||
("Alice featuring Bob", True),
|
||||
("Alice (ft. Bob)", True),
|
||||
("Alice (feat. Bob)", True),
|
||||
("Alice [ft. Bob]", True),
|
||||
("Alice [feat. Bob]", True),
|
||||
("Alice defeat Bob", False),
|
||||
("Aliceft.Bob", False),
|
||||
("Alice (defeat Bob)", False),
|
||||
("Live and Let Go", False),
|
||||
("Come With Me", False),
|
||||
],
|
||||
)
|
||||
def test_contains_feat(given: str, expected: bool) -> None:
|
||||
assert ftintitle.contains_feat(given) is expected
|
||||
|
|
|
|||
Loading…
Reference in a new issue