From 2c1e4d878b79ca3bdbba125d3e17b4139a56c609 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Fri, 10 Nov 2017 11:17:27 +0100 Subject: [PATCH 1/9] Advanced fetchart source config: add the (still unused) match_by constructor argument --- beetsplug/fetchart.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index 8bee6e804..e4a12b84e 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -192,9 +192,12 @@ class RequestMixin(object): # ART SOURCES ################################################################ class ArtSource(RequestMixin): - def __init__(self, log, config): + VALID_MATCHING_CRITERIA = ['default'] + + def __init__(self, log, config, match_by=None): self._log = log self._config = config + self.match_by = match_by or self.VALID_MATCHING_CRITERIA def get(self, album, plugin, paths): raise NotImplementedError() @@ -289,6 +292,7 @@ class RemoteArtSource(ArtSource): class CoverArtArchive(RemoteArtSource): NAME = u"Cover Art Archive" + VALID_MATCHING_CRITERIA = ['release', 'releasegroup'] if util.SNI_SUPPORTED: URL = 'https://coverartarchive.org/release/{mbid}/front' @@ -301,10 +305,10 @@ class CoverArtArchive(RemoteArtSource): """Return the Cover Art Archive and Cover Art Archive release group URLs using album MusicBrainz release ID and release group ID. """ - if album.mb_albumid: + if 'release' in self.match_by and album.mb_albumid: yield self._candidate(url=self.URL.format(mbid=album.mb_albumid), match=Candidate.MATCH_EXACT) - if album.mb_releasegroupid: + if 'releasegroup' in self.match_by and album.mb_releasegroupid: yield self._candidate( url=self.GROUP_URL.format(mbid=album.mb_releasegroupid), match=Candidate.MATCH_FALLBACK) @@ -770,7 +774,7 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): sources_name.append(u'filesystem') except ValueError: pass - self.sources = [ART_SOURCES[s](self._log, self.config) + self.sources = [ART_SOURCES[s](self._log, self.config, None) for s in sources_name] # Asynchronous; after music is added to the library. From 971584b4b23978502d7b174a799fb8177e68e020 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Mon, 13 Nov 2017 01:54:39 +0100 Subject: [PATCH 2/9] Improve readability of plugins.sanitize_choices --- beets/plugins.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index 4a2475f50..1dc1f6496 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -502,9 +502,12 @@ def sanitize_choices(choices, choices_all): others = [x for x in choices_all if x not in choices] res = [] for s in choices: - if s in list(choices_all) + ['*']: - if not (s in seen or seen.add(s)): - res.extend(list(others) if s == '*' else [s]) + if s not in seen: + if s in list(choices_all): + res.append(s) + elif s == '*': + res.extend(others) + seen.add(s) return res From 5a043f28bd99ddb630967eded0a0f74dbf46f4b6 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Mon, 13 Nov 2017 01:55:15 +0100 Subject: [PATCH 3/9] Advanced fetchart source config: add validation function --- beets/plugins.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/beets/plugins.py b/beets/plugins.py index 1dc1f6496..5e71c532e 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -511,6 +511,44 @@ def sanitize_choices(choices, choices_all): return res +def sanitize_pairs(pairs, pairs_all): + """Clean up a single-element mapping configuration attribute as returned + by `confit`'s `Pairs` template: keep only two-element tuples present in + pairs_all, remove duplicate elements, expand ('str', '*') and ('*', '*') + wildcards while keeping the original order. Note that ('*', '*') and + ('*', 'whatever') have the same effect. + + For example, + + >>> sanitize_pairs( + ... [('foo', 'baz bar'), ('key', '*'), ('*', '*')], + ... [('foo', 'bar'), ('foo', 'baz'), ('foo', 'foobar'), + ... ('key', 'value')] + ... ) + [('foo', 'baz'), ('foo', 'bar'), ('key', 'value'), ('foo', 'foobar')] + """ + pairs_all = list(pairs_all) + seen = set() + others = [x for x in pairs_all if x not in pairs] + res = [] + for k, values in pairs: + for v in values.split(): + x = (k, v) + if x in pairs_all: + if not x in seen: + seen.add(x) + res.append(x) + elif k == '*': + new = [o for o in others if o not in seen] + seen.update(new) + res.extend(new) + elif v == '*': + new = [o for o in others if o not in seen and o[0] == k] + seen.update(new) + res.extend(new) + return res + + def notify_info_yielded(event): """Makes a generator send the event 'event' every time it yields. This decorator is supposed to decorate a generator, but any function From 60bffbadbdee3652f65ff495a0436797a5296f71 Mon Sep 17 00:00:00 2001 From: wordofglass Date: Mon, 13 Nov 2017 01:56:57 +0100 Subject: [PATCH 4/9] Advanced fetchart source config: write (restore?) confit' as_pairs() --- beets/util/confit.py | 79 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 14 deletions(-) diff --git a/beets/util/confit.py b/beets/util/confit.py index 73ae97abc..7b5d39f29 100644 --- a/beets/util/confit.py +++ b/beets/util/confit.py @@ -413,6 +413,12 @@ class ConfigView(object): """ return self.get(StrSeq(split=split)) + def as_pairs(self, default_value=None): + """Get the value as a sequence of pairs of two strings. Equivalent to + `get(Pairs())`. + """ + return self.get(Pairs(default_value=default_value)) + def as_str(self): """Get the value as a (Unicode) string. Equivalent to `get(unicode)` on Python 2 and `get(str)` on Python 3. @@ -1242,30 +1248,75 @@ class StrSeq(Template): super(StrSeq, self).__init__() self.split = split + def _convert_value(self, x, view): + if isinstance(x, STRING): + return x + elif isinstance(x, bytes): + return x.decode('utf-8', 'ignore') + else: + self.fail(u'must be a list of strings', view, True) + def convert(self, value, view): if isinstance(value, bytes): value = value.decode('utf-8', 'ignore') if isinstance(value, STRING): if self.split: - return value.split() + value = value.split() else: - return [value] + value = [value] + else: + try: + value = list(value) + except TypeError: + self.fail(u'must be a whitespace-separated string or a list', + view, True) + return [self._convert_value(v, view) for v in value] + + +class Pairs(StrSeq): + """A template for ordered key-value pairs. + + This can either be given with the same syntax as for `StrSeq` (i.e. without + values), or as a list of strings and/or single-element mappings such as:: + + - key: value + - [key, value] + - key + + The result is a list of two-element tuples. If no value is provided, the + `default_value` will be returned as the second element. + """ + + def __init__(self, default_value=None): + """Create a new template. + + `default` is the dictionary value returned for items that are not + a mapping, but a single string. + """ + super(Pairs, self).__init__(split=True) + self.default_value = default_value + + def _convert_value(self, x, view): try: - value = list(value) - except TypeError: - self.fail(u'must be a whitespace-separated string or a list', - view, True) - - def convert(x): - if isinstance(x, STRING): - return x - elif isinstance(x, bytes): - return x.decode('utf-8', 'ignore') + return (super(Pairs, self)._convert_value(x, view), + self.default_value) + except ConfigTypeError: + if isinstance(x, collections.Mapping): + if len(x) != 1: + self.fail(u'must be a single-element mapping', view, True) + k, v = iter_first(x.items()) + elif isinstance(x, collections.Sequence): + if len(x) != 2: + self.fail(u'must be a two-element list', view, True) + k, v = x else: - self.fail(u'must be a list of strings', view, True) - return list(map(convert, value)) + # Is this even possible? -> Likely, if some !directive cause + # YAML to parse this to some custom type. + self.fail(u'must be a single string, mapping, or a list' + str(x), view, True) + return (super(Pairs, self)._convert_value(k, view), + super(Pairs, self)._convert_value(v, view)) class Filename(Template): From e7a3e27ed9528fb175579eb602b1ba29638b4ebe Mon Sep 17 00:00:00 2001 From: wordofglass Date: Mon, 13 Nov 2017 02:03:13 +0100 Subject: [PATCH 5/9] Advanced fetchart source config: Actually use the new syntax --- beetsplug/fetchart.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index e4a12b84e..b98a68593 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -761,21 +761,30 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): if not self.config['google_key'].get() and \ u'google' in available_sources: available_sources.remove(u'google') - sources_name = plugins.sanitize_choices( - self.config['sources'].as_str_seq(), available_sources) + available_sources = [(s, c) + for s in available_sources + for c in ART_SOURCES[s].VALID_MATCHING_CRITERIA] + sources = plugins.sanitize_pairs( + self.config['sources'].as_pairs(default_value='*'), + available_sources) + if 'remote_priority' in self.config: self._log.warning( u'The `fetch_art.remote_priority` configuration option has ' u'been deprecated. Instead, place `filesystem` at the end of ' u'your `sources` list.') if self.config['remote_priority'].get(bool): - try: - sources_name.remove(u'filesystem') - sources_name.append(u'filesystem') - except ValueError: - pass - self.sources = [ART_SOURCES[s](self._log, self.config, None) - for s in sources_name] + fs = [] + others = [] + for s, c in sources: + if s == 'filesystem': + fs.append((s, c)) + else: + others.append((s, c)) + sources = others + fs + + self.sources = [ART_SOURCES[s](self._log, self.config, match_by=[c]) + for s, c in sources] # Asynchronous; after music is added to the library. def fetch_art(self, session, task): From 318f0c4d16710712ed49d10c621fbf52705161dd Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Fri, 23 Feb 2018 16:00:41 +0100 Subject: [PATCH 6/9] Advanced fetchart source config: pep8 --- beets/plugins.py | 2 +- beets/util/confit.py | 4 +++- beetsplug/fetchart.py | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index 5e71c532e..1bd2cacd5 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -535,7 +535,7 @@ def sanitize_pairs(pairs, pairs_all): for v in values.split(): x = (k, v) if x in pairs_all: - if not x in seen: + if x not in seen: seen.add(x) res.append(x) elif k == '*': diff --git a/beets/util/confit.py b/beets/util/confit.py index 7b5d39f29..b5513f48e 100644 --- a/beets/util/confit.py +++ b/beets/util/confit.py @@ -1314,7 +1314,9 @@ class Pairs(StrSeq): else: # Is this even possible? -> Likely, if some !directive cause # YAML to parse this to some custom type. - self.fail(u'must be a single string, mapping, or a list' + str(x), view, True) + self.fail(u'must be a single string, mapping, or a list' + u'' + str(x), + view, True) return (super(Pairs, self)._convert_value(k, view), super(Pairs, self)._convert_value(v, view)) diff --git a/beetsplug/fetchart.py b/beetsplug/fetchart.py index b98a68593..0e106694d 100644 --- a/beetsplug/fetchart.py +++ b/beetsplug/fetchart.py @@ -762,8 +762,8 @@ class FetchArtPlugin(plugins.BeetsPlugin, RequestMixin): u'google' in available_sources: available_sources.remove(u'google') available_sources = [(s, c) - for s in available_sources - for c in ART_SOURCES[s].VALID_MATCHING_CRITERIA] + for s in available_sources + for c in ART_SOURCES[s].VALID_MATCHING_CRITERIA] sources = plugins.sanitize_pairs( self.config['sources'].as_pairs(default_value='*'), available_sources) From dee288545758e9670c816c964b5fb6461f293296 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sat, 24 Feb 2018 10:54:50 +0100 Subject: [PATCH 7/9] docs/plugins/fetchart: fix internal link target location --- docs/plugins/fetchart.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plugins/fetchart.rst b/docs/plugins/fetchart.rst index e375f9f57..85d22078a 100644 --- a/docs/plugins/fetchart.rst +++ b/docs/plugins/fetchart.rst @@ -104,8 +104,6 @@ already have it; the ``-f`` or ``--force`` switch makes it search for art in Web databases regardless. If you specify a query, only matching albums will be processed; otherwise, the command processes every album in your library. -.. _image-resizing: - Display Only Missing Album Art ------------------------------ @@ -117,6 +115,8 @@ art:: By default the command will display all results, the ``-q`` or ``--quiet`` switch will only display results for album arts that are still missing. +.. _image-resizing: + Image Resizing -------------- From db85c28c7f4899e3db1c650b0fb04ff59a06c0ab Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sat, 24 Feb 2018 10:55:29 +0100 Subject: [PATCH 8/9] Advanced fetchart source config: documentation --- docs/plugins/fetchart.rst | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/plugins/fetchart.rst b/docs/plugins/fetchart.rst index 85d22078a..58ca0b1f4 100644 --- a/docs/plugins/fetchart.rst +++ b/docs/plugins/fetchart.rst @@ -54,7 +54,8 @@ file. The available options are: matches at the cost of some speed. They are searched in the given order, thus in the default config, no remote (Web) art source are queried if local art is found in the filesystem. To use a local image as fallback, - move it to the end of the list. + move it to the end of the list. For even more fine-grained control over + the search order, see the section on :ref:`album-art-sources` below. - **google_key**: Your Google API key (to enable the Google Custom Search backend). Default: None. @@ -135,6 +136,8 @@ environment variable so that ImageMagick comes first or use Pillow instead. .. _Pillow: https://github.com/python-pillow/Pillow .. _ImageMagick: http://www.imagemagick.org/ +.. _album-art-sources: + Album Art Sources ----------------- @@ -150,6 +153,25 @@ file whose name contains "cover", "front", "art", "album" or "folder", but in the absence of well-known names, it will use any image file in the same folder as your music files. +For some of the art sources, the backend service can match artwork by various +criteria. If you want finer control over the search order in such cases, the +following alternative syntax for the ``sources`` option can be used:: + + fetchart: + sources: + - filesystem + - coverart: release + - itunes + - coverart: releasegroup + - '*' + +where listing a source without matching criteria will default to trying all +available strategies. Entries of the forms ``coverart: release releasegroup`` +and ``coverart: *`` are also valid. +Currently, the ``coverart`` source is the only backend to support several +such values, namely ``release`` and ``releasegroup``, which refer to the +respective MusicBrainz IDs. + When you choose to apply changes during an import, beets will search for art as described above. For "as-is" imports (and non-autotagged imports using the ``-A`` flag), beets only looks for art on the local filesystem. From a6ae1570f92437276f2fb926d32bf88badf92422 Mon Sep 17 00:00:00 2001 From: wisp3rwind <17089248+wisp3rwind@users.noreply.github.com> Date: Sat, 24 Feb 2018 11:29:36 +0100 Subject: [PATCH 9/9] Advanced fetchart source config: changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index f423ea832..3121b3394 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -12,6 +12,9 @@ New features: recording skipped directories to the incremental list, so you can revisit them later. Thanks to :user:`sekjun9878`. :bug:`2773` +* :doc:`/plugins/fetchart`: extended syntax for the ``sources`` option to give + fine-grained control over the search order for backends with several matching + strategies. Fixes: