From 1da972f4bb28569b1bed4676cb4302aabe5baf8d Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Tue, 30 May 2017 16:15:28 +0200 Subject: [PATCH 1/9] implemented `set_field` cli parsing --- beets/config_default.yaml | 3 ++- beets/ui/__init__.py | 18 ++++++++++++++++++ beets/ui/commands.py | 6 ++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/beets/config_default.yaml b/beets/config_default.yaml index b7e6b1e2b..439a93f55 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -25,7 +25,8 @@ import: pretend: false search_ids: [] duplicate_action: ask - bell: no + bell: no + set_fields: {} clutter: ["Thumbs.DB", ".DS_Store"] ignore: [".*", "*~", "System Volume Information", "lost+found"] diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 60652fa08..0a215535b 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1060,6 +1060,24 @@ class SubcommandsOptionParser(CommonOptionsParser): suboptions, subargs = subcommand.parse_args(args) return subcommand, suboptions, subargs + @staticmethod + def _store_dict(option, opt_str, value, parser): + """Custom action callback to parse options which have ``key=value`` + pairs as values. All such pairs passed for this option are + aggregated into a dictionary. + """ + dest = option.dest + option_values = getattr(parser.values, dest, None) + + if option_values is None: + # This is the first supplied ``key=value`` pair of option. + # Initialize empty dictionary and get a reference to it. + setattr(parser.values, dest, dict()) + option_values = getattr(parser.values, dest) + + key, value = value.split('=') + option_values[key] = value + optparse.Option.ALWAYS_TYPED_ACTIONS += ('callback',) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 995ff87e9..39e2bcb45 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -40,6 +40,7 @@ from beets import config from beets import logging from beets.util.confit import _package_path import six +from . import SubcommandsOptionParser VARIOUS_ARTISTS = u'Various Artists' PromptChoice = namedtuple('PromptChoice', ['short', 'long', 'callback']) @@ -1017,6 +1018,11 @@ import_cmd.parser.add_option( metavar='ID', help=u'restrict matching to a specific metadata backend ID' ) +import_cmd.parser.add_option( + u'--set-field', dest='set_fields', action='callback', callback=SubcommandsOptionParser._store_dict, + metavar='FIELD=VALUE', + help=u'set the given fields to the supplied values after importing' +) import_cmd.func = import_func default_commands.append(import_cmd) From 569098a31804a72529e25d3a3a47cd9df7d1bb51 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Tue, 30 May 2017 16:56:33 +0200 Subject: [PATCH 2/9] store_dict parser action now stores unicode, and implementend set_fields functionality in importer --- beets/importer.py | 28 +++++++++++++++++++++++++++- beets/ui/__init__.py | 2 +- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index ce9ebc01c..801e0cdca 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -419,7 +419,7 @@ class ImportTask(BaseImportTask): from the `candidates` list. * `find_duplicates()` Returns a list of albums from `lib` with the - same artist and album name as the task. + same artist and album name as the task. * `apply_metadata()` Sets the attributes of the items from the task's `match` attribute. @@ -429,6 +429,9 @@ class ImportTask(BaseImportTask): * `manipulate_files()` Copy, move, and write files depending on the session configuration. + * `set_fields()` Sets the fields given at CLI or configuration to + the specified values. + * `finalize()` Update the import progress and cleanup the file system. """ @@ -529,6 +532,14 @@ class ImportTask(BaseImportTask): util.remove(item.path) util.prune_dirs(os.path.dirname(item.path), lib.directory) + def set_fields(self): + set_fields_dict = config['import']['set_fields'] + for field in set_fields_dict.keys(): + print(type(set_fields_dict[field].get())) + value = set_fields_dict[field].get() + log.debug(u'Set field {1}={2} for {0}', displayable_path(self.paths), field, value) + self.album[field] = value + self.album.store() def finalize(self, session): """Save progress, clean up files, and emit plugin event. @@ -877,6 +888,16 @@ class SingletonImportTask(ImportTask): def reload(self): self.item.load() + def set_fields(self): + """Similar to ``ImportTask.set_fields``, but for singleton items. + """ + set_fields_dict = config['import']['set_fields'] + for field in set_fields_dict.keys(): + value = set_fields_dict[field].get() + log.debug(u'Set field {1}={2} for {0}', displayable_path(self.paths), field, value) + self.item[field] = value + self.item.store() + # FIXME The inheritance relationships are inverted. This is why there # are so many methods which pass. More responsibility should be delegated to @@ -1385,6 +1406,11 @@ def apply_choice(session, task): task.add(session.lib) + # If ``set_fields`` is set, set those fields to the + # configured values. + if config['import']['set_fields']: + task.set_fields() + @pipeline.mutator_stage def plugin_stage(session, func, task): diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 0a215535b..7091dbbaf 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1075,7 +1075,7 @@ class SubcommandsOptionParser(CommonOptionsParser): setattr(parser.values, dest, dict()) option_values = getattr(parser.values, dest) - key, value = value.split('=') + key, value = map(lambda s: util.text_string(s), value.split('=')) option_values[key] = value From 91722aea36753de6658a03451e9e9b742a7cae41 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Tue, 30 May 2017 21:29:49 +0200 Subject: [PATCH 3/9] added documentation --- beets/importer.py | 12 +++++++++--- beets/ui/commands.py | 5 +++-- docs/changelog.rst | 5 +++++ docs/reference/cli.rst | 11 +++++++++++ docs/reference/config.rst | 20 ++++++++++++++++++++ 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 801e0cdca..247b46968 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -532,12 +532,15 @@ class ImportTask(BaseImportTask): util.remove(item.path) util.prune_dirs(os.path.dirname(item.path), lib.directory) + def set_fields(self): set_fields_dict = config['import']['set_fields'] for field in set_fields_dict.keys(): - print(type(set_fields_dict[field].get())) value = set_fields_dict[field].get() - log.debug(u'Set field {1}={2} for {0}', displayable_path(self.paths), field, value) + log.debug(u'Set field {1}={2} for {0}', + displayable_path(self.paths), + field, + value) self.album[field] = value self.album.store() @@ -894,7 +897,10 @@ class SingletonImportTask(ImportTask): set_fields_dict = config['import']['set_fields'] for field in set_fields_dict.keys(): value = set_fields_dict[field].get() - log.debug(u'Set field {1}={2} for {0}', displayable_path(self.paths), field, value) + log.debug(u'Set field {1}={2} for {0}', + displayable_path(self.paths), + field, + value) self.item[field] = value self.item.store() diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 39e2bcb45..240c776ee 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1019,9 +1019,10 @@ import_cmd.parser.add_option( help=u'restrict matching to a specific metadata backend ID' ) import_cmd.parser.add_option( - u'--set-field', dest='set_fields', action='callback', callback=SubcommandsOptionParser._store_dict, + u'--set-field', dest='set_fields', action='callback', + callback=SubcommandsOptionParser._store_dict, metavar='FIELD=VALUE', - help=u'set the given fields to the supplied values after importing' + help=u'set the given fields to the supplied values' ) import_cmd.func = import_func default_commands.append(import_cmd) diff --git a/docs/changelog.rst b/docs/changelog.rst index caea9c6a4..0da327ac4 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -61,6 +61,11 @@ New features: MusicBrainz. Thanks to :user:`dosoe`. :bug:`2519` :bug:`2529` +* It is now possible to set fields to certain values during import, using + either the `importer.set_fields` dictionary in the config file, or by + passing one or more `--set-field field=value` options on the command-line. + :bug: `1881` + Fixes: diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index b4c7b9e1b..b9f29c9c9 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -138,6 +138,17 @@ Optional command flags: searching for other candidates by using the ``--search-id SEARCH_ID`` option. Multiple IDs can be specified by simply repeating the option several times. +* You can supply ``--set-field`` options with ``field=value`` pairs to assign to + those fields the specified values on import, in addition to such field/value + pairs defined in the ``importer.set_fields`` dictionary in the configuration + file. Make sure to use an option per field/value pair, like so: + :: + + beet import --set-field genre="Alternative Rock" --set-field mood="emotional" + + Note that values for the fields specified on the command-line override the + ones defined for those fields in the configuration file. + .. _rarfile: https://pypi.python.org/pypi/rarfile/2.2 .. only:: html diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 82c11238a..1018b9bb4 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -586,6 +586,26 @@ Ring the terminal bell to get your attention when the importer needs your input. Default: ``no``. +.. _set_fields: + +set_fields +~~~~~~~~~~ + +A dictionary of field/value pairs, each one used to set a field to the +corresponding value during import. + +Example: :: + + set_fields: + genre: 'To Listen' + collection: 'Unordered' + +Note that field/value pairs supplied via ``--set-field`` options on the +command-line are processed in addition to those specified here. Those values +override the ones defined here in the case of fields with the same name. + +Default: ``{}`` (empty). + .. _musicbrainz-config: MusicBrainz Options From b5a8891872d7e4b386a1cf9d381cde5706482503 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Wed, 31 May 2017 08:17:59 +0200 Subject: [PATCH 4/9] implemented error handling for invalid, non-key/value cli argumnets --- beets/ui/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 7091dbbaf..2cea00be7 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1075,7 +1075,15 @@ class SubcommandsOptionParser(CommonOptionsParser): setattr(parser.values, dest, dict()) option_values = getattr(parser.values, dest) - key, value = map(lambda s: util.text_string(s), value.split('=')) + try: + key, value = map(lambda s: util.text_string(s), value.split('=')) + if not (key and value): + raise ValueError + except ValueError: + raise UserError( + "supplied argument `{0}' is not of the form `key=value'" + .format(value)) + option_values[key] = value From c64878179eb007174203897050d485fb7549b215 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Thu, 1 Jun 2017 11:57:24 +0200 Subject: [PATCH 5/9] finished tests for set_fields on importer --- test/test_importer.py | 64 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/test/test_importer.py b/test/test_importer.py index 26dec3de8..cbfbda778 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -543,6 +543,38 @@ class ImportSingletonTest(_common.TestCase, ImportHelper): self.assertEqual(len(self.lib.items()), 2) self.assertEqual(len(self.lib.albums()), 2) + def test_set_fields(self): + genre = u"\U0001F3B7 Jazz" + collection = u"To Listen" + + config['import']['set_fields'] = { + u'collection': collection, + u'genre': genre + } + + # As-is item import. + self.assertEqual(self.lib.albums().get(), None) + self.importer.add_choice(importer.action.ASIS) + self.importer.run() + + for item in self.lib.items(): + item.load() # TODO: Not sure this is necessary. + self.assertEqual(item.genre, genre) + self.assertEqual(item.collection, collection) + # Remove item from library to test again with APPLY choice. + item.remove() + + # Autotagged. + self.assertEqual(self.lib.albums().get(), None) + self.importer.clear_choices() + self.importer.add_choice(importer.action.APPLY) + self.importer.run() + + for item in self.lib.items(): + item.load() + self.assertEqual(item.genre, genre) + self.assertEqual(item.collection, collection) + class ImportTest(_common.TestCase, ImportHelper): """Test APPLY, ASIS and SKIP choices. @@ -672,6 +704,38 @@ class ImportTest(_common.TestCase, ImportHelper): with self.assertRaises(AttributeError): self.lib.items().get().data_source + def test_set_fields(self): + genre = u"\U0001F3B7 Jazz" + collection = u"To Listen" + + config['import']['set_fields'] = { + u'collection': collection, + u'genre': genre + } + + # As-is album import. + self.assertEqual(self.lib.albums().get(), None) + self.importer.add_choice(importer.action.ASIS) + self.importer.run() + + for album in self.lib.albums(): + album.load() # TODO: Not sure this is necessary. + self.assertEqual(album.genre, genre) + self.assertEqual(album.collection, collection) + # Remove album from library to test again with APPLY choice. + album.remove() + + # Autotagged. + self.assertEqual(self.lib.albums().get(), None) + self.importer.clear_choices() + self.importer.add_choice(importer.action.APPLY) + self.importer.run() + + for album in self.lib.albums(): + album.load() + self.assertEqual(album.genre, genre) + self.assertEqual(album.collection, collection) + class ImportTracksTest(_common.TestCase, ImportHelper): """Test TRACKS and APPLY choice. From 52d5d2310b74d562faeca653b3d1e146d8df6b97 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Mon, 12 Jun 2017 16:46:09 +0200 Subject: [PATCH 6/9] refactoring according to feedback in pull request --- beets/ui/__init__.py | 53 ++++++++++++++++++++------------------- beets/ui/commands.py | 6 ++--- docs/changelog.rst | 2 +- docs/reference/cli.rst | 7 +++--- docs/reference/config.rst | 2 +- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 2cea00be7..8b0d57106 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -769,6 +769,33 @@ def show_path_changes(path_changes): pad = max_width - len(source) log.info(u'{0} {1} -> {2}', source, ' ' * pad, dest) +# Helper functions for option parsing. + +def _store_dict(option, opt_str, value, parser): + """Custom action callback to parse options which have ``key=value`` + pairs as values. All such pairs passed for this option are + aggregated into a dictionary. + """ + dest = option.dest + option_values = getattr(parser.values, dest, None) + + if option_values is None: + # This is the first supplied ``key=value`` pair of option. + # Initialize empty dictionary and get a reference to it. + setattr(parser.values, dest, dict()) + option_values = getattr(parser.values, dest) + + try: + key, value = map(lambda s: util.text_string(s), value.split('=')) + if not (key and value): + raise ValueError + except ValueError: + raise UserError( + "supplied argument `{0}' is not of the form `key=value'" + .format(value)) + + option_values[key] = value + class CommonOptionsParser(optparse.OptionParser, object): """Offers a simple way to add common formatting options. @@ -1060,32 +1087,6 @@ class SubcommandsOptionParser(CommonOptionsParser): suboptions, subargs = subcommand.parse_args(args) return subcommand, suboptions, subargs - @staticmethod - def _store_dict(option, opt_str, value, parser): - """Custom action callback to parse options which have ``key=value`` - pairs as values. All such pairs passed for this option are - aggregated into a dictionary. - """ - dest = option.dest - option_values = getattr(parser.values, dest, None) - - if option_values is None: - # This is the first supplied ``key=value`` pair of option. - # Initialize empty dictionary and get a reference to it. - setattr(parser.values, dest, dict()) - option_values = getattr(parser.values, dest) - - try: - key, value = map(lambda s: util.text_string(s), value.split('=')) - if not (key and value): - raise ValueError - except ValueError: - raise UserError( - "supplied argument `{0}' is not of the form `key=value'" - .format(value)) - - option_values[key] = value - optparse.Option.ALWAYS_TYPED_ACTIONS += ('callback',) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 240c776ee..0003efc5d 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -40,7 +40,7 @@ from beets import config from beets import logging from beets.util.confit import _package_path import six -from . import SubcommandsOptionParser +from . import _store_dict VARIOUS_ARTISTS = u'Various Artists' PromptChoice = namedtuple('PromptChoice', ['short', 'long', 'callback']) @@ -1019,8 +1019,8 @@ import_cmd.parser.add_option( help=u'restrict matching to a specific metadata backend ID' ) import_cmd.parser.add_option( - u'--set-field', dest='set_fields', action='callback', - callback=SubcommandsOptionParser._store_dict, + u'--set', dest='set_fields', action='callback', + callback=_store_dict, metavar='FIELD=VALUE', help=u'set the given fields to the supplied values' ) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0da327ac4..6fcdb5792 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -63,7 +63,7 @@ New features: :bug:`2519` :bug:`2529` * It is now possible to set fields to certain values during import, using either the `importer.set_fields` dictionary in the config file, or by - passing one or more `--set-field field=value` options on the command-line. + passing one or more `--set field=value` options on the command-line. :bug: `1881` diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index b9f29c9c9..38d4f40ca 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -138,13 +138,12 @@ Optional command flags: searching for other candidates by using the ``--search-id SEARCH_ID`` option. Multiple IDs can be specified by simply repeating the option several times. -* You can supply ``--set-field`` options with ``field=value`` pairs to assign to +* You can supply ``--set`` options with ``field=value`` pairs to assign to those fields the specified values on import, in addition to such field/value pairs defined in the ``importer.set_fields`` dictionary in the configuration - file. Make sure to use an option per field/value pair, like so: - :: + file. Make sure to use an option per field/value pair, like so:: - beet import --set-field genre="Alternative Rock" --set-field mood="emotional" + beet import --set genre="Alternative Rock" --set mood="emotional" Note that values for the fields specified on the command-line override the ones defined for those fields in the configuration file. diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 1018b9bb4..bcd30169b 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -600,7 +600,7 @@ Example: :: genre: 'To Listen' collection: 'Unordered' -Note that field/value pairs supplied via ``--set-field`` options on the +Note that field/value pairs supplied via ``--set`` options on the command-line are processed in addition to those specified here. Those values override the ones defined here in the case of fields with the same name. From 53d0421d4350c6fb6771cb0a0a808289cb9e58a0 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Mon, 12 Jun 2017 16:51:22 +0200 Subject: [PATCH 7/9] added docstring for set_fields methods --- beets/importer.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/beets/importer.py b/beets/importer.py index 247b46968..6ceb4f831 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -534,6 +534,9 @@ class ImportTask(BaseImportTask): lib.directory) def set_fields(self): + """Sets the fields given at CLI or configuration to the specified + values. + """ set_fields_dict = config['import']['set_fields'] for field in set_fields_dict.keys(): value = set_fields_dict[field].get() @@ -892,7 +895,8 @@ class SingletonImportTask(ImportTask): self.item.load() def set_fields(self): - """Similar to ``ImportTask.set_fields``, but for singleton items. + """Sets the fields given at CLI or configuration to the specified + values. """ set_fields_dict = config['import']['set_fields'] for field in set_fields_dict.keys(): From 24658989daa80f0c714c84d3a8d6af7d42e5e607 Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Mon, 12 Jun 2017 17:02:41 +0200 Subject: [PATCH 8/9] refactoring according to suggestions in pull request --- beets/importer.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 6ceb4f831..d36a5a0a0 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -537,15 +537,14 @@ class ImportTask(BaseImportTask): """Sets the fields given at CLI or configuration to the specified values. """ - set_fields_dict = config['import']['set_fields'] - for field in set_fields_dict.keys(): - value = set_fields_dict[field].get() + for field, view in config['import']['set_fields'].items(): + value = view.get() log.debug(u'Set field {1}={2} for {0}', displayable_path(self.paths), field, value) self.album[field] = value - self.album.store() + self.album.store() def finalize(self, session): """Save progress, clean up files, and emit plugin event. @@ -898,15 +897,14 @@ class SingletonImportTask(ImportTask): """Sets the fields given at CLI or configuration to the specified values. """ - set_fields_dict = config['import']['set_fields'] - for field in set_fields_dict.keys(): - value = set_fields_dict[field].get() + for field, view in config['import']['set_fields'].items(): + value = view.get() log.debug(u'Set field {1}={2} for {0}', displayable_path(self.paths), field, value) self.item[field] = value - self.item.store() + self.item.store() # FIXME The inheritance relationships are inverted. This is why there @@ -1418,6 +1416,9 @@ def apply_choice(session, task): # If ``set_fields`` is set, set those fields to the # configured values. + # NOTE: This cannot be done before the ``task.add()`` call above, + # because then the ``ImportTask`` won't have an `album` for which + # it can set the fields. if config['import']['set_fields']: task.set_fields() From 762f9ca054782f368051347967878c89995182ad Mon Sep 17 00:00:00 2001 From: Bart Kleijngeld Date: Tue, 13 Jun 2017 14:33:08 +0200 Subject: [PATCH 9/9] revised to fix flake8 warnings --- beets/ui/__init__.py | 1 + test/test_importer.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 8b0d57106..d014cc5d0 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -771,6 +771,7 @@ def show_path_changes(path_changes): # Helper functions for option parsing. + def _store_dict(option, opt_str, value, parser): """Custom action callback to parse options which have ``key=value`` pairs as values. All such pairs passed for this option are diff --git a/test/test_importer.py b/test/test_importer.py index cbfbda778..1e0dd1fb9 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -558,7 +558,7 @@ class ImportSingletonTest(_common.TestCase, ImportHelper): self.importer.run() for item in self.lib.items(): - item.load() # TODO: Not sure this is necessary. + item.load() # TODO: Not sure this is necessary. self.assertEqual(item.genre, genre) self.assertEqual(item.collection, collection) # Remove item from library to test again with APPLY choice. @@ -719,7 +719,7 @@ class ImportTest(_common.TestCase, ImportHelper): self.importer.run() for album in self.lib.albums(): - album.load() # TODO: Not sure this is necessary. + album.load() # TODO: Not sure this is necessary. self.assertEqual(album.genre, genre) self.assertEqual(album.collection, collection) # Remove album from library to test again with APPLY choice.