Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions beets/autotag/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@

from beets import config, logging, plugins
from beets.util import cached_classproperty, unique_list
from beets.util.deprecation import deprecate_for_maintainers
from beets.util.deprecation import (
ALBUM_LEGACY_TO_LIST_FIELD,
ITEM_LEGACY_TO_LIST_FIELD,
deprecate_for_maintainers,
)

if TYPE_CHECKING:
from collections.abc import Sequence
Expand Down Expand Up @@ -136,7 +140,7 @@ class Info(AttrDict[Any]):

IGNORED_FIELDS: ClassVar[set[str]] = {"data_url"}
MEDIA_FIELD_MAP: ClassVar[dict[str, str]] = {}
LEGACY_TO_LIST_FIELD: ClassVar[dict[str, str]] = {"genre": "genres"}
LEGACY_TO_LIST_FIELD: ClassVar[dict[str, str]]

@cached_classproperty
def nullable_fields(cls) -> set[str]:
Expand Down Expand Up @@ -281,6 +285,7 @@ class AlbumInfo(Info):
"releasegroup_id": "mb_releasegroupid",
"va": "comp",
}
LEGACY_TO_LIST_FIELD: ClassVar[dict[str, str]] = ALBUM_LEGACY_TO_LIST_FIELD

@property
def id(self) -> str | None:
Expand Down Expand Up @@ -388,13 +393,7 @@ class TrackInfo(Info):
"track_id": "mb_trackid",
"medium_index": "track",
}
LEGACY_TO_LIST_FIELD: ClassVar[dict[str, str]] = {
**Info.LEGACY_TO_LIST_FIELD,
"remixer": "remixers",
"lyricist": "lyricists",
"composer": "composers",
"arranger": "arrangers",
}
LEGACY_TO_LIST_FIELD: ClassVar[dict[str, str]] = ITEM_LEGACY_TO_LIST_FIELD

@property
def id(self) -> str | None:
Expand Down
3 changes: 3 additions & 0 deletions beets/library/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
samefile,
syspath,
)
from beets.util.deprecation import maybe_replace_legacy_field
from beets.util.functemplate import Template, template

from .exceptions import FileOperationError, ReadError, WriteError
Expand Down Expand Up @@ -101,6 +102,8 @@ def field_query(
cls, field: str, pattern: str, query_cls: FieldQueryType
) -> FieldQuery:
"""Get a `FieldQuery` for the given field on this model."""
field = maybe_replace_legacy_field(field, cls is Album)

fast = field in cls.all_db_fields
if field in cls.shared_db_fields:
# This field exists in both tables, so SQLite will encounter
Expand Down
9 changes: 7 additions & 2 deletions beets/ui/commands/modify.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from beets import library, ui
from beets.util import functemplate
from beets.util.deprecation import maybe_replace_legacy_field

from .utils import do_query

Expand Down Expand Up @@ -79,10 +80,13 @@ def print_and_modify(obj, mods, dels):
return ui.show_model_changes(obj)


def modify_parse_args(args):
def modify_parse_args(args, is_album: bool):
"""Split the arguments for the modify subcommand into query parts,
assignments (field=value), and deletions (field!). Returns the result as
a three-tuple in that order.

Replace legacy string fields with list equivalents, and supply deprecation
warnings for the user.
"""
mods = {}
dels = []
Expand All @@ -92,14 +96,15 @@ def modify_parse_args(args):
dels.append(arg[:-1]) # Strip trailing !.
elif "=" in arg and ":" not in arg.split("=", 1)[0]:
key, val = arg.split("=", 1)
key = maybe_replace_legacy_field(key, is_album, modify=True)
mods[key] = val
else:
query.append(arg)
return query, mods, dels


def modify_func(lib, opts, args):
query, mods, dels = modify_parse_args(args)
query, mods, dels = modify_parse_args(args, is_album=opts.album)
if not mods and not dels:
raise ui.UserError("no modifications specified")
modify_items(
Expand Down
31 changes: 31 additions & 0 deletions beets/util/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,26 @@
from packaging.version import Version

import beets
from beets import logging

if TYPE_CHECKING:
from logging import Logger


ALBUM_LEGACY_TO_LIST_FIELD = {
"genre": "genres",
}
ITEM_LEGACY_TO_LIST_FIELD = {
"genre": "genres",
"arranger": "arrangers",
"composer": "composers",
"lyricist": "lyricists",
"remixer": "remixers",
}

log = logging.getLogger("beets")


def _format_message(old: str, new: str | None = None) -> str:
next_major = f"{Version(beets.__version__).major + 1}.0.0"
msg = f"{old} is deprecated and will be removed in version {next_major}."
Expand Down Expand Up @@ -58,3 +73,19 @@ def deprecate_imports(

return getattr(import_module(new_module), name)
raise AttributeError(f"module '{old_module}' has no attribute '{name}'")


def maybe_replace_legacy_field(
field: str, is_album: bool, modify: bool = False
) -> str:
legacy_to_list_field = (
ALBUM_LEGACY_TO_LIST_FIELD if is_album else ITEM_LEGACY_TO_LIST_FIELD
)
if list_field := legacy_to_list_field.get(field):
new = f"'{list_field}'"
if modify:
new += " (separate values by '; ')"
deprecate_for_user(log, f"The '{field}' field", new)
return list_field

return field
6 changes: 6 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ Bug fixes
- :doc:`plugins/deezer`: Fix a regression in 2.8.0 where selecting a Deezer
match during import could crash with ``AttributeError: 'AlbumInfo' object has
no attribute 'raw_data'`` when Deezer returned numeric artist IDs. :bug:`6503`
- :ref:`modify-cmd` accepts legacy singular field names such as ``genre``,
``composer``, ``lyricist``, ``remixer``, and ``arranger`` in assignments,
rewrites them to the corresponding multi-valued fields, and warns users to
switch to the plural field names. :ref:`list-cmd`, and query expressions,
accept the same legacy singular field names and warn users to switch to the
plural field names. :bug:`6483`

For plugin developers
~~~~~~~~~~~~~~~~~~~~~
Expand Down
5 changes: 5 additions & 0 deletions docs/reference/cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,11 @@ To adjust a multi-valued field, such as ``genres``, ``remixers``, ``lyricists``,
``composers``, or ``arrangers``, separate the values with |semicolon_space|. For
example, ``beet modify genres="rock; pop"``.

For compatibility, ``modify`` assignments and query expressions still accept
legacy singular names such as ``genre``, ``composer``, ``lyricist``,
``remixer``, and ``arranger``, but beets will warn and translate them to the
plural multi-valued fields. Prefer the plural field names in new commands.

The ``-a`` option changes to querying album fields instead of track fields and
also enables to operate on albums in addition to the individual tracks. Without
this flag, the command will only change *track-level* data, even if all the
Expand Down
5 changes: 2 additions & 3 deletions test/autotag/test_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
from beets.autotag.hooks import (
AlbumInfo,
AlbumMatch,
Info,
TrackInfo,
TrackMatch,
correct_list_fields,
Expand Down Expand Up @@ -76,7 +75,7 @@ def test_init_info(
self, str_value, list_value, expected_warning, expected_list_value
):
with expected_warning:
actual_list_value = Info._get_list_from_string_value(
actual_list_value = TrackInfo._get_list_from_string_value(
"genre", "genres", str_value, list_value
)

Expand All @@ -85,7 +84,7 @@ def test_init_info(
def test_set_str_value(
self, str_value, list_value, expected_warning, expected_list_value
):
info = Info(genres=list_value)
info = TrackInfo(genres=list_value)
with expected_warning:
info["genre"] = str_value

Expand Down
27 changes: 27 additions & 0 deletions test/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

"""Various tests for querying the library database."""

import logging
import sys
from functools import partial
from pathlib import Path
Expand Down Expand Up @@ -72,6 +73,7 @@ def lib(self, helper):
year=2001,
comp=True,
genres=["rock"],
composers=["composer"],
),
helper.create_item(
title="second",
Expand Down Expand Up @@ -239,6 +241,31 @@ def test_fast_vs_slow(self, lib, make_q):
map(dict, lib.items(q_slow))
)

@pytest.mark.parametrize(
"q, legacy_field",
[
pytest.param("genres::rock", None, id="non-legacy-genres-field"),
pytest.param("genre::rock", "genre", id="legacy-genre-field"),
pytest.param(
"composers::composer", None, id="non-legacy-composer-field"
),
pytest.param(
"composer::composer", "composer", id="legacy-composer-field"
),
],
)
def test_legacy_field(self, caplog, lib, q, legacy_field):
with caplog.at_level(logging.WARNING, logger="beets"):
actual_titles = {i.title for i in lib.items(q)}

assert actual_titles == {"first"}
if legacy_field:
assert caplog.records, "No log records were captured"
assert len(caplog.records) == 1
message = str(caplog.records[0].msg)
assert f"The '{legacy_field}' field is deprecated" in message
assert f"Use '{legacy_field}s' instead." in message


class TestMatch:
@pytest.fixture(scope="class")
Expand Down
42 changes: 38 additions & 4 deletions test/ui/commands/test_modify.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import unittest

import pytest
from mediafile import MediaFile

from beets import logging
from beets.test.helper import BeetsTestCase, IOMixin
from beets.ui.commands.modify import modify_parse_args
from beets.util import syspath
Expand Down Expand Up @@ -193,23 +195,55 @@ def test_delete_initial_key_tag(self):
assert mediafile.initial_key is None

def test_arg_parsing_colon_query(self):
query, mods, _ = modify_parse_args(["title:oldTitle", "title=newTitle"])
query, mods, _ = modify_parse_args(
["title:oldTitle", "title=newTitle"], is_album=False
)
assert query == ["title:oldTitle"]
assert mods == {"title": "newTitle"}

def test_arg_parsing_delete(self):
query, _, dels = modify_parse_args(["title:oldTitle", "title!"])
query, _, dels = modify_parse_args(
["title:oldTitle", "title!"], is_album=False
)
assert query == ["title:oldTitle"]
assert dels == ["title"]

def test_arg_parsing_query_with_exclaimation(self):
query, mods, _ = modify_parse_args(
["title:oldTitle!", "title=newTitle!"]
["title:oldTitle!", "title=newTitle!"], is_album=False
)
assert query == ["title:oldTitle!"]
assert mods == {"title": "newTitle!"}

def test_arg_parsing_equals_in_value(self):
query, mods, _ = modify_parse_args(["title:foo=bar", "title=newTitle"])
query, mods, _ = modify_parse_args(
["title:foo=bar", "title=newTitle"], is_album=False
)
assert query == ["title:foo=bar"]
assert mods == {"title": "newTitle"}


@pytest.mark.parametrize(
"is_album, legacy_field, list_field",
[
pytest.param(True, "genre", "genres", id="album-genre"),
pytest.param(False, "genre", "genres", id="item-genre"),
pytest.param(False, "composer", "composers", id="item-composer"),
],
)
def test_arg_parsing_rewrites_legacy_list_fields(
is_album, legacy_field, list_field, caplog
):
with caplog.at_level(logging.WARNING, logger="beets"):
query, mods, dels = modify_parse_args(
[f"{legacy_field}=value1; value2"], is_album=is_album
)

assert query == []
assert mods == {list_field: "value1; value2"}
assert dels == []
assert caplog.records, "No log records were captured"
assert len(caplog.records) == 1
message = str(caplog.records[0].msg)
assert f"The '{legacy_field}' field is deprecated" in message
assert f"Use '{list_field}' (separate values by '; ') instead." in message
Loading