From 1c95b7c46fb74bb367082948dcd4fa3d81b94539 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Mon, 11 Jan 2016 17:39:05 +0100 Subject: [PATCH 1/9] Add musicbrainz id option to importer * Add '-m', '--musicbrainzid' option to the import command, allowing the user to specify a single MusicBrainz ID to be used for the candidate lookup instead of trying to find suitable candidates. * Modify lookup_candidates() of ImportTask and SingletonImportTask to use the musicbrainz id if present. --- beets/importer.py | 15 +++++++++++++-- beets/ui/commands.py | 4 ++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index e0d69b9680..413750ef32 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -581,8 +581,13 @@ def handle_created(self, session): def lookup_candidates(self): """Retrieve and store candidates for this album. """ + # Use a MusicBrainz id directly if provided by the importer -m option. + mb_id = None + if config['import']['musicbrainz_id'].exists(): + mb_id = config['import']['musicbrainz_id'].get() + artist, album, candidates, recommendation = \ - autotag.tag_album(self.items) + autotag.tag_album(self.items, search_id=mb_id) self.cur_artist = artist self.cur_album = album self.candidates = candidates @@ -821,7 +826,13 @@ def _emit_imported(self, lib): plugins.send('item_imported', lib=lib, item=item) def lookup_candidates(self): - candidates, recommendation = autotag.tag_item(self.item) + # Use a MusicBrainz id directly if provided by the importer -m option. + mb_id = None + if config['import']['musicbrainz_id'].exists(): + mb_id = config['import']['musicbrainz_id'].get() + + candidates, recommendation = autotag.tag_item(self.item, + search_id=mb_id) self.candidates = candidates self.rec = recommendation diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 773c10caed..6616cb1bcc 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1022,6 +1022,10 @@ def import_func(lib, opts, args): '--pretend', dest='pretend', action='store_true', help='just print the files to import' ) +import_cmd.parser.add_option( + '-m', '--musicbrainzid', dest='musicbrainz_id', + help='restrict the matching to a single MusicBrainz id' +) import_cmd.func = import_func default_commands.append(import_cmd) From 48c92fbf5a1ddff59594f458364a8333d1e90321 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 19 Jan 2016 21:43:32 +0100 Subject: [PATCH 2/9] Allow multiple MB ids to be passed to the importer * Modify the "--musicbrainzid" argument to the importer so multiple IDs can be specified by the user instead of a single one. * Revise autotag.match.tag_album and autotag.match.tag_item signature to expect a list of IDs (search_ids) instead of a single one (search_id), and add logic for handling and returning multiple matches for those IDs. * Update calls to those functions in other parts of the code. --- beets/autotag/match.py | 42 +++++++++++++++++++++++------------------- beets/importer.py | 16 ++++++++-------- beets/ui/commands.py | 6 +++--- beetsplug/bench.py | 2 +- 4 files changed, 35 insertions(+), 31 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index d4bad9e87f..df5183fc8b 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -370,7 +370,7 @@ def _add_candidate(items, results, info): def tag_album(items, search_artist=None, search_album=None, - search_id=None): + search_ids=[]): """Return a tuple of a artist name, an album name, a list of `AlbumMatch` candidates from the metadata backend, and a `Recommendation`. @@ -397,9 +397,11 @@ def tag_album(items, search_artist=None, search_album=None, candidates = {} # Search by explicit ID. - if search_id is not None: - log.debug(u'Searching for album ID: {0}', search_id) - search_cands = hooks.albums_for_id(search_id) + if search_ids: + search_cands = [] + for search_id in search_ids: + log.debug(u'Searching for album ID: {0}', search_id) + search_cands.extend(hooks.albums_for_id(search_id)) # Use existing metadata or text search. else: @@ -444,33 +446,35 @@ def tag_album(items, search_artist=None, search_album=None, def tag_item(item, search_artist=None, search_title=None, - search_id=None): + search_ids=[]): """Attempts to find metadata for a single track. Returns a `(candidates, recommendation)` pair where `candidates` is a list of TrackMatch objects. `search_artist` and `search_title` may be used to override the current metadata for the purposes of the MusicBrainz - title; likewise `search_id`. + title; likewise `search_ids`. """ # Holds candidates found so far: keys are MBIDs; values are # (distance, TrackInfo) pairs. candidates = {} # First, try matching by MusicBrainz ID. - trackid = search_id or item.mb_trackid - if trackid: - log.debug(u'Searching for track ID: {0}', trackid) - for track_info in hooks.tracks_for_id(trackid): - dist = track_distance(item, track_info, incl_artist=True) - candidates[track_info.track_id] = \ - hooks.TrackMatch(dist, track_info) - # If this is a good match, then don't keep searching. - rec = _recommendation(candidates.values()) - if rec == Recommendation.strong and not config['import']['timid']: - log.debug(u'Track ID match.') - return candidates.values(), rec + trackids = search_ids or filter(None, [item.mb_trackid]) + if trackids: + for trackid in trackids: + log.debug(u'Searching for track ID: {0}', trackid) + for track_info in hooks.tracks_for_id(trackid): + dist = track_distance(item, track_info, incl_artist=True) + candidates[track_info.track_id] = \ + hooks.TrackMatch(dist, track_info) + # If this is a good match, then don't keep searching. + rec = _recommendation(candidates.values()) + if rec == Recommendation.strong and \ + not config['import']['timid']: + log.debug(u'Track ID match.') + return candidates.values(), rec # If we're searching by ID, don't proceed. - if search_id is not None: + if search_ids != []: if candidates: return candidates.values(), rec else: diff --git a/beets/importer.py b/beets/importer.py index 413750ef32..28a32ec7e6 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -582,12 +582,12 @@ def lookup_candidates(self): """Retrieve and store candidates for this album. """ # Use a MusicBrainz id directly if provided by the importer -m option. - mb_id = None - if config['import']['musicbrainz_id'].exists(): - mb_id = config['import']['musicbrainz_id'].get() + mb_ids = [] + if config['import']['musicbrainz_ids'].exists(): + mb_ids = config['import']['musicbrainz_ids'].get() artist, album, candidates, recommendation = \ - autotag.tag_album(self.items, search_id=mb_id) + autotag.tag_album(self.items, search_ids=mb_ids) self.cur_artist = artist self.cur_album = album self.candidates = candidates @@ -827,12 +827,12 @@ def _emit_imported(self, lib): def lookup_candidates(self): # Use a MusicBrainz id directly if provided by the importer -m option. - mb_id = None - if config['import']['musicbrainz_id'].exists(): - mb_id = config['import']['musicbrainz_id'].get() + mb_ids = [] + if config['import']['musicbrainz_ids'].exists(): + mb_ids = config['import']['musicbrainz_ids'].get() candidates, recommendation = autotag.tag_item(self.item, - search_id=mb_id) + search_ids=mb_ids) self.candidates = candidates self.rec = recommendation diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 6616cb1bcc..474b6650f1 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -734,7 +734,7 @@ def choose_match(self, task): search_id = manual_id(False) if search_id: _, _, candidates, rec = autotag.tag_album( - task.items, search_id=search_id + task.items, search_ids=[search_id] ) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. @@ -787,7 +787,7 @@ def choose_item(self, task): search_id = manual_id(True) if search_id: candidates, rec = autotag.tag_item(task.item, - search_id=search_id) + search_ids=[search_id]) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. post_choice = extra_ops[choice](self, task) @@ -1023,7 +1023,7 @@ def import_func(lib, opts, args): help='just print the files to import' ) import_cmd.parser.add_option( - '-m', '--musicbrainzid', dest='musicbrainz_id', + '-m', '--musicbrainzid', dest='musicbrainz_ids', action='append', help='restrict the matching to a single MusicBrainz id' ) import_cmd.func = import_func diff --git a/beetsplug/bench.py b/beetsplug/bench.py index 4d6c6a656d..41f575cd2d 100644 --- a/beetsplug/bench.py +++ b/beetsplug/bench.py @@ -75,7 +75,7 @@ def match_benchmark(lib, prof, query=None, album_id=None): # Run the match. def _run_match(): - match.tag_album(items, search_id=album_id) + match.tag_album(items, search_ids=[album_id]) if prof: cProfile.runctx('_run_match()', {}, {'_run_match': _run_match}, 'match.prof') From 865be11ba1aa8d03b9b32a20472389a521566269 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Tue, 19 Jan 2016 21:51:41 +0100 Subject: [PATCH 3/9] Add tests for importer musicbrainz id argument * Add tests for the "--musicbrainzid" argument (one/several ids for matching an album/singleton; direct test on task.lookup_candidates() for album/singleton). --- test/test_importer.py | 78 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/test/test_importer.py b/test/test_importer.py index b9f75f7ef5..c088492d18 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1688,6 +1688,84 @@ def test_import_pretend_empty(self): .format(displayable_path(self.empty_path))]) +class ImportMusicBrainzIdTest(_common.TestCase, ImportHelper): + """Test the --musicbrainzid argument.""" + # The Beatles - Revolver (14 tracks) + ID_REVOLVER = ('https://musicbrainz.org/release/' + 'd74365b0-288d-3b85-b344-7f9f603378b6') + # The Beatles - Rubber Soul (14 tracks) + ID_RUBBER = ('https://musicbrainz.org/release/' + 'd5d19748-72fc-3f20-aae7-7ca22da283a0') + # The Beatles - Rubber Soul - Michelle + ID_MICHELLE = ('https://musicbrainz.org/recording/' + '45846566-fb79-4758-a006-15cf69e0d138') + # The Beatles - Rubber Soul - Girl + ID_GIRL = ('https://musicbrainz.org/recording/' + '91ec42bc-a04d-497e-b829-439831c8dbcb') + + def setUp(self): + self.setup_beets() + self._create_import_dir(1) + + def tearDown(self): + self.teardown_beets() + + def test_one_mbid_one_album(self): + self.config['import']['musicbrainz_ids'] = [self.ID_REVOLVER] + self._setup_import_session() + self.importer.add_choice(importer.action.APPLY) + self.importer.run() + self.assertEqual(self.lib.albums().get().album, 'Revolver') + + def test_several_mbid_one_album(self): + self.config['import']['musicbrainz_ids'] = [self.ID_REVOLVER, + self.ID_RUBBER] + self._setup_import_session() + self.importer.add_choice(2) + self.importer.add_choice(importer.action.APPLY) + self.importer.run() + self.assertEqual(self.lib.albums().get().album, 'Rubber Soul') + + def test_one_mbid_one_singleton(self): + self.config['import']['musicbrainz_ids'] = [self.ID_MICHELLE] + self._setup_import_session(singletons=True) + self.importer.add_choice(importer.action.APPLY) + self.importer.run() + self.assertEqual(self.lib.items().get().title, 'Michelle') + + def test_several_mbid_one_singleton(self): + self.config['import']['musicbrainz_ids'] = [self.ID_MICHELLE, + self.ID_GIRL] + self._setup_import_session(singletons=True) + self.importer.add_choice(2) + self.importer.add_choice(importer.action.APPLY) + self.importer.run() + self.assertEqual(self.lib.items().get().title, 'Girl') + + def test_candidates_album(self): + """Test directly task.lookup_candidates().""" + self.config['import']['musicbrainz_ids'] = [self.ID_REVOLVER, + self.ID_RUBBER, + 'invalid id'] # discarded + task = importer.ImportTask(paths=self.import_dir, + toppath='top path', + items=[_common.item()]) + task.lookup_candidates() + self.assertEqual(set(['Revolver', 'Rubber Soul']), + set([c.info.album for c in task.candidates])) + + def test_candidates_singleton(self): + """Test directly task.lookup_candidates().""" + self.config['import']['musicbrainz_ids'] = [self.ID_MICHELLE, + self.ID_GIRL, + 'invalid id'] # discarded + task = importer.SingletonImportTask(toppath='top path', + item=_common.item()) + task.lookup_candidates() + self.assertEqual(set(['Michelle', 'Girl']), + set([c.info.title for c in task.candidates])) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 4e5ddac9499dd7a610897c725c2aa8edc3cda7eb Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 20 Jan 2016 17:03:16 +0100 Subject: [PATCH 4/9] Avoid querying MB during ImportMusicBrainzIdTest * Replace the entities used on ImportMusicBrainzIdTest mocking the calls to musicbrainzngs.get_release_by_id and musicbrainzngs.get_recording_by_id instead of querying MusicBrainz. * Other cleanup and docstring fixes. --- test/test_importer.py | 155 +++++++++++++++++++++++++++++++++--------- 1 file changed, 121 insertions(+), 34 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index 880590f4df..4797bb084e 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1691,82 +1691,169 @@ def test_import_pretend_empty(self): class ImportMusicBrainzIdTest(_common.TestCase, ImportHelper): """Test the --musicbrainzid argument.""" - # The Beatles - Revolver (14 tracks) - ID_REVOLVER = ('https://musicbrainz.org/release/' - 'd74365b0-288d-3b85-b344-7f9f603378b6') - # The Beatles - Rubber Soul (14 tracks) - ID_RUBBER = ('https://musicbrainz.org/release/' - 'd5d19748-72fc-3f20-aae7-7ca22da283a0') - # The Beatles - Rubber Soul - Michelle - ID_MICHELLE = ('https://musicbrainz.org/recording/' - '45846566-fb79-4758-a006-15cf69e0d138') - # The Beatles - Rubber Soul - Girl - ID_GIRL = ('https://musicbrainz.org/recording/' - '91ec42bc-a04d-497e-b829-439831c8dbcb') + + MB_RELEASE_PREFIX = 'https://musicbrainz.org/release/' + MB_RECORDING_PREFIX = 'https://musicbrainz.org/recording/' + ID_RELEASE_0 = '00000000-0000-0000-0000-000000000000' + ID_RELEASE_1 = '11111111-1111-1111-1111-111111111111' + ID_RECORDING_0 = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + ID_RECORDING_1 = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb' def setUp(self): self.setup_beets() self._create_import_dir(1) + # Patch calls to musicbrainzngs. + self.release_patcher = patch('musicbrainzngs.get_release_by_id', + side_effect=mocked_get_release_by_id) + self.recording_patcher = patch('musicbrainzngs.get_recording_by_id', + side_effect=mocked_get_recording_by_id) + self.release_patcher.start() + self.recording_patcher.start() + def tearDown(self): + self.recording_patcher.stop() + self.release_patcher.stop() self.teardown_beets() def test_one_mbid_one_album(self): - self.config['import']['musicbrainz_ids'] = [self.ID_REVOLVER] + self.config['import']['musicbrainz_ids'] = \ + [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0] self._setup_import_session() + self.importer.add_choice(importer.action.APPLY) self.importer.run() - self.assertEqual(self.lib.albums().get().album, 'Revolver') + self.assertEqual(self.lib.albums().get().album, 'VALID_RELEASE_0') def test_several_mbid_one_album(self): - self.config['import']['musicbrainz_ids'] = [self.ID_REVOLVER, - self.ID_RUBBER] + self.config['import']['musicbrainz_ids'] = \ + [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, + self.MB_RELEASE_PREFIX + self.ID_RELEASE_1] self._setup_import_session() - self.importer.add_choice(2) + + self.importer.add_choice(2) # Pick the 2nd best match (release 1). self.importer.add_choice(importer.action.APPLY) self.importer.run() - self.assertEqual(self.lib.albums().get().album, 'Rubber Soul') + self.assertEqual(self.lib.albums().get().album, 'VALID_RELEASE_1') def test_one_mbid_one_singleton(self): - self.config['import']['musicbrainz_ids'] = [self.ID_MICHELLE] + self.config['import']['musicbrainz_ids'] = \ + [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0] self._setup_import_session(singletons=True) + self.importer.add_choice(importer.action.APPLY) self.importer.run() - self.assertEqual(self.lib.items().get().title, 'Michelle') + self.assertEqual(self.lib.items().get().title, 'VALID_RECORDING_0') def test_several_mbid_one_singleton(self): - self.config['import']['musicbrainz_ids'] = [self.ID_MICHELLE, - self.ID_GIRL] + self.config['import']['musicbrainz_ids'] = \ + [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, + self.MB_RECORDING_PREFIX + self.ID_RECORDING_1] self._setup_import_session(singletons=True) - self.importer.add_choice(2) + + self.importer.add_choice(2) # Pick the 2nd best match (recording 1). self.importer.add_choice(importer.action.APPLY) self.importer.run() - self.assertEqual(self.lib.items().get().title, 'Girl') + self.assertEqual(self.lib.items().get().title, 'VALID_RECORDING_1') def test_candidates_album(self): - """Test directly task.lookup_candidates().""" - self.config['import']['musicbrainz_ids'] = [self.ID_REVOLVER, - self.ID_RUBBER, - 'invalid id'] # discarded + """Test directly ImportTask.lookup_candidates().""" + self.config['import']['musicbrainz_ids'] = \ + [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, + self.MB_RELEASE_PREFIX + self.ID_RELEASE_1, + 'an invalid and discarded id'] + task = importer.ImportTask(paths=self.import_dir, toppath='top path', items=[_common.item()]) task.lookup_candidates() - self.assertEqual(set(['Revolver', 'Rubber Soul']), + self.assertEqual(set(['VALID_RELEASE_0', 'VALID_RELEASE_1']), set([c.info.album for c in task.candidates])) def test_candidates_singleton(self): - """Test directly task.lookup_candidates().""" - self.config['import']['musicbrainz_ids'] = [self.ID_MICHELLE, - self.ID_GIRL, - 'invalid id'] # discarded + """Test directly SingletonImportTask.lookup_candidates().""" + self.config['import']['musicbrainz_ids'] = \ + [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, + self.MB_RECORDING_PREFIX + self.ID_RECORDING_1, + 'an invalid and discarded id'] + task = importer.SingletonImportTask(toppath='top path', item=_common.item()) task.lookup_candidates() - self.assertEqual(set(['Michelle', 'Girl']), + self.assertEqual(set(['VALID_RECORDING_0', 'VALID_RECORDING_1']), set([c.info.title for c in task.candidates])) +# Helpers for ImportMusicBrainzIdTest. + + +def mocked_get_release_by_id(id_, includes=[], release_status=[], + release_type=[]): + """Mimic musicbrainzngs.get_release_by_id, accepting only a restricted list + of MB ids (ID_RELEASE_0, ID_RELEASE_1). The returned dict differs only in + the release title and artist name, so that ID_RELEASE_0 is a closer match + to the items created by ImportHelper._create_import_dir().""" + # Map IDs to (release title, artist), so the distances are different. + releases = {ImportMusicBrainzIdTest.ID_RELEASE_0: ('VALID_RELEASE_0', + 'TAG ARTIST'), + ImportMusicBrainzIdTest.ID_RELEASE_1: ('VALID_RELEASE_1', + 'DISTANT_MATCH')} + + return { + 'release': { + 'title': releases[id_][0], + 'id': id_, + 'medium-list': [{ + 'track-list': [{ + 'recording': { + 'title': 'foo', + 'id': 'bar', + 'length': 59, + }, + 'position': 9, + }], + 'position': 5, + }], + 'artist-credit': [{ + 'artist': { + 'name': releases[id_][1], + 'id': 'some-id', + }, + }], + 'release-group': { + 'id': 'another-id', + } + } + } + + +def mocked_get_recording_by_id(id_, includes=[], release_status=[], + release_type=[]): + """Mimic musicbrainzngs.get_recording_by_id, accepting only a restricted + list of MB ids (ID_RECORDING_0, ID_RECORDING_1). The returned dict differs + only in the recording title and artist name, so that ID_RECORDING_0 is a + closer match to the items created by ImportHelper._create_import_dir().""" + # Map IDs to (recording title, artist), so the distances are different. + releases = {ImportMusicBrainzIdTest.ID_RECORDING_0: ('VALID_RECORDING_0', + 'TAG ARTIST'), + ImportMusicBrainzIdTest.ID_RECORDING_1: ('VALID_RECORDING_1', + 'DISTANT_MATCH')} + + return { + 'recording': { + 'title': releases[id_][0], + 'id': id_, + 'length': 59, + }, + 'artist-credit': [{ + 'artist': { + 'name': releases[id_][1], + 'id': 'some-id', + }, + }], + } + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 39cf4651b8d2bcb4879af6d527acd9815bb9aebb Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Wed, 20 Jan 2016 20:22:48 +0100 Subject: [PATCH 5/9] Fix singleton candidate ordering when using MB id * Fix an issue that caused the candidates for a singleton not to be returned ordered by distance from autotag.match.tag_item(), when searching multiple MusicBrainz ids (ie. several "--musicbrainzid" arguments). The candidates are now explicitely reordered before being returned and before the recommendation is computed. * Fix test_importer.mocked_get_recording_by_id so that the artist is nested properly (and as a result, taken into account into the distance calculations). --- beets/autotag/match.py | 6 +++--- test/test_importer.py | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index df5183fc8b..d95dc7a08e 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -467,16 +467,16 @@ def tag_item(item, search_artist=None, search_title=None, candidates[track_info.track_id] = \ hooks.TrackMatch(dist, track_info) # If this is a good match, then don't keep searching. - rec = _recommendation(candidates.values()) + rec = _recommendation(sorted(candidates.itervalues())) if rec == Recommendation.strong and \ not config['import']['timid']: log.debug(u'Track ID match.') - return candidates.values(), rec + return sorted(candidates.itervalues()), rec # If we're searching by ID, don't proceed. if search_ids != []: if candidates: - return candidates.values(), rec + return sorted(candidates.itervalues()), rec else: return [], Recommendation.none diff --git a/test/test_importer.py b/test/test_importer.py index 4797bb084e..295f1a701b 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1844,13 +1844,13 @@ def mocked_get_recording_by_id(id_, includes=[], release_status=[], 'title': releases[id_][0], 'id': id_, 'length': 59, - }, - 'artist-credit': [{ - 'artist': { - 'name': releases[id_][1], - 'id': 'some-id', - }, - }], + 'artist-credit': [{ + 'artist': { + 'name': releases[id_][1], + 'id': 'some-id', + }, + }], + } } From b5262274827da83d3d39a4d23a09d3ebc69b63c7 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Thu, 21 Jan 2016 17:08:53 +0100 Subject: [PATCH 6/9] Cleanup and documentation for MB_id importer arg * Style cleanup and fixes for the "--musicbrainzid" import argument. * Allow the input of several IDs (separated by spaces) on the "enter Id" importer prompt. * Add basic documentation. --- beets/autotag/match.py | 12 ++++++++---- beets/config_default.yaml | 1 + beets/importer.py | 8 ++------ beets/ui/commands.py | 6 +++--- docs/guides/tagger.rst | 9 +++++++-- docs/reference/cli.rst | 5 +++++ 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index d95dc7a08e..8baaf01081 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -380,8 +380,11 @@ def tag_album(items, search_artist=None, search_album=None, The `AlbumMatch` objects are generated by searching the metadata backends. By default, the metadata of the items is used for the - search. This can be customized by setting the parameters. The - `mapping` field of the album has the matched `items` as keys. + search. This can be customized by setting the parameters. + `search_ids` is a list of MusicBrainz release IDs: if specified, + it will restrict the candidates to those IDs, ignoring + `search_artist` and `search album`. The `mapping` field of the + album has the matched `items` as keys. The recommendation is calculated from the match quality of the candidates. @@ -451,7 +454,8 @@ def tag_item(item, search_artist=None, search_title=None, `(candidates, recommendation)` pair where `candidates` is a list of TrackMatch objects. `search_artist` and `search_title` may be used to override the current metadata for the purposes of the MusicBrainz - title; likewise `search_ids`. + title. `search_ids` may be used for restricting the search to a list + of MusicBrainz IDs. """ # Holds candidates found so far: keys are MBIDs; values are # (distance, TrackInfo) pairs. @@ -474,7 +478,7 @@ def tag_item(item, search_artist=None, search_title=None, return sorted(candidates.itervalues()), rec # If we're searching by ID, don't proceed. - if search_ids != []: + if search_ids: if candidates: return sorted(candidates.itervalues()), rec else: diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 545fa96385..3fdfaf836b 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -22,6 +22,7 @@ import: flat: no group_albums: no pretend: false + musicbrainz_ids: [] clutter: ["Thumbs.DB", ".DS_Store"] ignore: [".*", "*~", "System Volume Information"] diff --git a/beets/importer.py b/beets/importer.py index 28a32ec7e6..519eecc7e1 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -582,9 +582,7 @@ def lookup_candidates(self): """Retrieve and store candidates for this album. """ # Use a MusicBrainz id directly if provided by the importer -m option. - mb_ids = [] - if config['import']['musicbrainz_ids'].exists(): - mb_ids = config['import']['musicbrainz_ids'].get() + mb_ids = config['import']['musicbrainz_ids'].as_str_seq() artist, album, candidates, recommendation = \ autotag.tag_album(self.items, search_ids=mb_ids) @@ -827,9 +825,7 @@ def _emit_imported(self, lib): def lookup_candidates(self): # Use a MusicBrainz id directly if provided by the importer -m option. - mb_ids = [] - if config['import']['musicbrainz_ids'].exists(): - mb_ids = config['import']['musicbrainz_ids'].get() + mb_ids = config['import']['musicbrainz_ids'].as_str_seq() candidates, recommendation = autotag.tag_item(self.item, search_ids=mb_ids) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 74a6da0ed0..c68dcf10fd 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -734,7 +734,7 @@ def choose_match(self, task): search_id = manual_id(False) if search_id: _, _, candidates, rec = autotag.tag_album( - task.items, search_ids=[search_id] + task.items, search_ids=search_id.split(' ') ) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. @@ -786,8 +786,8 @@ def choose_item(self, task): # Ask for a track ID. search_id = manual_id(True) if search_id: - candidates, rec = autotag.tag_item(task.item, - search_ids=[search_id]) + candidates, rec = autotag.tag_item( + task.item, search_ids=search_id.split(' ')) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. post_choice = extra_ops[choice](self, task) diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index 4a4c561848..95975c038e 100644 --- a/docs/guides/tagger.rst +++ b/docs/guides/tagger.rst @@ -160,10 +160,10 @@ When beets needs your input about a match, it says something like this:: Beirut - Lon Gisland (Similarity: 94.4%) * Scenic World (Second Version) -> Scenic World - [A]pply, More candidates, Skip, Use as-is, as Tracks, Enter search, or aBort? + [A]pply, More candidates, Skip, Use as-is, as Tracks, Enter search, enter Id, or aBort? When beets asks you this question, it wants you to enter one of the capital -letters: A, M, S, U, T, G, E, or B. That is, you can choose one of the +letters: A, M, S, U, T, G, E, I or B. That is, you can choose one of the following: * *A*: Apply the suggested changes shown and move on. @@ -190,6 +190,11 @@ following: option if beets hasn't found any good options because the album is mistagged or untagged. +* *I*: Enter a MusicBrainz id to use as search in the database. Use this option + to specify a MusicBrainz entity (release or recording) directly, by pasting + its ID or the full URL. You can also specify several IDs by separating them + by a space. + * *B*: Cancel this import task altogether. No further albums will be tagged; beets shuts down immediately. The next time you attempt to import the same directory, though, beets will ask you if you want to resume tagging where you diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index c072c0c0f5..25b3fcbc13 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -132,6 +132,11 @@ Optional command flags: option. If set, beets will just print a list of files that it would otherwise import. +* If you already have a MusicBrainz ID that matches the items to be imported, + you can instruct beets to restrict the search to that ID instead of searching + for other candidates by using the ``--musicbrainzid MB_ID`` option. Multiple + IDs can be specified by simply repeating the option several times. + .. _rarfile: https://pypi.python.org/pypi/rarfile/2.2 .. only:: html From 4eedd2bd8dbf9073eddfb94ed593dba12981df07 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Thu, 21 Jan 2016 20:33:54 +0100 Subject: [PATCH 7/9] Store user-supplied MB ids on the Tasks * Store the user-supplied MusicBrainz IDs (via the "--musicbrainzid" importer argument) on ImporTask.task.musicbrainz_ids during the lookup_candidates() pipeline stage. * Update test cases to reflect the changes. --- beets/importer.py | 22 ++++++++++++---------- test/test_importer.py | 18 ++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 519eecc7e1..fb7195bc61 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -434,6 +434,7 @@ def __init__(self, toppath, paths, items): self.rec = None self.should_remove_duplicates = False self.is_album = True + self.musicbrainz_ids = [] # user-supplied candidate IDs. def set_choice(self, choice): """Given an AlbumMatch or TrackMatch object or an action constant, @@ -579,13 +580,12 @@ def handle_created(self, session): return tasks def lookup_candidates(self): - """Retrieve and store candidates for this album. + """Retrieve and store candidates for this album. User-specified + candidate IDs are stored in self.musicbrainz_ids: if present, the + initial lookup is restricted to only those IDs. """ - # Use a MusicBrainz id directly if provided by the importer -m option. - mb_ids = config['import']['musicbrainz_ids'].as_str_seq() - artist, album, candidates, recommendation = \ - autotag.tag_album(self.items, search_ids=mb_ids) + autotag.tag_album(self.items, search_ids=self.musicbrainz_ids) self.cur_artist = artist self.cur_album = album self.candidates = candidates @@ -824,11 +824,8 @@ def _emit_imported(self, lib): plugins.send('item_imported', lib=lib, item=item) def lookup_candidates(self): - # Use a MusicBrainz id directly if provided by the importer -m option. - mb_ids = config['import']['musicbrainz_ids'].as_str_seq() - - candidates, recommendation = autotag.tag_item(self.item, - search_ids=mb_ids) + candidates, recommendation = autotag.tag_item( + self.item, search_ids=self.musicbrainz_ids) self.candidates = candidates self.rec = recommendation @@ -1253,6 +1250,11 @@ def lookup_candidates(session, task): plugins.send('import_task_start', session=session, task=task) log.debug(u'Looking up: {0}', displayable_path(task.paths)) + + # Restrict the initial lookup to IDs specified by the user via the -m + # option. Currently all the IDs are passed onto the tasks directly. + task.musicbrainz_ids = session.config['musicbrainz_ids'].as_str_seq() + task.lookup_candidates() diff --git a/test/test_importer.py b/test/test_importer.py index 295f1a701b..21c11bc6e9 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1758,27 +1758,25 @@ def test_several_mbid_one_singleton(self): def test_candidates_album(self): """Test directly ImportTask.lookup_candidates().""" - self.config['import']['musicbrainz_ids'] = \ - [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, - self.MB_RELEASE_PREFIX + self.ID_RELEASE_1, - 'an invalid and discarded id'] - task = importer.ImportTask(paths=self.import_dir, toppath='top path', items=[_common.item()]) + task.musicbrainz_ids = [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, + self.MB_RELEASE_PREFIX + self.ID_RELEASE_1, + 'an invalid and discarded id'] + task.lookup_candidates() self.assertEqual(set(['VALID_RELEASE_0', 'VALID_RELEASE_1']), set([c.info.album for c in task.candidates])) def test_candidates_singleton(self): """Test directly SingletonImportTask.lookup_candidates().""" - self.config['import']['musicbrainz_ids'] = \ - [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, - self.MB_RECORDING_PREFIX + self.ID_RECORDING_1, - 'an invalid and discarded id'] - task = importer.SingletonImportTask(toppath='top path', item=_common.item()) + task.musicbrainz_ids = [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, + self.MB_RECORDING_PREFIX + self.ID_RECORDING_1, + 'an invalid and discarded id'] + task.lookup_candidates() self.assertEqual(set(['VALID_RECORDING_0', 'VALID_RECORDING_1']), set([c.info.title for c in task.candidates])) From 79d84c0e4f2325c4102dd1febacf642b811b05a4 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 22 Jan 2016 16:31:00 +0100 Subject: [PATCH 8/9] Style and doc fixes for MB_id importer argument * Rename the importer argument and related variables to make it more generic, as the feature should be independent of the backend used and not restricted to MusicBrainz. * Update documentation and docstrings accordingly. * Add changelog entry. --- beets/autotag/match.py | 4 ++-- beets/config_default.yaml | 2 +- beets/importer.py | 10 +++++----- beets/ui/commands.py | 9 +++++---- docs/changelog.rst | 4 ++++ docs/guides/tagger.rst | 12 ++++++------ docs/reference/cli.rst | 8 ++++---- test/test_importer.py | 20 ++++++++++---------- 8 files changed, 37 insertions(+), 32 deletions(-) diff --git a/beets/autotag/match.py b/beets/autotag/match.py index 8baaf01081..3565dba3fe 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -381,7 +381,7 @@ def tag_album(items, search_artist=None, search_album=None, The `AlbumMatch` objects are generated by searching the metadata backends. By default, the metadata of the items is used for the search. This can be customized by setting the parameters. - `search_ids` is a list of MusicBrainz release IDs: if specified, + `search_ids` is a list of metadata backend IDs: if specified, it will restrict the candidates to those IDs, ignoring `search_artist` and `search album`. The `mapping` field of the album has the matched `items` as keys. @@ -455,7 +455,7 @@ def tag_item(item, search_artist=None, search_title=None, TrackMatch objects. `search_artist` and `search_title` may be used to override the current metadata for the purposes of the MusicBrainz title. `search_ids` may be used for restricting the search to a list - of MusicBrainz IDs. + of metadata backend IDs. """ # Holds candidates found so far: keys are MBIDs; values are # (distance, TrackInfo) pairs. diff --git a/beets/config_default.yaml b/beets/config_default.yaml index 3fdfaf836b..5bc9211676 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -22,7 +22,7 @@ import: flat: no group_albums: no pretend: false - musicbrainz_ids: [] + search_ids: [] clutter: ["Thumbs.DB", ".DS_Store"] ignore: [".*", "*~", "System Volume Information"] diff --git a/beets/importer.py b/beets/importer.py index fb7195bc61..868ac69227 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -434,7 +434,7 @@ def __init__(self, toppath, paths, items): self.rec = None self.should_remove_duplicates = False self.is_album = True - self.musicbrainz_ids = [] # user-supplied candidate IDs. + self.search_ids = [] # user-supplied candidate IDs. def set_choice(self, choice): """Given an AlbumMatch or TrackMatch object or an action constant, @@ -581,11 +581,11 @@ def handle_created(self, session): def lookup_candidates(self): """Retrieve and store candidates for this album. User-specified - candidate IDs are stored in self.musicbrainz_ids: if present, the + candidate IDs are stored in self.search_ids: if present, the initial lookup is restricted to only those IDs. """ artist, album, candidates, recommendation = \ - autotag.tag_album(self.items, search_ids=self.musicbrainz_ids) + autotag.tag_album(self.items, search_ids=self.search_ids) self.cur_artist = artist self.cur_album = album self.candidates = candidates @@ -825,7 +825,7 @@ def _emit_imported(self, lib): def lookup_candidates(self): candidates, recommendation = autotag.tag_item( - self.item, search_ids=self.musicbrainz_ids) + self.item, search_ids=self.search_ids) self.candidates = candidates self.rec = recommendation @@ -1253,7 +1253,7 @@ def lookup_candidates(session, task): # Restrict the initial lookup to IDs specified by the user via the -m # option. Currently all the IDs are passed onto the tasks directly. - task.musicbrainz_ids = session.config['musicbrainz_ids'].as_str_seq() + task.search_ids = session.config['search_ids'].as_str_seq() task.lookup_candidates() diff --git a/beets/ui/commands.py b/beets/ui/commands.py index c68dcf10fd..3e3acb7c3a 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -734,7 +734,7 @@ def choose_match(self, task): search_id = manual_id(False) if search_id: _, _, candidates, rec = autotag.tag_album( - task.items, search_ids=search_id.split(' ') + task.items, search_ids=search_id.split() ) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. @@ -787,7 +787,7 @@ def choose_item(self, task): search_id = manual_id(True) if search_id: candidates, rec = autotag.tag_item( - task.item, search_ids=search_id.split(' ')) + task.item, search_ids=search_id.split()) elif choice in extra_ops.keys(): # Allow extra ops to automatically set the post-choice. post_choice = extra_ops[choice](self, task) @@ -1023,8 +1023,9 @@ def import_func(lib, opts, args): help='just print the files to import' ) import_cmd.parser.add_option( - '-m', '--musicbrainzid', dest='musicbrainz_ids', action='append', - help='restrict the matching to a single MusicBrainz id' + '-S', '--search-id', dest='search_ids', action='append', + metavar='BACKEND_ID', + help='restrict matching to a specific metadata backend ID' ) import_cmd.func = import_func default_commands.append(import_cmd) diff --git a/docs/changelog.rst b/docs/changelog.rst index 51fab31008..b6c8bfb25f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -20,6 +20,10 @@ New: session. :bug:`1779` * :doc:`/plugins/info`: A new option will print only fields' names and not their values. Thanks to :user:`GuilhermeHideki`. :bug:`1812` +* A new ``--search-id`` importer option lets you specify one or several + matching MusicBrainz/Discogs IDs directly, bypassing the default initial + candidate search. Also, the ``enter Id`` prompt choice now accepts several + IDs, separated by spaces. :bug:`1808` .. _AcousticBrainz: http://acousticbrainz.org/ diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index 95975c038e..0b21644bf1 100644 --- a/docs/guides/tagger.rst +++ b/docs/guides/tagger.rst @@ -60,8 +60,8 @@ all of these limitations. because beets by default infers tags based on existing metadata. But this is not a hard and fast rule---there are a few ways to tag metadata-poor music: - * You can use the *E* option described below to search in MusicBrainz for - a specific album or song. + * You can use the *E* or *I* options described below to search in + MusicBrainz for a specific album or song. * The :doc:`Acoustid plugin ` extends the autotagger to use acoustic fingerprinting to find information for arbitrary audio. Install that plugin if you're willing to spend a little more CPU power @@ -190,10 +190,10 @@ following: option if beets hasn't found any good options because the album is mistagged or untagged. -* *I*: Enter a MusicBrainz id to use as search in the database. Use this option - to specify a MusicBrainz entity (release or recording) directly, by pasting - its ID or the full URL. You can also specify several IDs by separating them - by a space. +* *I*: Enter a metadata backend ID to use as search in the database. Use this + option to specify a backend entity (for example, a MusicBrainz release or + recording) directly, by pasting its ID or the full URL. You can also specify + several IDs by separating them by a space. * *B*: Cancel this import task altogether. No further albums will be tagged; beets shuts down immediately. The next time you attempt to import the same diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 25b3fcbc13..2ded2c48de 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -132,10 +132,10 @@ Optional command flags: option. If set, beets will just print a list of files that it would otherwise import. -* If you already have a MusicBrainz ID that matches the items to be imported, - you can instruct beets to restrict the search to that ID instead of searching - for other candidates by using the ``--musicbrainzid MB_ID`` option. Multiple - IDs can be specified by simply repeating the option several times. +* If you already have a metadata backend ID that matches the items to be + imported, you can instruct beets to restrict the search to that ID instead of + searching for other candidates by using the ``--search_id SEARCH_ID`` option. + Multiple IDs can be specified by simply repeating the option several times. .. _rarfile: https://pypi.python.org/pypi/rarfile/2.2 diff --git a/test/test_importer.py b/test/test_importer.py index 21c11bc6e9..56f4a17a53 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1717,7 +1717,7 @@ def tearDown(self): self.teardown_beets() def test_one_mbid_one_album(self): - self.config['import']['musicbrainz_ids'] = \ + self.config['import']['search_ids'] = \ [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0] self._setup_import_session() @@ -1726,7 +1726,7 @@ def test_one_mbid_one_album(self): self.assertEqual(self.lib.albums().get().album, 'VALID_RELEASE_0') def test_several_mbid_one_album(self): - self.config['import']['musicbrainz_ids'] = \ + self.config['import']['search_ids'] = \ [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, self.MB_RELEASE_PREFIX + self.ID_RELEASE_1] self._setup_import_session() @@ -1737,7 +1737,7 @@ def test_several_mbid_one_album(self): self.assertEqual(self.lib.albums().get().album, 'VALID_RELEASE_1') def test_one_mbid_one_singleton(self): - self.config['import']['musicbrainz_ids'] = \ + self.config['import']['search_ids'] = \ [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0] self._setup_import_session(singletons=True) @@ -1746,7 +1746,7 @@ def test_one_mbid_one_singleton(self): self.assertEqual(self.lib.items().get().title, 'VALID_RECORDING_0') def test_several_mbid_one_singleton(self): - self.config['import']['musicbrainz_ids'] = \ + self.config['import']['search_ids'] = \ [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, self.MB_RECORDING_PREFIX + self.ID_RECORDING_1] self._setup_import_session(singletons=True) @@ -1761,9 +1761,9 @@ def test_candidates_album(self): task = importer.ImportTask(paths=self.import_dir, toppath='top path', items=[_common.item()]) - task.musicbrainz_ids = [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, - self.MB_RELEASE_PREFIX + self.ID_RELEASE_1, - 'an invalid and discarded id'] + task.search_ids = [self.MB_RELEASE_PREFIX + self.ID_RELEASE_0, + self.MB_RELEASE_PREFIX + self.ID_RELEASE_1, + 'an invalid and discarded id'] task.lookup_candidates() self.assertEqual(set(['VALID_RELEASE_0', 'VALID_RELEASE_1']), @@ -1773,9 +1773,9 @@ def test_candidates_singleton(self): """Test directly SingletonImportTask.lookup_candidates().""" task = importer.SingletonImportTask(toppath='top path', item=_common.item()) - task.musicbrainz_ids = [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, - self.MB_RECORDING_PREFIX + self.ID_RECORDING_1, - 'an invalid and discarded id'] + task.search_ids = [self.MB_RECORDING_PREFIX + self.ID_RECORDING_0, + self.MB_RECORDING_PREFIX + self.ID_RECORDING_1, + 'an invalid and discarded id'] task.lookup_candidates() self.assertEqual(set(['VALID_RECORDING_0', 'VALID_RECORDING_1']), From 4f51302d09cfaab99ceaec020f6672ebd7a991a4 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Fri, 22 Jan 2016 19:48:22 +0100 Subject: [PATCH 9/9] Fix typo on importer search by id documentation --- docs/reference/cli.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index 2ded2c48de..8c04b6c908 100644 --- a/docs/reference/cli.rst +++ b/docs/reference/cli.rst @@ -134,7 +134,7 @@ Optional command flags: * If you already have a metadata backend ID that matches the items to be imported, you can instruct beets to restrict the search to that ID instead of - searching for other candidates by using the ``--search_id SEARCH_ID`` option. + searching for other candidates by using the ``--search-id SEARCH_ID`` option. Multiple IDs can be specified by simply repeating the option several times. .. _rarfile: https://pypi.python.org/pypi/rarfile/2.2