Skip to content
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

Multi genres / only "genres" field wip #5606

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion beets/autotag/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def _apply_metadata(
if value is None and field not in nullable_fields:
continue

db_obj[field] = value
setattr(db_obj, field, value)


def correct_list_fields(m: LibModel) -> None:
Expand Down
5 changes: 4 additions & 1 deletion beets/autotag/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
return id(self)


class AlbumInfo(AttrDict):

Check failure on line 59 in beets/autotag/hooks.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "AttrDict"
"""Describes a canonical release that may be used to match a release
in the library. Consists of these data members:

Expand Down Expand Up @@ -100,6 +100,7 @@
country: str | None = None,
style: str | None = None,
genre: str | None = None,
genres: str | None = None,
albumstatus: str | None = None,
media: str | None = None,
albumdisambig: str | None = None,
Expand Down Expand Up @@ -143,6 +144,7 @@
self.country = country
self.style = style
self.genre = genre
self.genres = genres or ([genre] if genre else [])
self.albumstatus = albumstatus
self.media = media
self.albumdisambig = albumdisambig
Expand Down Expand Up @@ -212,6 +214,7 @@
bpm: str | None = None,
initial_key: str | None = None,
genre: str | None = None,
genres: str | None = None,
album: str | None = None,
**kwargs,
):
Expand Down Expand Up @@ -245,7 +248,7 @@
self.work_disambig = work_disambig
self.bpm = bpm
self.initial_key = initial_key
self.genre = genre
self.genres = genres
self.album = album
self.update(kwargs)

Expand Down
5 changes: 2 additions & 3 deletions beets/autotag/mb.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@
)


def _preferred_alias(aliases: list):

Check failure on line 135 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "list"
"""Given an list of alias structures for an artist credit, select
and return the user's preferred alias alias or None if no matching
alias is found.
Expand Down Expand Up @@ -173,7 +173,7 @@
default release event if a preferred event is not found.
"""
countries = config["match"]["preferred"]["countries"].as_str_seq()
countries = cast(Sequence, countries)

Check failure on line 176 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "Sequence"

for country in countries:
for event in release.get("release-event-list", {}):
Expand All @@ -187,7 +187,7 @@


def _multi_artist_credit(
credit: list[dict], include_join_phrase: bool

Check failure on line 190 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
) -> tuple[list[str], list[str], list[str]]:
"""Given a list representing an ``artist-credit`` block, accumulate
data into a triple of joined artist name lists: canonical, sort, and
Expand Down Expand Up @@ -235,7 +235,7 @@
)


def _flatten_artist_credit(credit: list[dict]) -> tuple[str, str, str]:

Check failure on line 238 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
"""Given a list representing an ``artist-credit`` block, flatten the
data into a triple of joined artist name strings: canonical, sort, and
credit.
Expand All @@ -250,7 +250,7 @@
)


def _artist_ids(credit: list[dict]) -> list[str]:

Check failure on line 253 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
"""
Given a list representing an ``artist-credit``,
return a list of artist IDs
Expand All @@ -277,7 +277,7 @@


def track_info(
recording: dict,

Check failure on line 280 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
index: int | None = None,
medium: int | None = None,
medium_index: int | None = None,
Expand Down Expand Up @@ -401,7 +401,7 @@
setattr(info, key, date_num)


def album_info(release: dict) -> beets.autotag.hooks.AlbumInfo:

Check failure on line 404 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
"""Takes a MusicBrainz release result dictionary and returns a beets
AlbumInfo object containing the interesting data about that release.
"""
Expand Down Expand Up @@ -614,10 +614,10 @@
for source in sources:
for genreitem in source:
genres[genreitem["name"]] += int(genreitem["count"])
info.genre = "; ".join(
info.genres = [
genre
for genre, _count in sorted(genres.items(), key=lambda g: -g[1])
)
]

# We might find links to external sources (Discogs, Bandcamp, ...)
external_ids = config["musicbrainz"]["external_ids"].get()
Expand Down Expand Up @@ -757,8 +757,8 @@


def _find_actual_release_from_pseudo_release(
pseudo_rel: dict,

Check failure on line 760 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
) -> dict | None:

Check failure on line 761 in beets/autotag/mb.py

View workflow job for this annotation

GitHub Actions / Check types with mypy

Missing type parameters for generic type "dict"
try:
relations = pseudo_rel["release"]["release-relation-list"]
except KeyError:
Expand Down Expand Up @@ -808,7 +808,6 @@
"barcode",
"asin",
"style",
"genre",
]
}
merged.update(from_actual)
Expand Down
12 changes: 8 additions & 4 deletions beets/dbcore/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ def copy(self) -> Model:
# Essential field accessors.

@classmethod
def _type(cls, key) -> types.Type:
def _type(cls, key):
"""Get the type of a field, a `Type` instance.

If the field has no explicit type, it is given the base `Type`,
Expand Down Expand Up @@ -528,7 +528,7 @@ def all_keys(cls):
def update(self, values):
"""Assign all values in the given dict."""
for key, value in values.items():
self[key] = value
setattr(self, key, value)

def items(self) -> Iterator[tuple[str, Any]]:
"""Iterate over (key, value) pairs that this object contains.
Expand Down Expand Up @@ -559,7 +559,11 @@ def __getattr__(self, key):
raise AttributeError(f"no such field {key!r}")

def __setattr__(self, key, value):
if key.startswith("_"):
if (
key.startswith("_")
or key in dir(self)
and isinstance(getattr(self.__class__, key), property)
):
super().__setattr__(key, value)
else:
self[key] = value
Expand Down Expand Up @@ -714,7 +718,7 @@ def _parse(cls, key, string: str) -> Any:

def set_parse(self, key, string: str):
"""Set the object's key to a value represented by a string."""
self[key] = self._parse(key, string)
setattr(self, key, self._parse(key, string))


# Database controller and supporting interfaces.
Expand Down
8 changes: 7 additions & 1 deletion beets/dbcore/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,13 @@ def parse(self, string: str):
return []
return string.split(self.delimiter)

def to_sql(self, model_value: list[str]):
def normalize(self, value: Any) -> list[str]:
if not value:
return []

return value.split(self.delimiter) if isinstance(value, str) else value

def to_sql(self, model_value: list[str]) -> str:
return self.delimiter.join(model_value)


Expand Down
57 changes: 41 additions & 16 deletions beets/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,27 @@ class LibModel(dbcore.Model["Library"]):
def writable_media_fields(cls) -> set[str]:
return set(MediaFile.fields()) & cls._fields.keys()

@property
def genre(self) -> str:
_type: types.DelimitedString = self._type("genres")
return _type.to_sql(self.get("genres"))

@genre.setter
def genre(self, value: str) -> None:
self.genres = value

@classmethod
def _getters(cls):
return {
"genre": lambda m: cls._fields["genres"].delimiter.join(m.genres)
}

def _template_funcs(self):
funcs = DefaultTemplateFunctions(self, self._db).functions()
funcs.update(plugins.template_funcs())
return funcs
return {
**DefaultTemplateFunctions(self, self._db).functions(),
**plugins.template_funcs(),
"genre": "$genres",
}

def store(self, fields=None):
super().store(fields)
Expand Down Expand Up @@ -533,7 +550,7 @@ class Item(LibModel):
"albumartists_sort": types.MULTI_VALUE_DSV,
"albumartist_credit": types.STRING,
"albumartists_credit": types.MULTI_VALUE_DSV,
"genre": types.STRING,
"genres": types.SEMICOLON_SPACE_DSV,
"style": types.STRING,
"discogs_albumid": types.INTEGER,
"discogs_artistid": types.INTEGER,
Expand Down Expand Up @@ -614,7 +631,7 @@ class Item(LibModel):
"comments",
"album",
"albumartist",
"genre",
"genres",
)

_types = {
Expand Down Expand Up @@ -689,10 +706,12 @@ def _cached_album(self, album):

@classmethod
def _getters(cls):
getters = plugins.item_field_getters()
getters["singleton"] = lambda i: i.album_id is None
getters["filesize"] = Item.try_filesize # In bytes.
return getters
return {
**plugins.item_field_getters(),
"singleton": lambda i: i.album_id is None,
"filesize": Item.try_filesize, # In bytes.
"genre": lambda i: cls._fields["genres"].delimiter.join(i.genres),
}

def duplicates_query(self, fields: list[str]) -> dbcore.AndQuery:
"""Return a query for entities with same values in the given fields."""
Expand Down Expand Up @@ -768,6 +787,10 @@ def get(self, key, default=None, with_album=True):

Set `with_album` to false to skip album fallback.
"""
if key in dir(self) and isinstance(
getattr(self.__class__, key), property
):
return getattr(self, key)
try:
return self._get(key, default, raise_=with_album)
except KeyError:
Expand Down Expand Up @@ -1181,7 +1204,7 @@ class Album(LibModel):
"albumartists_sort": types.MULTI_VALUE_DSV,
"albumartists_credit": types.MULTI_VALUE_DSV,
"album": types.STRING,
"genre": types.STRING,
"genres": types.SEMICOLON_SPACE_DSV,
"style": types.STRING,
"discogs_albumid": types.INTEGER,
"discogs_artistid": types.INTEGER,
Expand Down Expand Up @@ -1215,7 +1238,7 @@ class Album(LibModel):
"original_day": types.PaddedInt(2),
}

_search_fields = ("album", "albumartist", "genre")
_search_fields = ("album", "albumartist", "genres")

_types = {
"path": PathType(),
Expand All @@ -1237,7 +1260,7 @@ class Album(LibModel):
"albumartist_credit",
"albumartists_credit",
"album",
"genre",
"genres",
"style",
"discogs_albumid",
"discogs_artistid",
Expand Down Expand Up @@ -1293,10 +1316,12 @@ def relation_join(cls) -> str:
def _getters(cls):
# In addition to plugin-provided computed fields, also expose
# the album's directory as `path`.
getters = plugins.album_field_getters()
getters["path"] = Album.item_dir
getters["albumtotal"] = Album._albumtotal
return getters
return {
**super()._getters(),
**plugins.album_field_getters(),
"path": Album.item_dir,
"albumtotal": Album._albumtotal,
}

def items(self):
"""Return an iterable over the items associated with this
Expand Down
2 changes: 2 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ New features:
* Beets now uses ``platformdirs`` to determine the default music directory.
This location varies between systems -- for example, users can configure it
on Unix systems via ``user-dirs.dirs(5)``.
* New multi-valued ``genres`` tag. This change brings up the ``genres`` tag to the same state as the ``*artists*`` multi-valued tags (see :bug:`4743` for details).
:bug:`5426`

Bug fixes:

Expand Down
18 changes: 11 additions & 7 deletions test/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,19 @@ def test_store_changes_database_value(self):
assert new_year == 1987

def test_store_only_writes_dirty_fields(self):
original_genre = self.i.genre
self.i._values_fixed["genre"] = "beatboxing" # change w/o dirtying
original_artist = self.i.artist
self.i._values_fixed["artist"] = "beatboxing" # change w/o dirtying
self.i.store()
new_genre = (
self.lib._connection()
.execute("select genre from items where title = ?", (self.i.title,))
.fetchone()["genre"]
assert (
(
self.lib._connection()
.execute(
"select artist from items where title = ?", (self.i.title,)
)
.fetchone()["artist"]
)
== original_artist
)
assert new_genre == original_genre

def test_store_clears_dirty_flags(self):
self.i.composer = "tvp"
Expand Down
20 changes: 10 additions & 10 deletions test/test_sort.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,23 @@ def setUp(self):
albums = [
Album(
album="Album A",
genre="Rock",
label="Label",
year=2001,
flex1="Flex1-1",
flex2="Flex2-A",
albumartist="Foo",
),
Album(
album="Album B",
genre="Rock",
label="Label",
year=2001,
flex1="Flex1-2",
flex2="Flex2-A",
albumartist="Bar",
),
Album(
album="Album C",
genre="Jazz",
label="Records",
year=2005,
flex1="Flex1-1",
flex2="Flex2-B",
Expand Down Expand Up @@ -236,19 +236,19 @@ def test_sort_desc(self):

def test_sort_two_field_asc(self):
q = ""
s1 = dbcore.query.FixedFieldSort("genre", True)
s1 = dbcore.query.FixedFieldSort("label", True)
s2 = dbcore.query.FixedFieldSort("album", True)
sort = dbcore.query.MultipleSort()
sort.add_sort(s1)
sort.add_sort(s2)
results = self.lib.albums(q, sort)
assert results[0]["genre"] <= results[1]["genre"]
assert results[1]["genre"] <= results[2]["genre"]
assert results[1]["genre"] == "Rock"
assert results[2]["genre"] == "Rock"
assert results[0]["label"] <= results[1]["label"]
assert results[1]["label"] <= results[2]["label"]
assert results[1]["label"] == "Label"
assert results[2]["label"] == "Records"
assert results[1]["album"] <= results[2]["album"]
# same thing with query string
q = "genre+ album+"
q = "label+ album+"
results2 = self.lib.albums(q)
for r1, r2 in zip(results, results2):
assert r1.id == r2.id
Expand Down Expand Up @@ -388,7 +388,7 @@ def setUp(self):

album = Album(
album="album",
genre="alternative",
label="label",
year="2001",
flex1="flex1",
flex2="flex2-A",
Expand Down
4 changes: 2 additions & 2 deletions test/test_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ def test_selective_modified_album_metadata_not_moved(self):
mf.album = "differentAlbum"
mf.genre = "differentGenre"
mf.save()
self._update(move=True, fields=["genre"])
self._update(move=True, fields=["genres"])
item = self.lib.items().get()
assert b"differentAlbum" not in item.path
assert item.genre == "differentGenre"
Expand Down Expand Up @@ -1445,7 +1445,7 @@ def test_completion(self):
assert tester.returncode == 0
assert out == b"completion tests passed\n", (
"test/test_completion.sh did not execute properly. "
f'Output:{out.decode("utf-8")}'
f"Output:{out.decode('utf-8')}"
)


Expand Down
Loading