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.

Sign up for GitHub

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

Jump to bottom

Implement multi-value genres tag #5426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement multi-value genres tag #5426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions beets/autotag/hooks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ def __init__(
country: Optional[str] = None,
style: Optional[str] = None,
genre: Optional[str] = None,
genres: Optional[str] = None,
albumstatus: Optional[str] = None,
media: Optional[str] = None,
albumdisambig: Optional[str] = None,
Expand Down Expand Up @@ -152,6 +153,7 @@ def __init__(
self.country = country
self.style = style
self.genre = genre
self.genres = genres
self.albumstatus = albumstatus
self.media = media
self.albumdisambig = albumdisambig
Expand Down Expand Up @@ -221,6 +223,7 @@ def __init__(
bpm: Optional[str] = None,
initial_key: Optional[str] = None,
genre: Optional[str] = None,
genres: Optional[str] = None,
album: Optional[str] = None,
**kwargs,
):
Expand Down Expand Up @@ -255,6 +258,7 @@ def __init__(
self.bpm = bpm
self.initial_key = initial_key
self.genre = genre
self.genres = genres
self.album = album
self.update(kwargs)

Expand Down
3 changes: 3 additions & 0 deletions beets/library.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ class Item(LibModel):
"albumartist_credit": types.STRING,
"albumartists_credit": types.MULTI_VALUE_DSV,
"genre": types.STRING,
"genres": types.MULTI_VALUE_DSV,
"style": types.STRING,
"discogs_albumid": types.INTEGER,
"discogs_artistid": types.INTEGER,
Expand Down Expand Up @@ -1173,6 +1174,7 @@ class Album(LibModel):
"albumartists_credit": types.MULTI_VALUE_DSV,
"album": types.STRING,
"genre": types.STRING,
"genres": types.MULTI_VALUE_DSV,
"style": types.STRING,
"discogs_albumid": types.INTEGER,
"discogs_artistid": types.INTEGER,
Expand Down Expand Up @@ -1229,6 +1231,7 @@ class Album(LibModel):
"albumartists_credit",
"album",
"genre",
"genres",
"style",
"discogs_albumid",
"discogs_artistid",
Expand Down
1 change: 1 addition & 0 deletions docs/changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ New features:
* :doc:`plugins/autobpm`: Add new configuration option ``beat_track_kwargs``
which enables adjusting keyword arguments supplied to librosa's
``beat_track`` function call.
* New multi-value `genres` tag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you expand a bit here, maybe with an example that shows why it's useful? The point of such a tag should be made obvious to end users scrolling through.


Bug fixes:

Expand Down
30 changes: 26 additions & 4 deletions test/test_library.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,13 +708,13 @@ def test_if_def_false_complete(self):
self._assert_dest(b"/base/not_played")

def test_first(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try and test this %first{} function with the new genres field, to show how the user may use it to extract the first genre?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the only way to use %first with these MULTI_VALUE_DSV fields is this (for the record, that doesn't even work for me and I have to do %first{%first{$albumartists,1,0,␀},1,0,\\} in my own config) which I think is pretty awful, not documented anywhere and not even confirmed as the intended method.

Without it being confirmed that that is the intended method of use, or a future improvement to the ergonomics, I wouldn't want to encode this in a test as the expected usage. This PR just brings up the genres field to the same state as the artists field, and any work beyond that is out of scope.

self.i.genres = "Pop; Rock; Classical Crossover"
self._setf("%first{$genres}")
self.i.semicolon_delimited_field = "Pop; Rock; Classical Crossover"
self._setf("%first{$semicolon_delimited_field}")
self._assert_dest(b"/base/Pop")

def test_first_skip(self):
self.i.genres = "Pop; Rock; Classical Crossover"
self._setf("%first{$genres,1,2}")
self.i.semicolon_delimited_field = "Pop; Rock; Classical Crossover"
self._setf("%first{$semicolon_delimited_field,1,2}")
self._assert_dest(b"/base/Classical Crossover")

def test_first_different_sep(self):
Expand Down Expand Up @@ -1306,6 +1306,28 @@ def test_write_date_field(self):
item.write()
assert MediaFile(syspath(item.path)).year == clean_year

def test_write_multi_genres(self):
item = self.add_item_fixture(genre="old genre")
item.write(
tags={"genres": ["g1", "g2"]},
)

# Ensure it reads all genres
assert MediaFile(syspath(item.path)).genres == ["g1", "g2"]

# Ensure reading single genre outputs the first of the genres
assert MediaFile(syspath(item.path)).genre == "g1"

def test_write_multi_genres_both_single_and_multi(self):
item = self.add_item_fixture(genre="old genre 1")
item.write(
tags={"genre": "single genre", "genres": ["multi genre"]},
)

# Ensure the multi takes precedence
assert MediaFile(syspath(item.path)).genre == "multi genre"
assert MediaFile(syspath(item.path)).genres == ["multi genre"]


class ItemReadTest(unittest.TestCase):
def test_unreadable_raise_read_error(self):
Expand Down