-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add native support for multiple genres per album/track #6169
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,7 +234,12 @@ def __init__(self, data: JSONDict): | |
| if "artists" in data: | ||
| self.artists = [(x["id"], str(x["name"])) for x in data["artists"]] | ||
| if "genres" in data: | ||
| self.genres = [str(x["name"]) for x in data["genres"]] | ||
| genre_list = [str(x["name"]) for x in data["genres"]] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given my comment above, I expect |
||
| if beets.config["multi_value_genres"]: | ||
| self.genres = genre_list | ||
| else: | ||
| # Even when disabled, populate with first genre for consistency | ||
| self.genres = [genre_list[0]] if genre_list else [] | ||
sourcery-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| def artists_str(self) -> str | None: | ||
| if self.artists is not None: | ||
|
|
@@ -306,11 +311,22 @@ def __init__(self, data: JSONDict): | |
| self.bpm = data.get("bpm") | ||
| self.initial_key = str((data.get("key") or {}).get("shortName")) | ||
|
|
||
| # Use 'subgenre' and if not present, 'genre' as a fallback. | ||
| # Extract genres list from subGenres or genres | ||
| if data.get("subGenres"): | ||
| self.genre = str(data["subGenres"][0].get("name")) | ||
| genre_list = [str(x.get("name")) for x in data["subGenres"]] | ||
| elif data.get("genres"): | ||
| self.genre = str(data["genres"][0].get("name")) | ||
| genre_list = [str(x.get("name")) for x in data["genres"]] | ||
| else: | ||
| genre_list = [] | ||
|
|
||
| if beets.config["multi_value_genres"]: | ||
| # New behavior: populate both genres list and joined string | ||
| separator = beets.config["genre_separator"].get(str) | ||
| self.genres = genre_list | ||
| self.genre = separator.join(genre_list) if genre_list else None | ||
| else: | ||
| # Old behavior: only populate single genre field with first value | ||
| self.genre = genre_list[0] if genre_list else None | ||
|
|
||
|
|
||
| class BeatportPlugin(MetadataSourcePlugin): | ||
|
|
@@ -484,6 +500,7 @@ def _get_album_info(self, release: BeatportRelease) -> AlbumInfo: | |
| data_source=self.data_source, | ||
| data_url=release.url, | ||
| genre=release.genre, | ||
| genres=release.genres, | ||
| year=release_date.year if release_date else None, | ||
| month=release_date.month if release_date else None, | ||
| day=release_date.day if release_date else None, | ||
|
|
@@ -509,6 +526,7 @@ def _get_track_info(self, track: BeatportTrack) -> TrackInfo: | |
| bpm=track.bpm, | ||
| initial_key=track.initial_key, | ||
| genre=track.genre, | ||
| genres=track.genres, | ||
| ) | ||
|
|
||
| def _get_artist(self, artists): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -329,12 +329,35 @@ def _format_and_stringify(self, tags: list[str]) -> str: | |
| else: | ||
| formatted = tags | ||
|
|
||
| return self.config["separator"].as_str().join(formatted) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The adjustments in
|
||
| # Use global genre_separator when multi_value_genres is enabled | ||
| # for consistency with sync logic, otherwise use plugin's own separator | ||
| if config["multi_value_genres"]: | ||
| separator = config["genre_separator"].get(str) | ||
| else: | ||
| separator = self.config["separator"].as_str() | ||
|
|
||
| return separator.join(formatted) | ||
|
|
||
| def _get_existing_genres(self, obj: LibModel) -> list[str]: | ||
| """Return a list of genres for this Item or Album. Empty string genres | ||
| are removed.""" | ||
| separator = self.config["separator"].get() | ||
| # Prefer the genres field if it exists (multi-value support) | ||
| if isinstance(obj, library.Item): | ||
| genres_list = obj.get("genres", with_album=False) | ||
| else: | ||
| genres_list = obj.get("genres") | ||
|
|
||
| # If genres field exists and is not empty, use it | ||
| if genres_list: | ||
| return [g for g in genres_list if g] | ||
|
|
||
| # Otherwise fall back to splitting the genre field | ||
| # Use global genre_separator when multi_value_genres is enabled | ||
| if config["multi_value_genres"]: | ||
| separator = config["genre_separator"].get(str) | ||
| else: | ||
| separator = self.config["separator"].get() | ||
|
|
||
| if isinstance(obj, library.Item): | ||
| item_genre = obj.get("genre", with_album=False).split(separator) | ||
| else: | ||
|
|
@@ -473,6 +496,17 @@ def _fetch_and_log_genre(self, obj: LibModel) -> None: | |
| obj.genre, label = self._get_genre(obj) | ||
| self._log.debug("Resolved ({}): {}", label, obj.genre) | ||
|
|
||
| # Also populate the genres list field if multi_value_genres is enabled | ||
| if config["multi_value_genres"]: | ||
| if obj.genre: | ||
| # Use global genre_separator for consistency with sync logic | ||
| separator = config["genre_separator"].get(str) | ||
| obj.genres = [ | ||
| g.strip() for g in obj.genre.split(separator) if g.strip() | ||
| ] | ||
| else: | ||
| obj.genres = [] | ||
|
|
||
| ui.show_model_changes(obj, fields=["genre"], print_obj=False) | ||
|
|
||
| @singledispatchmethod | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -736,10 +736,19 @@ def album_info(self, release: JSONDict) -> beets.autotag.hooks.AlbumInfo: | |
| for source in sources: | ||
| for genreitem in source: | ||
| genres[genreitem["name"]] += int(genreitem["count"]) | ||
| info.genre = "; ".join( | ||
| genre_list = [ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be just info.genres = [
genre
for genre, _count in sorted(genres.items(), key=lambda g: -g[1])
] |
||
| genre | ||
| for genre, _count in sorted(genres.items(), key=lambda g: -g[1]) | ||
| ) | ||
| ] | ||
|
|
||
| if config["multi_value_genres"]: | ||
| # New behavior: populate genres list and joined genre string | ||
| separator = config["genre_separator"].get(str) | ||
| info.genres = genre_list | ||
| info.genre = separator.join(genre_list) if genre_list else None | ||
| else: | ||
| # Old behavior: only populate single genre field with first value | ||
| info.genre = genre_list[0] if genre_list else None | ||
|
|
||
| # We might find links to external sources (Discogs, Bandcamp, ...) | ||
| external_ids = self.config["external_ids"].get() | ||
|
|
||
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.
This should be simplified to:
I think we should migrate to multi-value genres for good and not provide users with an option to not to. Ultimately, this is just about how we store them in our database, so it's best to have a single way of doing it. Thus, there's no need for
multi_value_genresorgenre_separatoroptions.