diff --git a/picard/album.py b/picard/album.py index 54dfbd4633..75023bdfaa 100644 --- a/picard/album.py +++ b/picard/album.py @@ -858,10 +858,6 @@ def children_metadata_items(self): yield from track.files yield from self.unmatched_files.files - def delete_genres_from_tags(self): - for genre in self.genres: - del self.folksonomy_tags[genre] - class NatAlbum(Album): diff --git a/picard/track.py b/picard/track.py index ae3f30bbaf..09c142725c 100644 --- a/picard/track.py +++ b/picard/track.py @@ -46,6 +46,7 @@ Counter, defaultdict, ) +from operator import attrgetter import re import traceback @@ -320,9 +321,8 @@ def _customize_metadata(self): if config.setting['use_genres']: self._convert_folksonomy_tags_to_genre() - self.album.delete_genres_from_tags() - self._add_folksonomy_tags() - self._add_genres() + self._add_folksonomy_tags_variable() + self._add_genres_variable() # Convert Unicode punctuation if config.setting['convert_punctuation']: @@ -355,24 +355,10 @@ def _genres_to_metadata(genres, limit=None, minusage=0, filters='', join_with=No def _convert_folksonomy_tags_to_genre(self): config = get_config() - genres = Counter(self.genres) - use_folksonomy = config.setting['folksonomy_tags'] - if use_folksonomy: - genres += self.album.folksonomy_tags + if config.setting['folksonomy_tags']: + genres = self._merged_folksonomy_tags else: - genres += self.album.genres - # Combine release and track genres - if self.album.release_group: - genres += self.album.release_group.genres - if not genres and config.setting['artists_genres']: - # For compilations use each track's artists to look up genres - if self.metadata['musicbrainz_albumartistid'] == VARIOUS_ARTISTS_ID: - for artist in self._track_artists: - genres += artist.genres - else: - for artist in self.album.get_album_artists(): - genres += artist.genres - + genres = self._merged_genres self.metadata['genre'] = self._genres_to_metadata( genres, limit=config.setting['max_genres'], @@ -381,18 +367,42 @@ def _convert_folksonomy_tags_to_genre(self): join_with=config.setting['join_genres'] ) - def _add_tags(self, tags, name): - self.metadata[name] = tags.keys() - - def _add_folksonomy_tags(self): - tags = Counter(self.folksonomy_tags) - tags += self.album.folksonomy_tags - self._add_tags(tags, '_folksonomy_tags') + @property + def _merged_folksonomy_tags(self): + return self._merge_folksonomy_tags('folksonomy_tags') - def _add_genres(self): - genre = Counter(self.genres) - genre += self.album.genres - self._add_tags(genre, '_genres') + @property + def _merged_genres(self): + return self._merge_folksonomy_tags('genres') + + def _merge_folksonomy_tags(self, name): + """Merge folksonomy_tags or genres from track, album, release group and artists. + """ + getter = attrgetter(name) + genres = getter(self) + if self.album: + genres += getter(self.album) + if self.album.release_group: + genres += getter(self.album.release_group) + if not genres: + config = get_config() + if config.setting['artists_genres']: + # For compilations use each track's artists to look up genres + if self.metadata['musicbrainz_albumartistid'] == VARIOUS_ARTISTS_ID: + for artist in self._track_artists: + genres += getter(artist) + elif self.album: + for artist in self.album.get_album_artists(): + genres += getter(artist) + return genres + + def _add_folksonomy_tags_variable(self): + tags = self._merged_folksonomy_tags - self._merged_genres + self.metadata['~folksonomy_tags'] = sorted(tags) + + def _add_genres_variable(self): + genres = self._merged_genres + self.metadata['~genres'] = sorted(genres) class NonAlbumTrack(Track): diff --git a/test/test_item.py b/test/test_item.py index 7784adaa76..15061bd434 100644 --- a/test/test_item.py +++ b/test/test_item.py @@ -2,7 +2,7 @@ # # Picard, the next-generation MusicBrainz tagger # -# Copyright (C) 2018, 2021, 2024 Philipp Wolfer +# Copyright (C) 2018, 2021, 2024-2025 Philipp Wolfer # Copyright (C) 2020-2021 Laurent Monin # # This program is free software; you can redistribute it and/or @@ -20,6 +20,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +from collections import Counter from unittest.mock import Mock from test.picardtestcase import PicardTestCase @@ -78,10 +79,11 @@ def test_set_genre_inc_params_with_user_tags(self): self.assertTrue(require_auth) def test_add_genres(self): - self.obj.add_genre('genre1', 2) - self.assertEqual(self.obj.genres['genre1'], 2) - self.obj.add_genre('genre1', 5) - self.assertEqual(self.obj.genres['genre1'], 7) + self.obj.add_genre('pop', 1) + self.obj.add_genre('rock', 1) + self.obj.add_genre('blues', 2) + self.obj.add_genre('pop', 2) + self.assertEqual(self.obj._genres, Counter(pop=3, rock=1, blues=2)) def test_set_genre_inc_custom_config(self): inc = set() diff --git a/test/test_track.py b/test/test_track.py index 71731708e0..9692f2436f 100644 --- a/test/test_track.py +++ b/test/test_track.py @@ -3,7 +3,7 @@ # Picard, the next-generation MusicBrainz tagger # # Copyright (C) 2021-2022 Laurent Monin -# Copyright (C) 2021-2022, 2024 Philipp Wolfer +# Copyright (C) 2021-2022, 2024-2025 Philipp Wolfer # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -24,7 +24,16 @@ from test.picardtestcase import PicardTestCase -from picard.track import Track +from picard.album import ( + Album, + AlbumArtist, +) +from picard.const import VARIOUS_ARTISTS_ID +from picard.releasegroup import ReleaseGroup +from picard.track import ( + Track, + TrackArtist, +) class TrackTest(PicardTestCase): @@ -32,8 +41,70 @@ def test_can_link_fingerprint(self): track = Track('123') self.assertTrue(track.can_link_fingerprint) + def test_merge_folksonomy_tags_no_album(self): + track = Track('123') + track._genres = Counter(pop=6, rock=7, blues=2) + self.assertEqual(track._merge_folksonomy_tags('genres'), track._genres) + + def test_merge_folksonomy_tags_with_album(self): + track = Track('123') + track._genres = Counter(pop=6, rock=7) + album = Album('456') + album._genres = Counter(rock=3, blues=2) + release_group = ReleaseGroup('789') + release_group._genres = Counter(blues=1) + album.release_group = release_group + track.album = album + expected = Counter(pop=6, rock=10, blues=3) + self.assertEqual(track._merge_folksonomy_tags('genres'), expected) + + def test_merge_folksonomy_tags_artist_genre_fallback(self): + self.set_config_values({'artists_genres': True}) + track = Track('123') + album = Album('456') + artist1 = AlbumArtist('1') + artist1._genres = Counter(rock=1, blues=2) + album._album_artists.append(artist1) + artist2 = AlbumArtist('2') + artist2._genres = Counter(pop=2, rock=1) + album._album_artists.append(artist2) + track.album = album + expected = artist1._genres + artist2._genres + self.assertEqual(track._merge_folksonomy_tags('genres'), expected) + + def test_merge_folksonomy_tags_various_artist_genre_fallback(self): + self.set_config_values({'artists_genres': True}) + track = Track('123') + track.metadata['musicbrainz_albumartistid'] = VARIOUS_ARTISTS_ID + album = Album('456') + album_artist = AlbumArtist('1') + album_artist._genres = Counter(country=1) + album._album_artists.append(album_artist) + track.album = album + track_artist1 = TrackArtist('2') + track_artist1._genres = Counter(rock=1, blues=2) + track._track_artists.append(track_artist1) + track_artist2 = TrackArtist('3') + track_artist2._genres = Counter(pop=2, rock=1) + track._track_artists.append(track_artist2) + expected = track_artist1._genres + track_artist2._genres + self.assertEqual(track._merge_folksonomy_tags('genres'), expected) + + def test_add_genres_variable(self): + track = Track('123') + track._genres = Counter(pop=6, rock=7, blues=2) + track._add_genres_variable() + self.assertEqual(track.metadata.getall('~genres'), ['blues', 'pop', 'rock']) + + def test_add_folksonomy_tags_variable(self): + track = Track('123') + track._genres = Counter(pop=6, rock=7, blues=2) + track._folksonomy_tags = Counter(live=2, pop=6, rock=7, blues=2, favorite=1) + track._add_folksonomy_tags_variable() + self.assertEqual(track.metadata.getall('~folksonomy_tags'), ['favorite', 'live']) + -class TrackGenres2MetadataTest(PicardTestCase): +class TrackGenresToMetadataTest(PicardTestCase): def test_empty(self): genres = Counter() ret = Track._genres_to_metadata(genres) @@ -79,10 +150,10 @@ def test_minusage(self): def test_filters(self): genres = Counter(pop=6, rock=7, blues=2) - ret = Track._genres_to_metadata(genres, filters="-blues") + ret = Track._genres_to_metadata(genres, filters='-blues') self.assertEqual(ret, ['Pop', 'Rock']) def test_join_with(self): genres = Counter(pop=6, rock=7, blues=2) - ret = Track._genres_to_metadata(genres, join_with=",") + ret = Track._genres_to_metadata(genres, join_with=',') self.assertEqual(ret, ['Blues,Pop,Rock'])