From 56c1ff8819a6702237bdb4b6897abd21f1a194c3 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Wed, 3 May 2023 17:02:38 +0200 Subject: [PATCH 01/10] Propagate album flex attr mods to items It seems that _deleting_ flex attrs from an album already propagate to items. Now also _modifications_ of album flex attrs propagate to items. --- beets/library.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beets/library.py b/beets/library.py index 46d2f416c..9f7f56883 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1385,6 +1385,8 @@ class Album(LibModel): track_updates[key] = self[key] elif key not in self: track_deletes.add(key) + else: # Must be a flex attr + track_updates[key] = self[key] with self._db.transaction(): super().store(fields) From 3587067c1fed01cc4de4549b33efa2c86ba8453d Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Fri, 16 Jun 2023 07:26:39 +0200 Subject: [PATCH 02/10] Option to override album mods propagation to items Adds a cli option to the modify command that prevents inheriting `modify -a` changes to album-tracks. --- beets/library.py | 12 ++++++------ beets/ui/commands.py | 11 ++++++++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/beets/library.py b/beets/library.py index 9f7f56883..baf7a312d 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1369,7 +1369,7 @@ class Album(LibModel): plugins.send('art_set', album=self) - def store(self, fields=None): + def store(self, fields=None, inherit=True): """Update the database with the album information. The album's tracks are also updated. @@ -1381,11 +1381,11 @@ class Album(LibModel): track_updates = {} track_deletes = set() for key in self._dirty: - if key in self.item_keys: + if key in self.item_keys and inherit: track_updates[key] = self[key] - elif key not in self: + elif key not in self and inherit: track_deletes.add(key) - else: # Must be a flex attr + elif inherit: # Must be a flex attr track_updates[key] = self[key] with self._db.transaction(): @@ -1402,7 +1402,7 @@ class Album(LibModel): del item[key] item.store() - def try_sync(self, write, move): + def try_sync(self, write, move, inherit=True): """Synchronize the album and its items with the database. Optionally, also write any new tags into the files and update their paths. @@ -1411,7 +1411,7 @@ class Album(LibModel): `move` controls whether files (both audio and album art) are moved. """ - self.store() + self.store(inherit=inherit) for item in self.items(): item.try_sync(write, move) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 750457234..8e6fda8e8 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1489,7 +1489,7 @@ default_commands.append(version_cmd) # modify: Declaratively change metadata. -def modify_items(lib, mods, dels, query, write, move, album, confirm): +def modify_items(lib, mods, dels, query, write, move, album, confirm, inherit): """Modifies matching items according to user-specified assignments and deletions. @@ -1542,7 +1542,7 @@ def modify_items(lib, mods, dels, query, write, move, album, confirm): # Apply changes to database and files with lib.transaction(): for obj in changed: - obj.try_sync(write, move) + obj.try_sync(write, move, inherit) def print_and_modify(obj, mods, dels): @@ -1585,7 +1585,8 @@ def modify_func(lib, opts, args): if not mods and not dels: raise ui.UserError('no modifications specified') modify_items(lib, mods, dels, query, ui.should_write(opts.write), - ui.should_move(opts.move), opts.album, not opts.yes) + ui.should_move(opts.move), opts.album, not opts.yes, + opts.inherit) modify_cmd = ui.Subcommand( @@ -1613,6 +1614,10 @@ modify_cmd.parser.add_option( '-y', '--yes', action='store_true', help='skip confirmation' ) +modify_cmd.parser.add_option( + '-I', '--noinherit', action='store_false', dest='inherit', default=True, + help="Don't inherit album-changes to tracks" +) modify_cmd.func = modify_func default_commands.append(modify_cmd) From 3c7f1223f686d8dc8e749d9b14b4f19c2fbaf25f Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Fri, 14 Jul 2023 18:02:30 +0200 Subject: [PATCH 03/10] Fix failing test_a_album*_edit_apply tests by excluding 'id' fields when storing within the Album model. --- beets/library.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/library.py b/beets/library.py index baf7a312d..3af15978a 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1385,7 +1385,7 @@ class Album(LibModel): track_updates[key] = self[key] elif key not in self and inherit: track_deletes.add(key) - elif inherit: # Must be a flex attr + elif key != 'id' and inherit: # Could be a flex attr or id track_updates[key] = self[key] with self._db.transaction(): From 94d00418b4d87a9765b4cb42a8847569b90a55c0 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Mon, 17 Jul 2023 18:48:19 +0200 Subject: [PATCH 04/10] Fix test_albuminfo_change_artist_does_not_change_items by adding (inherit=True) to fit with the new behaviour of the store() method and add a second test that checks the opposite. --- test/test_library.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/test_library.py b/test/test_library.py index 0e9637635..269771575 100644 --- a/test/test_library.py +++ b/test/test_library.py @@ -1022,10 +1022,17 @@ class AlbumInfoTest(_common.TestCase): self.assertEqual(i.albumartist, 'myNewArtist') self.assertNotEqual(i.artist, 'myNewArtist') + def test_albuminfo_change_artist_does_change_items(self): + ai = self.lib.get_album(self.i) + ai.artist = 'myNewArtist' + ai.store(inherit=True) + i = self.lib.items()[0] + self.assertEqual(i.artist, 'myNewArtist') + def test_albuminfo_change_artist_does_not_change_items(self): ai = self.lib.get_album(self.i) ai.artist = 'myNewArtist' - ai.store() + ai.store(inherit=False) i = self.lib.items()[0] self.assertNotEqual(i.artist, 'myNewArtist') From 8d835b8cabe45f71aa42e0ba564750734dd29347 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Wed, 2 Aug 2023 19:56:07 +0200 Subject: [PATCH 05/10] Fix ipfs plugin and test_ipfs by using store(inherit=False) for the creation of a new "ipfs album" as well as when test_ipfs creates album+items to compare with. Or put differently: Make ipfs and test_ipfs keep the old store() behaviour for which the plugin initially was built for. --- beetsplug/ipfs.py | 2 +- test/test_ipfs.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/ipfs.py b/beetsplug/ipfs.py index 5794143bd..11f131418 100644 --- a/beetsplug/ipfs.py +++ b/beetsplug/ipfs.py @@ -296,4 +296,4 @@ class IPFSPlugin(BeetsPlugin): self._log.info("Adding '{0}' to temporary library", album) new_album = tmplib.add_album(items) new_album.ipfs = album.ipfs - new_album.store() + new_album.store(inherit=False) diff --git a/test/test_ipfs.py b/test/test_ipfs.py index 8f72f5132..593a01b8f 100644 --- a/test/test_ipfs.py +++ b/test/test_ipfs.py @@ -87,7 +87,7 @@ class IPFSPluginTest(unittest.TestCase, TestHelper): album = self.lib.add_album(items) album.ipfs = "QmfM9ic5LJj7V6ecozFx1MkSoaaiq3PXfhJoFvyqzpLXSf" - album.store() + album.store(inherit=False) return album From 7b49b8680c58346db1ff11acb843d03b65fd876e Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Wed, 23 Aug 2023 06:36:29 +0200 Subject: [PATCH 06/10] Add changelog for #4823 --- docs/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ee8f4c2f6..b539c6d99 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -211,6 +211,11 @@ Bug fixes: the highest number of likes * :doc:`/plugins/lyrics`: Fix a crash with the Google backend when processing some web pages. :bug:`4875` +* Modifying flexible attributes of albums now cascade to the individual album + tracks, similar to how fixed album attributes have been cascading to tracks + already. A new option ``--noinherit/-I`` to :ref:`modify ` + allows changing this behaviour. + :bug:`4822` For packagers: From 31cd0ecb4ea2353ab8aff3f685c36d84daf77b25 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Thu, 3 Aug 2023 09:41:12 +0200 Subject: [PATCH 07/10] Add docs for `mod -a --noinherit` option and further clarify `mod -a` docs: Even though e39dcfc002557b87cc0e54a56b3022e20ded8828 and the linked discussion already does a very good job on clarifying what is actually happening when `mod -a` is issued, this commit adds further details about the difference between the album query and what is actually modified. --- docs/reference/cli.rst | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index da119d0f8..c72a76eab 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -257,7 +257,7 @@ modify `````` :: - beet modify [-MWay] [-f FORMAT] QUERY [FIELD=VALUE...] [FIELD!...] + beet modify [-IMWay] [-f FORMAT] QUERY [FIELD=VALUE...] [FIELD!...] Change the metadata for items or albums in the database. @@ -274,13 +274,17 @@ name into the artist field for all your tracks, and ``beet modify title='$track $title'`` will add track numbers to their title metadata. -The ``-a`` switch also operates on albums in addition to the individual tracks. +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 tracks belong to the same album. If you want to change an *album-level* field, such as ``year`` or ``albumartist``, you'll want to use the ``-a`` flag to avoid a confusing situation where the data for individual tracks conflicts with the data for the whole album. +Modifications issued using ``-a`` by default cascade to individual tracks. To +prevent this behaviour, ``-I/--noinherit`` can be used. + Items will automatically be moved around when necessary if they're in your library directory, but you can disable that with ``-M``. Tags will be written to the files according to the settings you have for imports, but these can be From ad3199941decbc0fbf0b79412d1ea110215e648d Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Sun, 6 Aug 2023 09:58:32 +0200 Subject: [PATCH 08/10] Clarify Album.store() docstring and comments, explaining the inherit flag, fixed/flex attrs and the strict exclusion of the id field. --- beets/library.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/beets/library.py b/beets/library.py index 3af15978a..71ce251cd 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1372,20 +1372,22 @@ class Album(LibModel): def store(self, fields=None, inherit=True): """Update the database with the album information. - The album's tracks are also updated. - `fields` represents the fields to be stored. If not specified, all fields will be. + + The album's tracks are also updated when the `inherit` flag is enabled. + This applies to fixed attributes as well as flexible ones. The `id` + attribute of the album will never be inherited. """ # Get modified track fields. track_updates = {} track_deletes = set() for key in self._dirty: - if key in self.item_keys and inherit: + if key in self.item_keys and inherit: # Fixed attr track_updates[key] = self[key] - elif key not in self and inherit: + elif key not in self and inherit: # Fixed or flex attr track_deletes.add(key) - elif key != 'id' and inherit: # Could be a flex attr or id + elif key != 'id' and inherit: # Could be a flex attr or id (fixed) track_updates[key] = self[key] with self._db.transaction(): From 9616afd3398b23fe6413430ace72cf94664e0e2f Mon Sep 17 00:00:00 2001 From: J0J0 Todos <2733783+JOJ0@users.noreply.github.com> Date: Tue, 22 Aug 2023 18:16:52 +0200 Subject: [PATCH 09/10] Fixes in docs for #4823 as suggested by @sampsyo. Co-authored-by: Adrian Sampson --- beets/ui/commands.py | 2 +- docs/reference/cli.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 8e6fda8e8..636d9b8f4 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1616,7 +1616,7 @@ modify_cmd.parser.add_option( ) modify_cmd.parser.add_option( '-I', '--noinherit', action='store_false', dest='inherit', default=True, - help="Don't inherit album-changes to tracks" + help="when modifying albums, don't also change item data" ) modify_cmd.func = modify_func default_commands.append(modify_cmd) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index c72a76eab..9306397a9 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -283,7 +283,7 @@ to avoid a confusing situation where the data for individual tracks conflicts with the data for the whole album. Modifications issued using ``-a`` by default cascade to individual tracks. To -prevent this behaviour, ``-I/--noinherit`` can be used. +prevent this behavior, use ``-I``/``--noinherit``. Items will automatically be moved around when necessary if they're in your library directory, but you can disable that with ``-M``. Tags will be written From d7b7d60111c8f8d28737306af1007bef2da92316 Mon Sep 17 00:00:00 2001 From: J0J0 Todos Date: Wed, 23 Aug 2023 06:32:17 +0200 Subject: [PATCH 10/10] Refactor Album.store for code readability --- beets/library.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/beets/library.py b/beets/library.py index 71ce251cd..405b546e2 100644 --- a/beets/library.py +++ b/beets/library.py @@ -1383,12 +1383,13 @@ class Album(LibModel): track_updates = {} track_deletes = set() for key in self._dirty: - if key in self.item_keys and inherit: # Fixed attr - track_updates[key] = self[key] - elif key not in self and inherit: # Fixed or flex attr - track_deletes.add(key) - elif key != 'id' and inherit: # Could be a flex attr or id (fixed) - track_updates[key] = self[key] + if inherit: + if key in self.item_keys: # is a fixed attribute + track_updates[key] = self[key] + elif key not in self: # is a fixed or a flexible attribute + track_deletes.add(key) + elif key != 'id': # is a flexible attribute + track_updates[key] = self[key] with self._db.transaction(): super().store(fields)