From 0ed98515adc8b83add36e1d847b5848831cb68a4 Mon Sep 17 00:00:00 2001 From: kooimens Date: Fri, 30 Oct 2015 17:04:11 +0100 Subject: [PATCH 01/26] Embedart: remove_art_file on import Should fix #1662. I think the fix is easy. Don't know if it's clean though. Did some tests (+/- 10 albums), all successful. This is the first time I'm using python so please let me know what I should improve:) --- beetsplug/embedart.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 10b30af0f..9fb575429 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -141,3 +141,10 @@ class EmbedCoverArtPlugin(BeetsPlugin): art.embed_album(self._log, album, max_width, True, self.config['compare_threshold'].get(int), self.config['ifempty'].get(bool)) + if self.config['remove_art_file'] and album.artpath is not None: + if os.path.isfile(album.artpath): + self._log.debug(u'Removing album art file ' + u'for {0}', album) + os.remove(album.artpath) + album.artpath = None + album.store() From 0257b1077f682715f2b6785e9eebdfa0e1d1e312 Mon Sep 17 00:00:00 2001 From: kooimens Date: Fri, 30 Oct 2015 17:49:57 +0100 Subject: [PATCH 02/26] Simplify some code --- beetsplug/embedart.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 9fb575429..9afcf9cf4 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -80,7 +80,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): art.embed_album(self._log, album, maxwidth, False, compare_threshold, ifempty) - if remove_art_file and album.artpath is not None: + if remove_art_file and album.artpath: if os.path.isfile(album.artpath): self._log.debug(u'Removing album art file ' u'for {0}', album) @@ -141,7 +141,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): art.embed_album(self._log, album, max_width, True, self.config['compare_threshold'].get(int), self.config['ifempty'].get(bool)) - if self.config['remove_art_file'] and album.artpath is not None: + if self.config['remove_art_file'] and album.artpath: if os.path.isfile(album.artpath): self._log.debug(u'Removing album art file ' u'for {0}', album) From 629a80a1d12818cfa0de0b8b8b8a0e24d753215a Mon Sep 17 00:00:00 2001 From: reiv Date: Sun, 1 Nov 2015 01:43:48 +0100 Subject: [PATCH 03/26] Importer: Delete orphaned album art on re-imports First step towards fixing #314. --- beets/importer.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/beets/importer.py b/beets/importer.py index 95163b6fd..ba1c47c94 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -547,6 +547,23 @@ class ImportTask(BaseImportTask): items = self.imported_items() + # If any original album art files were reused, leave them untouched. + for item in items: + album = item.get_album() + if album: + self.old_art_paths.discard(album.artpath) + + # Delete remaining original album art files. + for old_art_path in self.old_art_paths: + log.debug('Deleting orphaned album art: {0}', old_art_path) + util.remove(syspath(old_art_path), True) + # XXX: Ideally the directory would get pruned with self.prune() + # below, but when calling `beet import -L`, self.toppath is set + # to None, making self.prune() a no-op; hence this workaround of + # calling util.prune_dirs() directly. + util.prune_dirs(os.path.dirname(old_art_path), + clutter=config['clutter'].as_str_seq()) + # When copying and deleting originals, delete old files. if copy and delete: new_paths = [os.path.realpath(item.path) for item in items] @@ -652,6 +669,11 @@ class ImportTask(BaseImportTask): # Save the original paths of all items for deletion and pruning # in the next step (finalization). self.old_paths = [item.path for item in items] + + # Keep track of paths of all original album art for the same reason. + self.old_art_paths = set(filter(bool, + (album.artpath for album in self.replaced_albums.values()))) + for item in items: if move or copy or link: # In copy and link modes, treat re-imports specially: From 413fe6bbfd695725c9ca7d306dfe408d20ad3904 Mon Sep 17 00:00:00 2001 From: reiv Date: Sun, 1 Nov 2015 04:33:36 +0100 Subject: [PATCH 04/26] PEP8 amendments --- beets/importer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index ba1c47c94..8aecf54a3 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -671,8 +671,8 @@ class ImportTask(BaseImportTask): self.old_paths = [item.path for item in items] # Keep track of paths of all original album art for the same reason. - self.old_art_paths = set(filter(bool, - (album.artpath for album in self.replaced_albums.values()))) + self.old_art_paths = set(filter( + bool, (album.artpath for album in self.replaced_albums.values()))) for item in items: if move or copy or link: From 3250fa80ae7b5f7af97309c9a876e94246ac59dc Mon Sep 17 00:00:00 2001 From: reiv Date: Sun, 1 Nov 2015 02:18:19 +0100 Subject: [PATCH 05/26] fetchart: Only resize when necessary Fixes #1264: When a re-import triggers fetchart, the existing album art is no longer resized if it already has the correct dimensions. This avoids creating new album art files with unique filenames ("cover.n.jpg"). --- beetsplug/fetchart.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index b807891e0..be7efe68a 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -41,6 +41,9 @@ IMAGE_EXTENSIONS = ['png', 'jpg', 'jpeg'] CONTENT_TYPES = ('image/jpeg', 'image/png') DOWNLOAD_EXTENSION = '.jpg' +CANDIDATE_BAD = 0 +CANDIDATE_EXACT = 1 +CANDIDATE_DOWNSCALE = 2 def _logged_get(log, *args, **kwargs): """Like `requests.get`, but logs the effective URL to the specified @@ -506,10 +509,10 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): def _is_valid_image_candidate(self, candidate): if not candidate: - return False + return CANDIDATE_BAD if not (self.enforce_ratio or self.minwidth): - return True + return CANDIDATE_EXACT # get_size returns None if no local imaging backend is available size = ArtResizer.shared.get_size(candidate) @@ -519,10 +522,14 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): u'documentation for dependencies. ' u'The configuration options `minwidth` and ' u'`enforce_ratio` may be violated.') - return True + return CANDIDATE_EXACT - return size and size[0] >= self.minwidth and \ - (not self.enforce_ratio or size[0] == size[1]) + if size[0] >= self.minwidth and ( + not self.enforce_ratio or size[0] == size[1]): + if size[0] > self.maxwidth: + return CANDIDATE_DOWNSCALE + return CANDIDATE_EXACT + return CANDIDATE_BAD def art_for_album(self, album, paths, local_only=False): """Given an Album object, returns a path to downloaded art for the @@ -532,6 +539,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): are made. """ out = None + check = None # Local art. cover_names = self.config['cover_names'].as_str_seq() @@ -540,7 +548,8 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): if paths: for path in paths: candidate = self.fs_source.get(path, cover_names, cautious) - if self._is_valid_image_candidate(candidate): + check = self._is_valid_image_candidate(candidate) + if check: out = candidate self._log.debug('found local image {}', out) break @@ -552,12 +561,13 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): if self.maxwidth: url = ArtResizer.shared.proxy_url(self.maxwidth, url) candidate = self._fetch_image(url) - if self._is_valid_image_candidate(candidate): + check = self._is_valid_image_candidate(candidate) + if check: out = candidate self._log.debug('using remote image {}', out) break - if self.maxwidth and out: + if check == CANDIDATE_DOWNSCALE: out = ArtResizer.shared.resize(self.maxwidth, out) return out From 9182fd8f04ddfc42efa84f91d93ef3546749346b Mon Sep 17 00:00:00 2001 From: reiv Date: Sun, 1 Nov 2015 04:33:36 +0100 Subject: [PATCH 06/26] PEP8 amendments --- beetsplug/fetchart.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index be7efe68a..5eae63278 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -45,6 +45,7 @@ CANDIDATE_BAD = 0 CANDIDATE_EXACT = 1 CANDIDATE_DOWNSCALE = 2 + def _logged_get(log, *args, **kwargs): """Like `requests.get`, but logs the effective URL to the specified `log` at the `DEBUG` level. @@ -525,7 +526,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): return CANDIDATE_EXACT if size[0] >= self.minwidth and ( - not self.enforce_ratio or size[0] == size[1]): + not self.enforce_ratio or size[0] == size[1]): if size[0] > self.maxwidth: return CANDIDATE_DOWNSCALE return CANDIDATE_EXACT @@ -567,7 +568,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): self._log.debug('using remote image {}', out) break - if check == CANDIDATE_DOWNSCALE: + if self.maxwidth and out and check == CANDIDATE_DOWNSCALE: out = ArtResizer.shared.resize(self.maxwidth, out) return out From db08896d8cff2a0333f7cc21395d5570b8532f26 Mon Sep 17 00:00:00 2001 From: reiv Date: Mon, 2 Nov 2015 03:02:11 +0100 Subject: [PATCH 07/26] fetchart: add docstring to helper function _is_valid_image_candidate() now has three different return values. These are documented here. --- beetsplug/fetchart.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 5eae63278..9da781e6c 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -509,6 +509,13 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): return None def _is_valid_image_candidate(self, candidate): + """Determine whether the given candidate artwork is valid based on + its dimensions (width and ratio). + + Return `CANDIDATE_BAD` if the file is unusable. + Return `CANDIDATE_EXACT` if the file is usable as-is. + Return `CANDIDATE_DOWNSCALE` if the file must be resized. + """ if not candidate: return CANDIDATE_BAD From 69b0ae974599a9e00339a8eeeccc1ccbb03feb4d Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Nov 2015 14:40:00 -0800 Subject: [PATCH 08/26] Add an FAQ about changing music directories --- docs/faq.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/faq.rst b/docs/faq.rst index 447be45e1..050461587 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -208,6 +208,23 @@ Use the ``%asciify{}`` function in your path formats. See :ref:`template-functions`. +.. _move-dir: + +…point beets at a new music directory? +-------------------------------------- + +If you want to move your music from one directory to another, the best way is +to let beets do it for you. First, edit your configuration and set the +``directory`` setting to the new place. Then, type ``beet move`` to have beets +move all your files. + +If you've already moved your music *outside* of beets, you have a few options: + +- Move the music back (with an ordinary ``mv``) and then use the above steps. +- Delete your database and re-create it from the new paths using ``beet import -AWMC``. +- Resort to manually modifying the SQLite database (not recommended). + + Why does beets… =============== From bcef3a71232ab66384f1ff532c334ef111eb9175 Mon Sep 17 00:00:00 2001 From: kooimens Date: Mon, 2 Nov 2015 23:56:38 +0100 Subject: [PATCH 09/26] Create function remove_artfile Damn it, that was really hard for me:D. First time seriously using python. Please review it carefuly. --- beetsplug/embedart.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 9afcf9cf4..df7f95d1d 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -79,14 +79,7 @@ class EmbedCoverArtPlugin(BeetsPlugin): for album in lib.albums(decargs(args)): art.embed_album(self._log, album, maxwidth, False, compare_threshold, ifempty) - - if remove_art_file and album.artpath: - if os.path.isfile(album.artpath): - self._log.debug(u'Removing album art file ' - u'for {0}', album) - os.remove(album.artpath) - album.artpath = None - album.store() + self.remove_artfile(album) embed_cmd.func = embed_func @@ -141,10 +134,12 @@ class EmbedCoverArtPlugin(BeetsPlugin): art.embed_album(self._log, album, max_width, True, self.config['compare_threshold'].get(int), self.config['ifempty'].get(bool)) - if self.config['remove_art_file'] and album.artpath: - if os.path.isfile(album.artpath): - self._log.debug(u'Removing album art file ' - u'for {0}', album) - os.remove(album.artpath) - album.artpath = None - album.store() + self.remove_artfile(album) + def remove_artfile(self, album) + if self.config['remove_art_file'] and album.artpath: + if os.path.isfile(album.artpath): + self._log.debug(u'Removing album art file ' + u'for {0}', album) + os.remove(album.artpath) + album.artpath = None + album.store() From e84414c822a073848152787868c41bd97f713551 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Nov 2015 16:44:02 -0800 Subject: [PATCH 10/26] Fix syntax (and unused variable) in #1675 --- beetsplug/embedart.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index df7f95d1d..01f11d3a3 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -63,7 +63,6 @@ class EmbedCoverArtPlugin(BeetsPlugin): maxwidth = self.config['maxwidth'].get(int) compare_threshold = self.config['compare_threshold'].get(int) ifempty = self.config['ifempty'].get(bool) - remove_art_file = self.config['remove_art_file'].get(bool) def embed_func(lib, opts, args): if opts.file: @@ -135,7 +134,8 @@ class EmbedCoverArtPlugin(BeetsPlugin): self.config['compare_threshold'].get(int), self.config['ifempty'].get(bool)) self.remove_artfile(album) - def remove_artfile(self, album) + + def remove_artfile(self, album): if self.config['remove_art_file'] and album.artpath: if os.path.isfile(album.artpath): self._log.debug(u'Removing album art file ' From d79d11fb15d95810824843cffca9072381d83eff Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Nov 2015 16:45:45 -0800 Subject: [PATCH 11/26] Docstring and rewrap for #1675 --- beetsplug/embedart.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 01f11d3a3..9c0efa51e 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -136,10 +136,12 @@ class EmbedCoverArtPlugin(BeetsPlugin): self.remove_artfile(album) def remove_artfile(self, album): + """Possibly delete the album art file for an album (if the + appropriate configuration option is enabled. + """ if self.config['remove_art_file'] and album.artpath: if os.path.isfile(album.artpath): - self._log.debug(u'Removing album art file ' - u'for {0}', album) + self._log.debug('Removing album art file for {0}', album) os.remove(album.artpath) album.artpath = None album.store() From bfa2fbf6afbcdb45689da8c664067a118bf1957b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 2 Nov 2015 16:46:32 -0800 Subject: [PATCH 12/26] Changelog for #1675 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 070dfeaf7..7e4564978 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -20,6 +20,8 @@ Fixes: non-existent album art file would prevent the command from fetching new art. :bug:`1126` * :doc:`/plugins/thumbnails`: Fix a crash with Unicode paths. :bug:`1686` +* :doc:`/plugins/embedart`: The ``remove_art_file`` option now works on import + (as well as with the explicit command). :bug:`1662` :bug:`1675` 1.3.15 (October 17, 2015) From 93009911e00b29ae378ac27785873e71cb00c2a5 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 02:17:21 +0100 Subject: [PATCH 13/26] fetchart: Add test for image resizing This test ensures that images are resized if and only if they are wider than the maxwidth setting. --- test/test_art.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/test_art.py b/test/test_art.py index 3cc24a7a4..fa8406c58 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -21,6 +21,7 @@ import os import shutil import responses +from mock import patch from test import _common from test._common import unittest @@ -409,6 +410,12 @@ class ArtForAlbumTest(UseThePlugin): else: self.assertIsNone(local_artpath) + def _assertImageResized(self, image_file, should_resize): + self.image_file = image_file + with patch.object(ArtResizer.shared, 'resize') as mock_resize: + self.plugin.art_for_album(None, [''], True) + self.assertEqual(mock_resize.called, should_resize) + def _require_backend(self): """Skip the test if the art resizer doesn't have ImageMagick or PIL (so comparisons and measurements are unavailable). @@ -432,6 +439,12 @@ class ArtForAlbumTest(UseThePlugin): self.plugin.enforce_ratio = False self._assertImageIsValidArt(self.IMG_500x490, True) + def test_resize_if_necessary(self): + self._require_backend() + self.plugin.maxwidth = 300 + self._assertImageResized(self.IMG_225x225, False) + self._assertImageResized(self.IMG_348x348, True) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 19dcc25a93689883d6b210e831af722eda16ff05 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 02:19:47 +0100 Subject: [PATCH 14/26] fetchart: Improve validation of image dimensions This avoids images being resized unnecessarily if the dimensions are correct. --- beetsplug/fetchart.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 9da781e6c..df0eb84ac 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -519,7 +519,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): if not candidate: return CANDIDATE_BAD - if not (self.enforce_ratio or self.minwidth): + if not (self.enforce_ratio or self.minwidth or self.maxwidth): return CANDIDATE_EXACT # get_size returns None if no local imaging backend is available @@ -532,9 +532,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): u'`enforce_ratio` may be violated.') return CANDIDATE_EXACT - if size[0] >= self.minwidth and ( + if (not self.minwidth or size[0] >= self.minwidth) and ( not self.enforce_ratio or size[0] == size[1]): - if size[0] > self.maxwidth: + if not self.maxwidth or size[0] > self.maxwidth: return CANDIDATE_DOWNSCALE return CANDIDATE_EXACT return CANDIDATE_BAD From 5d0048c03d800683379fbcb3840fb4554a66e068 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 02:38:02 +0100 Subject: [PATCH 15/26] Changelog for #1683 --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4eafe9c90..7ad738c84 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,8 @@ Fixes: when there were not. :bug:`1652` * :doc:`plugins/lastgenre`: Clean up the reggae related genres somewhat. Thanks to :user:`Freso`. :bug:`1661` +* :doc:`plugins/fetchart`: The plugin now only resizes album art if necessary, + rather than always by default. :bug:`1264` 1.3.15 (October 17, 2015) From 33ea9f1c101183ff1d8fba68950120b967cf14a8 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 11:25:04 +0100 Subject: [PATCH 16/26] Revert all changes on this branch thus far Revert "PEP8 amendments" This reverts commit 413fe6bbfd695725c9ca7d306dfe408d20ad3904. Revert "Importer: Delete orphaned album art on..." This reverts commit 629a80a1d12818cfa0de0b8b8b8a0e24d753215a. --- beets/importer.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 8aecf54a3..95163b6fd 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -547,23 +547,6 @@ class ImportTask(BaseImportTask): items = self.imported_items() - # If any original album art files were reused, leave them untouched. - for item in items: - album = item.get_album() - if album: - self.old_art_paths.discard(album.artpath) - - # Delete remaining original album art files. - for old_art_path in self.old_art_paths: - log.debug('Deleting orphaned album art: {0}', old_art_path) - util.remove(syspath(old_art_path), True) - # XXX: Ideally the directory would get pruned with self.prune() - # below, but when calling `beet import -L`, self.toppath is set - # to None, making self.prune() a no-op; hence this workaround of - # calling util.prune_dirs() directly. - util.prune_dirs(os.path.dirname(old_art_path), - clutter=config['clutter'].as_str_seq()) - # When copying and deleting originals, delete old files. if copy and delete: new_paths = [os.path.realpath(item.path) for item in items] @@ -669,11 +652,6 @@ class ImportTask(BaseImportTask): # Save the original paths of all items for deletion and pruning # in the next step (finalization). self.old_paths = [item.path for item in items] - - # Keep track of paths of all original album art for the same reason. - self.old_art_paths = set(filter( - bool, (album.artpath for album in self.replaced_albums.values()))) - for item in items: if move or copy or link: # In copy and link modes, treat re-imports specially: From 21f926fb8928bc6a45a5c6879521095b337658d9 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 22:38:13 +0100 Subject: [PATCH 17/26] Add test for #314 Ensure that album art is preserved when an album is re-imported. --- test/test_importer.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/test_importer.py b/test/test_importer.py index 7b07ea331..b01a93459 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1592,6 +1592,18 @@ class ReimportTest(unittest.TestCase, ImportHelper): self.importer.run() self.assertEqual(self._item().added, 4747.0) + def test_reimported_item_preserves_art(self): + self._setup_session() + artpath = os.path.join(_common.RSRC, 'abbey.jpg') + replaced_album = self._album() + replaced_album.set_art(artpath) + replaced_album.store() + self.importer.run() + new_album = self._album() + new_artpath = new_album.art_destination(artpath) + self.assertEqual(new_album.artpath, new_artpath) + self.assertTrue(os.path.exists(new_artpath)) + class ImportPretendTest(_common.TestCase, ImportHelper): """ Test the pretend commandline option From 32934bd16a61b8b676e54a8c2699ccf936ae6e50 Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 22:43:05 +0100 Subject: [PATCH 18/26] Preserve album art on re-import (fixes #314) Copy the replaced album's artpath attribute to the new album. This causes the image file to be moved along with the music files. --- beets/importer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/beets/importer.py b/beets/importer.py index 95163b6fd..85d6e0824 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -718,6 +718,7 @@ class ImportTask(BaseImportTask): if replaced_album: self.album.added = replaced_album.added self.album.update(replaced_album._values_flex) + self.album.artpath = replaced_album.artpath self.album.store() log.debug( u'Reimported album: added {0}, flexible ' From 314dd0e6bc13870f954f80c316ac6b93436e3eab Mon Sep 17 00:00:00 2001 From: reiv Date: Tue, 3 Nov 2015 23:10:01 +0100 Subject: [PATCH 19/26] Update re-import test to leave no orphaned art Make sure that when an album is re-imported and its files are moved, the artwork isn't left behind in the old folder. --- test/test_importer.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index b01a93459..675ac7796 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1594,15 +1594,18 @@ class ReimportTest(unittest.TestCase, ImportHelper): def test_reimported_item_preserves_art(self): self._setup_session() - artpath = os.path.join(_common.RSRC, 'abbey.jpg') + art_source = os.path.join(_common.RSRC, 'abbey.jpg') replaced_album = self._album() - replaced_album.set_art(artpath) + replaced_album.set_art(art_source) replaced_album.store() + old_artpath = replaced_album.artpath self.importer.run() new_album = self._album() - new_artpath = new_album.art_destination(artpath) + new_artpath = new_album.art_destination(art_source) self.assertEqual(new_album.artpath, new_artpath) self.assertTrue(os.path.exists(new_artpath)) + if new_artpath != old_artpath: + self.assertFalse(os.path.exists(old_artpath)) class ImportPretendTest(_common.TestCase, ImportHelper): From 023a33ca3147a5448c3377bf818422008dee5602 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Nov 2015 22:29:33 -0800 Subject: [PATCH 20/26] Revert "Fix #1656, maybe: encode Discogs token strings" This reverts commit f784cd1a226fa1161ee3aacb44ddd3a1475b8af9. --- beetsplug/discogs.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 904220685..80b7ae3df 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -39,8 +39,7 @@ import os urllib3_logger = logging.getLogger('requests.packages.urllib3') urllib3_logger.setLevel(logging.CRITICAL) -USER_AGENT = 'beets/{0} +http://beets.radbox.org/'.format(beets.__version__) \ - .encode('utf8') +USER_AGENT = u'beets/{0} +http://beets.radbox.org/'.format(beets.__version__) # Exceptions that discogs_client should really handle but does not. CONNECTION_ERRORS = (ConnectionError, socket.error, httplib.HTTPException, @@ -66,8 +65,8 @@ class DiscogsPlugin(BeetsPlugin): def setup(self, session=None): """Create the `discogs_client` field. Authenticate if necessary. """ - c_key = self.config['apikey'].get(unicode).encode('utf8') - c_secret = self.config['apisecret'].get(unicode).encode('utf8') + c_key = self.config['apikey'].get(unicode) + c_secret = self.config['apisecret'].get(unicode) # Get the OAuth token from a file or log in. try: @@ -77,8 +76,8 @@ class DiscogsPlugin(BeetsPlugin): # No token yet. Generate one. token, secret = self.authenticate(c_key, c_secret) else: - token = tokendata['token'].encode('utf8') - secret = tokendata['secret'].encode('utf8') + token = tokendata['token'] + secret = tokendata['secret'] self.discogs_client = Client(USER_AGENT, c_key, c_secret, token, secret) @@ -121,7 +120,7 @@ class DiscogsPlugin(BeetsPlugin): with open(self._tokenfile(), 'w') as f: json.dump({'token': token, 'secret': secret}, f) - return token.encode('utf8'), secret.encode('utf8') + return token, secret def album_distance(self, items, album_info, mapping): """Returns the album distance. From a735a6b4a20b9d65c294834518f76e0b74b5212b Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Nov 2015 22:38:31 -0800 Subject: [PATCH 21/26] discogs: Better exception logging Might have helped diagnose #1669 more easily. --- beetsplug/discogs.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 80b7ae3df..d8568bfd1 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -150,8 +150,8 @@ class DiscogsPlugin(BeetsPlugin): return self.candidates(items, artist, album, va_likely) else: return [] - except CONNECTION_ERRORS as e: - self._log.debug(u'HTTP Connection Error: {0}', e) + except CONNECTION_ERRORS: + self._log.debug('Connection error in album search', exc_info=True) return [] def album_for_id(self, album_id): @@ -181,8 +181,8 @@ class DiscogsPlugin(BeetsPlugin): self.reset_auth() return self.album_for_id(album_id) return None - except CONNECTION_ERRORS as e: - self._log.debug(u'HTTP Connection Error: {0}', e) + except CONNECTION_ERRORS: + self._log.debug('Connection error in album lookup', exc_info=True) return None return self.get_album_info(result) @@ -203,9 +203,9 @@ class DiscogsPlugin(BeetsPlugin): try: releases = self.discogs_client.search(query, type='release').page(1) - except CONNECTION_ERRORS as exc: - self._log.debug("Communication error while searching for {0!r}: " - "{1}".format(query, exc)) + except CONNECTION_ERRORS: + self._log.debug("Communication error while searching for {0!r}", + query, exc_info=True) return [] return [self.get_album_info(release) for release in releases[:5]] From 0ebc4c799d8f50da3c986723f8b2a9fa25f3f826 Mon Sep 17 00:00:00 2001 From: reiv Date: Wed, 4 Nov 2015 15:08:42 +0100 Subject: [PATCH 22/26] fetchart: in auto, ignore albums with art When re-importing an album, we don't want fetchart to interfere with any existing album art. --- beetsplug/fetchart.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index b807891e0..2c20dd698 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -430,6 +430,9 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): def fetch_art(self, session, task): """Find art for the album being imported.""" if task.is_album: # Only fetch art for full albums. + if task.album.artpath and os.path.isfile(task.album.artpath): + # Album already has art (probably a re-import); skip it. + return if task.choice_flag == importer.action.ASIS: # For as-is imports, don't search Web sources for art. local = True From e32dda78b892289de4e4d4790144bdffe1b6b69f Mon Sep 17 00:00:00 2001 From: reiv Date: Wed, 4 Nov 2015 18:33:52 +0100 Subject: [PATCH 23/26] Changelog for #1681 --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 4eafe9c90..d71efac17 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -16,6 +16,10 @@ Fixes: when there were not. :bug:`1652` * :doc:`plugins/lastgenre`: Clean up the reggae related genres somewhat. Thanks to :user:`Freso`. :bug:`1661` +* The importer now correctly moves album art files when re-importing. + :bug:`314` +* :doc:`/plugins/fetchart`: In auto mode, skips albums that already have + art attached to them so as not to interfere with re-imports. :bug:`314` 1.3.15 (October 17, 2015) From be499558843450a23649a49a9508bbea0ee6fe4e Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 4 Nov 2015 20:07:49 -0800 Subject: [PATCH 24/26] metasync: More precise exception catching (#1700) --- beetsplug/metasync/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/beetsplug/metasync/__init__.py b/beetsplug/metasync/__init__.py index 380c72707..fd40590bb 100644 --- a/beetsplug/metasync/__init__.py +++ b/beetsplug/metasync/__init__.py @@ -111,11 +111,13 @@ class MetaSyncPlugin(BeetsPlugin): # Instantiate the meta sources for player in sources: try: - meta_source_instances[player] = \ - META_SOURCES[player](self.config, self._log) + cls = META_SOURCES[player] except KeyError: self._log.error(u'Unknown metadata source \'{0}\''.format( player)) + + try: + meta_source_instances[player] = cls(self.config, self._log) except (ImportError, ConfigValueError) as e: self._log.error(u'Failed to instantiate metadata source ' u'\'{0}\': {1}'.format(player, e)) From 19cf37263db792f979be1bc5951be4b3651b82fa Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 4 Nov 2015 20:11:58 -0800 Subject: [PATCH 25/26] Fix #1700: Tolerate missing Location in iTunes --- beetsplug/metasync/itunes.py | 3 ++- docs/changelog.rst | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/beetsplug/metasync/itunes.py b/beetsplug/metasync/itunes.py index b2795f9a4..17139f549 100644 --- a/beetsplug/metasync/itunes.py +++ b/beetsplug/metasync/itunes.py @@ -94,7 +94,8 @@ class Itunes(MetaSource): # Make the iTunes library queryable using the path self.collection = {_norm_itunes_path(track['Location']): track - for track in raw_library['Tracks'].values()} + for track in raw_library['Tracks'].values() + if 'Location' in track} def sync_from_source(self, item): result = self.collection.get(util.bytestring_path(item.path).lower()) diff --git a/docs/changelog.rst b/docs/changelog.rst index 666a76641..eb423079e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,8 @@ Fixes: * :doc:`/plugins/thumbnails`: Fix a crash with Unicode paths. :bug:`1686` * :doc:`/plugins/embedart`: The ``remove_art_file`` option now works on import (as well as with the explicit command). :bug:`1662` :bug:`1675` +* :doc:`/plugins/metasync`: Fix a crash when syncing with recent versions of + iTunes. :bug:`1700` 1.3.15 (October 17, 2015) From 7d52fa72aef6c94fa99b10489531693312cec5f9 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Wed, 4 Nov 2015 20:25:17 -0800 Subject: [PATCH 26/26] Fix #1699: Outdated API call get_fields() was removed in 54205998391b284801dba4cd1d595a591f23282a. --- beetsplug/duplicates.py | 2 +- docs/changelog.rst | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/beetsplug/duplicates.py b/beetsplug/duplicates.py index 991fec935..a8ad4ac90 100644 --- a/beetsplug/duplicates.py +++ b/beetsplug/duplicates.py @@ -271,7 +271,7 @@ class DuplicatesPlugin(BeetsPlugin): Return same number of items, with the head item modified. """ - fields = [f for sublist in Item.get_fields() for f in sublist] + fields = Item.all_keys() for f in fields: for o in objs[1:]: if getattr(objs[0], f, None) in (None, ''): diff --git a/docs/changelog.rst b/docs/changelog.rst index eb423079e..ac4202561 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -30,6 +30,7 @@ Fixes: (as well as with the explicit command). :bug:`1662` :bug:`1675` * :doc:`/plugins/metasync`: Fix a crash when syncing with recent versions of iTunes. :bug:`1700` +* :doc:`/plugins/duplicates`: Fix a crash when merging items. :bug:`1699` 1.3.15 (October 17, 2015)