diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py index d808028bf..e774e18eb 100644 --- a/beets/autotag/__init__.py +++ b/beets/autotag/__init__.py @@ -1,5 +1,5 @@ # This file is part of beets. -# Copyright 2011, Adrian Sampson. +# Copyright 2012, Adrian Sampson. # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -22,7 +22,7 @@ from beets import library, mediafile from beets.util import sorted_walk, ancestry # Parts of external interface. -from .hooks import AlbumInfo, TrackInfo +from .hooks import AlbumInfo, TrackInfo, AlbumMatch, TrackMatch from .match import AutotagError from .match import tag_item, tag_album from .match import RECOMMEND_STRONG, RECOMMEND_MEDIUM, RECOMMEND_NONE @@ -115,12 +115,12 @@ def apply_item_metadata(item, track_info): # At the moment, the other metadata is left intact (including album # and track number). Perhaps these should be emptied? -def apply_metadata(items, album_info, per_disc_numbering=False): - """Set the items' metadata to match an AlbumInfo object. The list of - items must be ordered. If `per_disc_numbering`, then the track - numbers are per-disc instead of per-release. +def apply_metadata(album_info, mapping, per_disc_numbering=False): + """Set the items' metadata to match an AlbumInfo object using a + mapping from Items to TrackInfo objects. If `per_disc_numbering`, + then the track numbers are per-disc instead of per-release. """ - for item, track_info in zip(items, album_info.tracks): + for item, track_info in mapping.iteritems(): # Album, artist, track count. if not item: continue @@ -130,7 +130,7 @@ def apply_metadata(items, album_info, per_disc_numbering=False): item.artist = album_info.artist item.albumartist = album_info.artist item.album = album_info.album - item.tracktotal = len(items) + item.tracktotal = len(album_info.tracks) # Artist sort and credit names. item.artist_sort = track_info.artist_sort or album_info.artist_sort diff --git a/beets/autotag/match.py b/beets/autotag/match.py index ae5be88a4..8ec6f966d 100644 --- a/beets/autotag/match.py +++ b/beets/autotag/match.py @@ -1,5 +1,5 @@ # This file is part of beets. -# Copyright 2011, Adrian Sampson. +# Copyright 2012, Adrian Sampson. # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -35,6 +35,8 @@ ALBUM_WEIGHT = 3.0 TRACK_WEIGHT = 1.0 # The weight of a missing track. MISSING_WEIGHT = 0.9 +# The weight of an extra (umatched) track. +UNMATCHED_WEIGHT = 0.6 # These distances are components of the track distance (that is, they # compete against each other but not ARTIST_WEIGHT and ALBUM_WEIGHT; # the overall TRACK_WEIGHT does that). @@ -248,9 +250,14 @@ def track_distance(item, track_info, incl_artist=False): return dist / dist_max -def distance(items, album_info): +def distance(items, album_info, mapping): """Determines how "significant" an album metadata change would be. - Returns a float in [0.0,1.0]. The list of items must be ordered. + Returns a float in [0.0,1.0]. `album_info` is an AlbumInfo object + reflecting the album to be compared. `items` is a sequence of all + Item objects that will be matched (order is not important). + `mapping` is a dictionary mapping Items to TrackInfo objects; the + keys are a subset of `items` and the values are a subset of + `album_info.tracks`. """ cur_artist, cur_album, _ = current_metadata(items) cur_artist = cur_artist or '' @@ -268,15 +275,18 @@ def distance(items, album_info): dist += string_dist(cur_album, album_info.album) * ALBUM_WEIGHT dist_max += ALBUM_WEIGHT - # Track distances. - for i, (item, track_info) in enumerate(zip(items, album_info.tracks)): - if item: - dist += track_distance(item, track_info, album_info.va) * \ - TRACK_WEIGHT - dist_max += TRACK_WEIGHT - else: - dist += MISSING_WEIGHT - dist_max += MISSING_WEIGHT + # Matched track distances. + for item, track in mapping.iteritems(): + dist += track_distance(item, track, album_info.va) * TRACK_WEIGHT + dist_max += TRACK_WEIGHT + + # Extra and unmatched tracks. + for track in set(album_info.tracks) - set(mapping.values()): + dist += MISSING_WEIGHT + dist_max += MISSING_WEIGHT + for item in set(items) - set(mapping.keys()): + dist += UNMATCHED_WEIGHT + dist_max += UNMATCHED_WEIGHT # Plugin distances. plugin_d, plugin_dm = plugins.album_distance(items, album_info) @@ -287,11 +297,12 @@ def distance(items, album_info): if dist_max == 0.0: return 0.0 else: - return dist/dist_max + return dist / dist_max def match_by_id(items): """If the items are tagged with a MusicBrainz album ID, returns an - info dict for the corresponding album. Otherwise, returns None. + AlbumInfo object for the corresponding album. Otherwise, returns + None. """ # Is there a consensus on the MB album ID? albumids = [item.mb_albumid for item in items if item.mb_albumid] @@ -313,15 +324,15 @@ def match_by_id(items): # present, but that event seems very unlikely. def recommendation(results): - """Given a sorted list of result tuples, returns a recommendation - flag (RECOMMEND_STRONG, RECOMMEND_MEDIUM, RECOMMEND_NONE) based - on the results' distances. + """Given a sorted list of AlbumMatch or TrackMatch objects, return a + recommendation flag (RECOMMEND_STRONG, RECOMMEND_MEDIUM, + RECOMMEND_NONE) based on the results' distances. """ if not results: # No candidates: no recommendation. rec = RECOMMEND_NONE else: - min_dist = results[0][0] + min_dist = results[0].distance if min_dist < STRONG_REC_THRESH: # Strong recommendation level. rec = RECOMMEND_STRONG @@ -331,7 +342,7 @@ def recommendation(results): elif min_dist <= MEDIUM_REC_THRESH: # Medium recommendation level. rec = RECOMMEND_MEDIUM - elif results[1][0] - min_dist >= REC_GAP_THRESH: + elif results[1].distance - min_dist >= REC_GAP_THRESH: # Gap between first two candidates is large. rec = RECOMMEND_MEDIUM else: @@ -339,11 +350,11 @@ def recommendation(results): rec = RECOMMEND_NONE return rec -def validate_candidate(items, results, info): +def _add_candidate(items, results, info): """Given a candidate AlbumInfo object, attempt to add the candidate - to the output dictionary of result tuples. This involves checking - the track count, ordering the items, checking for duplicates, and - calculating the distance. + to the output dictionary of AlbumMatch objects. This involves + checking the track count, ordering the items, checking for + duplicates, and calculating the distance. """ log.debug('Candidate: %s - %s' % (info.artist, info.album)) @@ -352,24 +363,15 @@ def validate_candidate(items, results, info): log.debug('Duplicate.') return - # Make sure the album has the correct number of tracks. - if len(items) > len(info.tracks): - log.debug('Too many items to match: %i > %i.' % - (len(items), len(info.tracks))) - return - - # Put items in order. + # Find mapping between the items and the track info. mapping, extra_items, extra_tracks = assign_items(items, info.tracks) - # TEMPORARY: make ordered item list with gaps. - ordered = [None] * len(info.tracks) - for item, track_info in mapping.iteritems(): - ordered[track_info.index - 1] = item # Get the change distance. - dist = distance(ordered, info) + dist = distance(items, info, mapping) log.debug('Success. Distance: %f' % dist) - results[info.album_id] = dist, ordered, info + results[info.album_id] = hooks.AlbumMatch(dist, info, mapping, + extra_items, extra_tracks) def tag_album(items, timid=False, search_artist=None, search_album=None, search_id=None): @@ -377,10 +379,8 @@ def tag_album(items, timid=False, search_artist=None, search_album=None, set of items comprised by an album. Returns everything relevant: - The current artist. - The current album. - - A list of (distance, items, info) tuples where info is a - dictionary containing the inferred tags and items is a - reordered version of the input items list. The candidates are - sorted by distance (i.e., best match first). + - A list of AlbumMatch objects. The candidates are sorted by + distance (i.e., best match first). - A recommendation, one of RECOMMEND_STRONG, RECOMMEND_MEDIUM, or RECOMMEND_NONE; indicating that the first candidate is very likely, it is somewhat likely, or no conclusion could @@ -404,7 +404,7 @@ def tag_album(items, timid=False, search_artist=None, search_album=None, else: id_info = match_by_id(items) if id_info: - validate_candidate(items, candidates, id_info) + _add_candidate(items, candidates, id_info) rec = recommendation(candidates.values()) log.debug('Album ID match recommendation is ' + str(rec)) if candidates and not timid: @@ -439,7 +439,7 @@ def tag_album(items, timid=False, search_artist=None, search_album=None, va_likely) log.debug(u'Evaluating %i candidates.' % len(search_cands)) for info in search_cands: - validate_candidate(items, candidates, info) + _add_candidate(items, candidates, info) # Sort and get the recommendation. candidates = sorted(candidates.itervalues()) @@ -449,10 +449,10 @@ def tag_album(items, timid=False, search_artist=None, search_album=None, def tag_item(item, timid=False, search_artist=None, search_title=None, search_id=None): """Attempts to find metadata for a single track. Returns a - `(candidates, recommendation)` pair where `candidates` is a list - of `(distance, track_info)` pairs. `search_artist` and - `search_title` may be used to override the current metadata for - the purposes of the MusicBrainz title; likewise `search_id`. + `(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`. """ # Holds candidates found so far: keys are MBIDs; values are # (distance, TrackInfo) pairs. @@ -465,7 +465,8 @@ def tag_item(item, timid=False, search_artist=None, search_title=None, track_info = hooks._track_for_id(trackid) if track_info: dist = track_distance(item, track_info, incl_artist=True) - candidates[track_info.track_id] = (dist, track_info) + 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 == RECOMMEND_STRONG and not timid: @@ -487,7 +488,7 @@ def tag_item(item, timid=False, search_artist=None, search_title=None, # Get and evaluate candidate metadata. for track_info in hooks._item_candidates(item, search_artist, search_title): dist = track_distance(item, track_info, incl_artist=True) - candidates[track_info.track_id] = (dist, track_info) + candidates[track_info.track_id] = hooks.TrackMatch(dist, track_info) # Sort by distance and return with recommendation. log.debug('Found %i candidates.' % len(candidates)) diff --git a/beets/importer.py b/beets/importer.py index 29ab428ce..8a8418656 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -316,7 +316,7 @@ class ImportTask(object): obj.is_album = False return obj - def set_match(self, cur_artist, cur_album, candidates, rec): + def set_candidates(self, cur_artist, cur_album, candidates, rec): """Sets the candidates for this album matched by the `autotag.tag_album` method. """ @@ -327,45 +327,39 @@ class ImportTask(object): self.candidates = candidates self.rec = rec - def set_null_match(self): + def set_null_candidates(self): """Set the candidates to indicate no album match was found. """ - self.set_match(None, None, None, None) + self.cur_artist = None + self.cur_album = None + self.candidates = None + self.rec = None - def set_item_match(self, candidates, rec): + def set_item_candidates(self, candidates, rec): """Set the match for a single-item task.""" assert not self.is_album assert self.item is not None - self.item_match = (candidates, rec) - - def set_null_item_match(self): - """For single-item tasks, mark the item as having no matches. - """ - assert not self.is_album - assert self.item is not None - self.item_match = None + self.candidates = candidates + self.rec = rec def set_choice(self, choice): - """Given either an (info, items) tuple or an action constant, - indicates that an action has been selected by the user (or - automatically). + """Given an AlbumMatch or TrackMatch object or an action constant, + indicates that an action has been selected for this task. """ assert not self.sentinel # Not part of the task structure: assert choice not in (action.MANUAL, action.MANUAL_ID) - assert choice != action.APPLY # Only used internally. + assert choice != action.APPLY # Only used internally. if choice in (action.SKIP, action.ASIS, action.TRACKS): self.choice_flag = choice - self.info = None + self.match = None else: - assert not isinstance(choice, action) if self.is_album: - info, items = choice - self.items = items # Reordered items list. + assert isinstance(choice, autotag.AlbumMatch) else: - info = choice - self.info = info - self.choice_flag = action.APPLY # Implicit choice. + assert isinstance(choice, autotag.TrackMatch) + self.choice_flag = action.APPLY # Implicit choice. + self.match = choice def save_progress(self): """Updates the progress state to indicate that this album has @@ -418,20 +412,26 @@ class ImportTask(object): if self.choice_flag is action.ASIS: return (self.cur_artist, self.cur_album) elif self.choice_flag is action.APPLY: - return (self.info.artist, self.info.album) + return (self.match.info.artist, self.match.info.album) else: if self.choice_flag is action.ASIS: return (self.item.artist, self.item.title) elif self.choice_flag is action.APPLY: - return (self.info.artist, self.info.title) + return (self.match.info.artist, self.match.info.title) - def all_items(self): - """If this is an album task, returns the list of non-None - (non-gap) items. If this is a singleton task, returns a list - containing the item. + def imported_items(self): + """Return a list of Items that should be added to the library. + If this is an album task, return the list of items in the + selected match or everything if the choice is ASIS. If this is a + singleton task, return a list containing the item. """ if self.is_album: - return [i for i in self.items if i] + if self.choice_flag == action.ASIS: + return list(self.items) + elif self.choice_flag == action.APPLY: + return self.match.mapping.keys() + else: + assert False else: return [self.item] @@ -559,9 +559,9 @@ def initial_lookup(config): log.debug('Looking up: %s' % task.path) try: - task.set_match(*autotag.tag_album(task.items, config.timid)) + task.set_candidates(*autotag.tag_album(task.items, config.timid)) except autotag.AutotagError: - task.set_null_match() + task.set_null_candidates() def user_query(config): """A coroutine for interfacing with the user about the tagging @@ -625,12 +625,12 @@ def show_progress(config): log.info(task.path) # Behave as if ASIS were selected. - task.set_null_match() + task.set_null_candidates() task.set_choice(action.ASIS) def apply_choices(config): - """A coroutine for applying changes to albums during the autotag - process. + """A coroutine for applying changes to albums and singletons during + the autotag process. """ task = None while True: @@ -638,7 +638,7 @@ def apply_choices(config): if task.should_skip(): continue - items = task.all_items() + items = task.imported_items() # Clear IDs in case the items are being re-tagged. for item in items: item.id = None @@ -648,11 +648,11 @@ def apply_choices(config): if task.should_write_tags(): if task.is_album: autotag.apply_metadata( - task.items, task.info, + task.match.info, task.match.mapping, per_disc_numbering=config.per_disc_numbering ) else: - autotag.apply_item_metadata(task.item, task.info) + autotag.apply_item_metadata(task.item, task.match.info) plugins.send('import_task_apply', config=config, task=task) # Infer album-level fields. @@ -738,7 +738,7 @@ def manipulate_files(config): continue # Move/copy files. - items = task.all_items() + items = task.imported_items() task.old_paths = [item.path for item in items] # For deletion. for item in items: if config.move: @@ -791,7 +791,7 @@ def finalize(config): task.save_history() continue - items = task.all_items() + items = task.imported_items() # Announce that we've added an album. if task.is_album: @@ -833,7 +833,7 @@ def item_lookup(config): plugins.send('import_task_start', task=task, config=config) - task.set_item_match(*autotag.tag_item(task.item, config.timid)) + task.set_item_candidates(*autotag.tag_item(task.item, config.timid)) def item_query(config): """A coroutine that queries the user for input on single-item @@ -871,7 +871,7 @@ def item_progress(config): continue log.info(displayable_path(task.item.path)) - task.set_null_item_match() + task.set_null_candidates() task.set_choice(action.ASIS) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 5a5969020..7a5961f7a 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -136,11 +136,11 @@ def dist_string(dist, color): out = ui.colorize('red', out) return out -def show_change(cur_artist, cur_album, items, info, dist, color=True, +def show_change(cur_artist, cur_album, match, color=True, per_disc_numbering=False): - """Print out a representation of the changes that will be made if - tags are changed from (cur_artist, cur_album, items) to info with - distance dist. + """Print out a representation of the changes that will be made if an + album's tags are changed according to `match`, which must be an AlbumMatch + object. """ def show_album(artist, album, partial=False): if artist: @@ -168,7 +168,7 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True, TrackInfo object. """ if per_disc_numbering: - if info.mediums > 1: + if match.info.mediums > 1: return u'{0}-{1}'.format(track_info.medium, track_info.medium_index) else: @@ -176,14 +176,12 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True, else: return unicode(track_info.index) - # Record if the match is partial or not. - partial_match = None in items - # Identify the album in question. - if cur_artist != info.artist or \ - (cur_album != info.album and info.album != VARIOUS_ARTISTS): - artist_l, artist_r = cur_artist or '', info.artist - album_l, album_r = cur_album or '', info.album + if cur_artist != match.info.artist or \ + (cur_album != match.info.album and + match.info.album != VARIOUS_ARTISTS): + artist_l, artist_r = cur_artist or '', match.info.artist + album_l, album_r = cur_album or '', match.info.album if artist_r == VARIOUS_ARTISTS: # Hide artists for VA releases. artist_l, artist_r = u'', u'' @@ -197,8 +195,8 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True, print_("To:") show_album(artist_r, album_r) else: - message = u"Tagging: %s - %s" % (info.artist, info.album) - if partial_match: + message = u"Tagging: %s - %s" % (match.info.artist, match.info.album) + if match.extra_items or match.extra_tracks: warning = PARTIAL_MATCH_MESSAGE if color: warning = ui.colorize('yellow', PARTIAL_MATCH_MESSAGE) @@ -206,15 +204,12 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True, print_(message) # Distance/similarity. - print_('(Similarity: %s)' % dist_string(dist, color)) + print_('(Similarity: %s)' % dist_string(match.distance, color)) # Tracks. - missing_tracks = [] - for item, track_info in zip(items, info.tracks): - if not item: - missing_tracks.append(track_info) - continue - + pairs = match.mapping.items() + pairs.sort(key=lambda (_, track_info): track_info.index) + for item, track_info in pairs: # Get displayable LHS and RHS values. cur_track = unicode(item.track) new_track = format_index(track_info) @@ -258,20 +253,25 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True, if display: print_(line) - # Missing tracks. - for track_info in missing_tracks: + # Missing and unmatched tracks. + for track_info in match.extra_tracks: line = u' * Missing track: {0} ({1})'.format(track_info.title, format_index(track_info)) if color: line = ui.colorize('yellow', line) print_(line) + for item in match.extra_items: + line = u' * Unmatched track: {0} ({1})'.format(item.title, item.track) + if color: + line = ui.colorize('yellow', line) + print_(line) -def show_item_change(item, info, dist, color): +def show_item_change(item, match, color): """Print out the change that would occur by tagging `item` with the - metadata from `info`. + metadata from `match`, a TrackMatch object. """ - cur_artist, new_artist = item.artist, info.artist - cur_title, new_title = item.title, info.title + cur_artist, new_artist = item.artist, match.info.artist + cur_title, new_title = item.title, match.info.title if cur_artist != new_artist or cur_title != new_title: if color: @@ -286,7 +286,7 @@ def show_item_change(item, info, dist, color): else: print_("Tagging track: %s - %s" % (cur_artist, cur_title)) - print_('(Similarity: %s)' % dist_string(dist, color)) + print_('(Similarity: %s)' % dist_string(match.distance, color)) def should_resume(config, path): return ui.input_yn("Import of the directory:\n%s" @@ -309,14 +309,13 @@ def choose_candidate(candidates, singleton, rec, color, timid, itemcount=None, per_disc_numbering=False): """Given a sorted list of candidates, ask the user for a selection of which candidate to use. Applies to both full albums and - singletons (tracks). For albums, the candidates are `(dist, items, - info)` triples and `cur_artist`, `cur_album`, and `itemcount` must - be provided. For singletons, the candidates are `(dist, info)` pairs - and `item` must be provided. + singletons (tracks). Candidates are either AlbumMatch or TrackMatch + objects depending on `singleton`. for albums, `cur_artist`, + `cur_album`, and `itemcount` must be provided. For singletons, + `item` must be provided. Returns the result of the choice, which may SKIP, ASIS, TRACKS, or - MANUAL or a candidate. For albums, a candidate is a `(info, items)` - pair; for items, it is just a TrackInfo object. + MANUAL or a candidate (an AlbumMatch/TrackMatch object). """ # Sanity check. if singleton: @@ -358,10 +357,7 @@ def choose_candidate(candidates, singleton, rec, color, timid, # Is the change good enough? bypass_candidates = False if rec != autotag.RECOMMEND_NONE: - if singleton: - dist, info = candidates[0] - else: - dist, items, info = candidates[0] + match = candidates[0] bypass_candidates = True while True: @@ -372,22 +368,24 @@ def choose_candidate(candidates, singleton, rec, color, timid, print_('Finding tags for track "%s - %s".' % (item.artist, item.title)) print_('Candidates:') - for i, (dist, info) in enumerate(candidates): - print_('%i. %s - %s (%s)' % (i+1, info.artist, - info.title, dist_string(dist, color))) + for i, match in enumerate(candidates): + print_('%i. %s - %s (%s)' % + (i + 1, match.info.artist, match.info.title, + dist_string(match.distance, color))) else: print_('Finding tags for album "%s - %s".' % (cur_artist, cur_album)) print_('Candidates:') - for i, (dist, items, info) in enumerate(candidates): - line = '%i. %s - %s' % (i+1, info.artist, info.album) + for i, match in enumerate(candidates): + line = '%i. %s - %s' % (i + 1, match.info.artist, + match.info.album) # Label and year disambiguation, if available. label, year = None, None - if info.label: - label = info.label - if info.year: - year = unicode(info.year) + if match.info.label: + label = match.info.label + if match.info.year: + year = unicode(match.info.year) if label and year: line += u' [%s, %s]' % (label, year) elif label: @@ -395,10 +393,10 @@ def choose_candidate(candidates, singleton, rec, color, timid, elif year: line += u' [%s]' % year - line += ' (%s)' % dist_string(dist, color) + line += ' (%s)' % dist_string(match.distance, color) # Point out the partial matches. - if None in items: + if match.extra_items or match.extra_tracks: warning = PARTIAL_MATCH_MESSAGE if color: warning = ui.colorize('yellow', warning) @@ -428,26 +426,23 @@ def choose_candidate(candidates, singleton, rec, color, timid, raise importer.ImportAbort() elif sel == 'i': return importer.action.MANUAL_ID - else: # Numerical selection. + else: # Numerical selection. if singleton: - dist, info = candidates[sel-1] + match = candidates[sel - 1] else: - dist, items, info = candidates[sel-1] + match = candidates[sel - 1] bypass_candidates = False # Show what we're about to do. if singleton: - show_item_change(item, info, dist, color) + show_item_change(item, match, color) else: - show_change(cur_artist, cur_album, items, info, dist, color, + show_change(cur_artist, cur_album, match, color, per_disc_numbering) # Exact match => tag automatically if we're not in timid mode. if rec == autotag.RECOMMEND_STRONG and not timid: - if singleton: - return info - else: - return info, items + return match # Ask for confirmation. if singleton: @@ -458,10 +453,7 @@ def choose_candidate(candidates, singleton, rec, color, timid, 'as Tracks', 'Enter search', 'enter Id', 'aBort') sel = ui.input_options(opts, color=color) if sel == 'a': - if singleton: - return info - else: - return info, items + return match elif sel == 'm': pass elif sel == 's': @@ -505,7 +497,7 @@ def manual_id(singleton): def choose_match(task, config): """Given an initial autotagging of items, go through an interactive dance with the user to ask for a choice of metadata. Returns an - (info, items) pair, ASIS, or SKIP. + AlbumMatch object, ASIS, or SKIP. """ # Show what we're tagging. print_() @@ -514,10 +506,9 @@ def choose_match(task, config): if config.quiet: # No input; just make a decision. if task.rec == autotag.RECOMMEND_STRONG: - dist, items, info = task.candidates[0] - show_change(task.cur_artist, task.cur_album, items, info, dist, - config.color) - return info, items + match = task.candidates[0] + show_change(task.cur_artist, task.cur_album, match, config.color) + return match else: return _quiet_fall_back(config) @@ -555,25 +546,25 @@ def choose_match(task, config): except autotag.AutotagError: candidates, rec = None, None else: - # We have a candidate! Finish tagging. Here, choice is - # an (info, items) pair as desired. - assert not isinstance(choice, importer.action) + # We have a candidate! Finish tagging. Here, choice is an + # AlbumMatch object. + assert isinstance(choice, autotag.AlbumMatch) return choice def choose_item(task, config): """Ask the user for a choice about tagging a single item. Returns - either an action constant or a TrackInfo object. + either an action constant or a TrackMatch object. """ print_() print_(task.item.path) - candidates, rec = task.item_match + candidates, rec = task.candidates, task.rec if config.quiet: # Quiet mode; make a decision. if rec == autotag.RECOMMEND_STRONG: - dist, track_info = candidates[0] - show_item_change(task.item, track_info, dist, config.color) - return track_info + match = candidates[0] + show_item_change(task.item, match, config.color) + return match else: return _quiet_fall_back(config) @@ -596,10 +587,10 @@ def choose_item(task, config): search_id = manual_id(True) if search_id: candidates, rec = autotag.tag_item(task.item, config.timid, - search_id=search_id) + search_id=search_id) else: # Chose a candidate. - assert not isinstance(choice, importer.action) + assert isinstance(choice, autotag.TrackMatch) return choice def resolve_duplicate(task, config): diff --git a/docs/changelog.rst b/docs/changelog.rst index c45b3b8e5..d9d8a4715 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,12 @@ which was previously baked into the import workflow, is now encapsulated in a plugin (the :doc:`/plugins/fetchart`). If you want to continue fetching cover art for your music, enable this plugin after upgrading to beets 1.0b15. +* The autotagger can now find matches for albums when you have extra tracks on + your filesystem that aren't present in the MusicBrainz catalog. Previously, if + you tried to match album with 15 audio files but the MusicBrainz entry had + only 14 tracks, beets would ignore this match. Now, beets will show you + matches even when they are "too short" and indicate which tracks from your + disk are unmatched. * Tracks on multi-disc albums can now be numbered per-disc instead of per-album via the :ref:`per_disc_numbering` config option. * The default output format for the ``beet list`` command is now configurable diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index 2c51541a0..e32f5a50b 100644 --- a/docs/guides/tagger.rst +++ b/docs/guides/tagger.rst @@ -64,7 +64,7 @@ all of these limitations. * Currently MP3, AAC, FLAC, Ogg Vorbis, Monkey's Audio, WavPack, and Musepack files are supported. (Do you use some other format? - `Let me know!`_ + `Let me know!`_) .. _Let me know!: mailto:adrian@radbox.org @@ -243,23 +243,15 @@ cover art `, :doc:`song lyrics `, and Missing Albums? --------------- -If you're having trouble tagging a particular album with beets, you might want to check the following possibilities: - -* Is the album present in `the MusicBrainz database`_? You can search on their - site to make sure it's cataloged there. If not, anyone can edit - MusicBrainz---so consider adding the data yourself. - -* Beets won't show you possibilities from MusicBrainz that have *fewer* tracks - than the current album. In other words, if you have extra tracks that aren't - included on the release, that candidate won't be displayed. (The tagger - should, on the other hand, show you candidates that have *more* tracks than - you do in the case that you're missing some of the album's songs. Beets will - warn you when any candidate is a partial match.) +If you're having trouble tagging a particular album with beets, check to make +sure the album is present in `the MusicBrainz database`_. You can search on +their site to make sure it's cataloged there. If not, anyone can edit +MusicBrainz---so consider adding the data yourself. .. _the MusicBrainz database: http://musicbrainz.org/ -If neither of these situations apply and you're still having trouble tagging -something, please `file a bug report`_. +If you think beets is ignoring an album that's listed in MusicBrainz, please +`file a bug report`_. .. _file a bug report: http://code.google.com/p/beets/issues/entry diff --git a/test/test_art.py b/test/test_art.py index 37ffc176f..a125de0c7 100644 --- a/test/test_art.py +++ b/test/test_art.py @@ -17,7 +17,7 @@ import _common from _common import unittest from beetsplug import fetchart -from beets.autotag import AlbumInfo +from beets.autotag import AlbumInfo, AlbumMatch from beets import library from beets import importer import os @@ -227,7 +227,7 @@ class ArtImporterTest(unittest.TestCase, _common.ExtraAsserts): artist_id = 'artistid', tracks = [], ) - self.task.set_choice((info, [self.i])) + self.task.set_choice(AlbumMatch(0, info, {}, set(), set())) def tearDown(self): fetchart.art_for_album = self.old_afa diff --git a/test/test_autotag.py b/test/test_autotag.py index 69533ff95..a07bad33c 100644 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -111,6 +111,15 @@ class TrackDistanceTest(unittest.TestCase): self.assertEqual(dist, 0.0) class AlbumDistanceTest(unittest.TestCase): + def _mapping(self, items, info): + out = {} + for i, t in zip(items, info.tracks): + out[i] = t + return out + + def _dist(self, items, info): + return match.distance(items, info, self._mapping(items, info)) + def test_identical_albums(self): items = [] items.append(_make_item('one', 1)) @@ -123,7 +132,7 @@ class AlbumDistanceTest(unittest.TestCase): va = False, album_id = None, artist_id = None, ) - self.assertEqual(match.distance(items, info), 0) + self.assertEqual(self._dist(items, info), 0) def test_incomplete_album(self): items = [] @@ -136,9 +145,10 @@ class AlbumDistanceTest(unittest.TestCase): va = False, album_id = None, artist_id = None, ) - self.assertNotEqual(match.distance(items, info), 0) + dist = self._dist(items, info) + self.assertNotEqual(dist, 0) # Make sure the distance is not too great - self.assertTrue(match.distance(items, info) < 0.2) + self.assertTrue(dist < 0.2) def test_global_artists_differ(self): items = [] @@ -152,7 +162,7 @@ class AlbumDistanceTest(unittest.TestCase): va = False, album_id = None, artist_id = None, ) - self.assertNotEqual(match.distance(items, info), 0) + self.assertNotEqual(self._dist(items, info), 0) def test_comp_track_artists_match(self): items = [] @@ -166,7 +176,7 @@ class AlbumDistanceTest(unittest.TestCase): va = True, album_id = None, artist_id = None, ) - self.assertEqual(match.distance(items, info), 0) + self.assertEqual(self._dist(items, info), 0) def test_comp_no_track_artists(self): # Some VA releases don't have track artists (incomplete metadata). @@ -184,7 +194,7 @@ class AlbumDistanceTest(unittest.TestCase): info.tracks[0].artist = None info.tracks[1].artist = None info.tracks[2].artist = None - self.assertEqual(match.distance(items, info), 0) + self.assertEqual(self._dist(items, info), 0) def test_comp_track_artists_do_not_match(self): items = [] @@ -198,7 +208,7 @@ class AlbumDistanceTest(unittest.TestCase): va = True, album_id = None, artist_id = None, ) - self.assertNotEqual(match.distance(items, info), 0) + self.assertNotEqual(self._dist(items, info), 0) def test_tracks_out_of_order(self): items = [] @@ -212,7 +222,7 @@ class AlbumDistanceTest(unittest.TestCase): va = False, album_id = None, artist_id = None, ) - dist = match.distance(items, info) + dist = self._dist(items, info) self.assertTrue(0 < dist < 0.2) def test_two_medium_release(self): @@ -230,7 +240,7 @@ class AlbumDistanceTest(unittest.TestCase): info.tracks[0].medium_index = 1 info.tracks[1].medium_index = 2 info.tracks[2].medium_index = 1 - dist = match.distance(items, info) + dist = self._dist(items, info) self.assertEqual(dist, 0) def test_per_medium_track_numbers(self): @@ -248,7 +258,7 @@ class AlbumDistanceTest(unittest.TestCase): info.tracks[0].medium_index = 1 info.tracks[1].medium_index = 2 info.tracks[2].medium_index = 1 - dist = match.distance(items, info) + dist = self._dist(items, info) self.assertEqual(dist, 0) def _mkmp3(path): @@ -472,7 +482,15 @@ class AssignmentTest(unittest.TestCase): for item, info in mapping.iteritems(): self.assertEqual(items.index(item), trackinfo.index(info)) -class ApplyTest(unittest.TestCase): +class ApplyTestUtil(object): + def _apply(self, info=None, per_disc_numbering=False): + info = info or self.info + mapping = {} + for i, t in zip(self.items, info.tracks): + mapping[i] = t + autotag.apply_metadata(info, mapping, per_disc_numbering) + +class ApplyTest(unittest.TestCase, ApplyTestUtil): def setUp(self): self.items = [] self.items.append(Item({})) @@ -500,52 +518,51 @@ class ApplyTest(unittest.TestCase): ) def test_titles_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].title, 'oneNew') self.assertEqual(self.items[1].title, 'twoNew') def test_album_and_artist_applied_to_all(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].album, 'albumNew') self.assertEqual(self.items[1].album, 'albumNew') self.assertEqual(self.items[0].artist, 'artistNew') self.assertEqual(self.items[1].artist, 'artistNew') def test_track_index_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].track, 1) self.assertEqual(self.items[1].track, 2) def test_track_total_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].tracktotal, 2) self.assertEqual(self.items[1].tracktotal, 2) def test_disc_index_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].disc, 1) self.assertEqual(self.items[1].disc, 2) def test_disc_total_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].disctotal, 2) self.assertEqual(self.items[1].disctotal, 2) def test_per_disc_numbering(self): - autotag.apply_metadata(self.items, self.info, - per_disc_numbering=True) + self._apply(per_disc_numbering=True) self.assertEqual(self.items[0].track, 1) self.assertEqual(self.items[1].track, 1) def test_mb_trackid_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].mb_trackid, 'dfa939ec-118c-4d0f-84a0-60f3d1e6522c') self.assertEqual(self.items[1].mb_trackid, '40130ed1-a27c-42fd-a328-1ebefb6caef4') def test_mb_albumid_and_artistid_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() for item in self.items: self.assertEqual(item.mb_albumid, '7edb51cb-77d6-4416-a23c-3a8c2994a2c7') @@ -553,13 +570,13 @@ class ApplyTest(unittest.TestCase): 'a6623d39-2d8e-4f70-8242-0a9553b91e50') def test_albumtype_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].albumtype, 'album') self.assertEqual(self.items[1].albumtype, 'album') def test_album_artist_overrides_empty_track_artist(self): my_info = copy.deepcopy(self.info) - autotag.apply_metadata(self.items, my_info) + self._apply(info=my_info) self.assertEqual(self.items[0].artist, 'artistNew') self.assertEqual(self.items[0].artist, 'artistNew') @@ -567,25 +584,25 @@ class ApplyTest(unittest.TestCase): my_info = copy.deepcopy(self.info) my_info.tracks[0].artist = 'artist1!' my_info.tracks[1].artist = 'artist2!' - autotag.apply_metadata(self.items, my_info) + self._apply(info=my_info) self.assertEqual(self.items[0].artist, 'artist1!') self.assertEqual(self.items[1].artist, 'artist2!') def test_artist_credit_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].albumartist_credit, 'albumArtistCredit') self.assertEqual(self.items[0].artist_credit, 'trackArtistCredit') self.assertEqual(self.items[1].albumartist_credit, 'albumArtistCredit') self.assertEqual(self.items[1].artist_credit, 'albumArtistCredit') def test_artist_sort_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].albumartist_sort, 'albumArtistSort') self.assertEqual(self.items[0].artist_sort, 'trackArtistSort') self.assertEqual(self.items[1].albumartist_sort, 'albumArtistSort') self.assertEqual(self.items[1].artist_sort, 'albumArtistSort') -class ApplyCompilationTest(unittest.TestCase): +class ApplyCompilationTest(unittest.TestCase, ApplyTestUtil): def setUp(self): self.items = [] self.items.append(Item({})) @@ -616,14 +633,14 @@ class ApplyCompilationTest(unittest.TestCase): ) def test_album_and_track_artists_separate(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].artist, 'artistOneNew') self.assertEqual(self.items[1].artist, 'artistTwoNew') self.assertEqual(self.items[0].albumartist, 'variousNew') self.assertEqual(self.items[1].albumartist, 'variousNew') def test_mb_albumartistid_applied(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertEqual(self.items[0].mb_albumartistid, '89ad4ac3-39f7-470e-963a-56509c546377') self.assertEqual(self.items[1].mb_albumartistid, @@ -634,14 +651,14 @@ class ApplyCompilationTest(unittest.TestCase): '80b3cf5e-18fe-4c59-98c7-e5bb87210710') def test_va_flag_cleared_does_not_set_comp(self): - autotag.apply_metadata(self.items, self.info) + self._apply() self.assertFalse(self.items[0].comp) self.assertFalse(self.items[1].comp) def test_va_flag_sets_comp(self): va_info = copy.deepcopy(self.info) va_info.va = True - autotag.apply_metadata(self.items, va_info) + self._apply(info=va_info) self.assertTrue(self.items[0].comp) self.assertTrue(self.items[1].comp) diff --git a/test/test_importer.py b/test/test_importer.py index c7e4476a2..45ddc2ac4 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -23,7 +23,7 @@ from _common import unittest from beets import library from beets import importer from beets import mediafile -from beets.autotag import AlbumInfo, TrackInfo +from beets.autotag import AlbumInfo, TrackInfo, AlbumMatch, TrackMatch TEST_TITLES = ('The Opener', 'The Second Track', 'The Last Track') class NonAutotaggedImportTest(unittest.TestCase): @@ -182,7 +182,8 @@ def _call_stages(config, items, choice_or_info, if isinstance(choice_or_info, importer.action): task.set_choice(choice_or_info) else: - task.set_choice((choice_or_info, items)) + mapping = dict(zip(items, choice_or_info.tracks)) + task.set_choice(AlbumMatch(0, choice_or_info, mapping, set(), set())) # Call the coroutines. for stage in stages: @@ -265,7 +266,7 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts): manip_coro.next() task = importer.ImportTask.item_task(self.i) - task.set_choice(self.info.tracks[0]) + task.set_choice(TrackMatch(0, self.info.tracks[0])) apply_coro.send(task) manip_coro.send(task) @@ -579,7 +580,7 @@ class InferAlbumDataTest(unittest.TestCase): self.task = importer.ImportTask(path='a path', toppath='top path', items=self.items) - self.task.set_null_match() + self.task.set_null_candidates() def _infer(self): importer._infer_album_fields(self.task) @@ -621,7 +622,7 @@ class InferAlbumDataTest(unittest.TestCase): self.assertEqual(self.items[0].albumartist, self.items[2].artist) def test_apply_gets_artist_and_id(self): - self.task.set_choice(({}, self.items)) # APPLY + self.task.set_choice(AlbumMatch(0, None, {}, set(), set())) # APPLY self._infer() @@ -633,7 +634,7 @@ class InferAlbumDataTest(unittest.TestCase): for item in self.items: item.albumartist = 'some album artist' item.mb_albumartistid = 'some album artist id' - self.task.set_choice(({}, self.items)) # APPLY + self.task.set_choice(AlbumMatch(0, None, {}, set(), set())) # APPLY self._infer() @@ -651,7 +652,7 @@ class InferAlbumDataTest(unittest.TestCase): def test_first_item_null_apply(self): self.items[0] = None - self.task.set_choice(({}, self.items)) # APPLY + self.task.set_choice(AlbumMatch(0, None, {}, set(), set())) # APPLY self._infer() self.assertFalse(self.items[1].comp) self.assertEqual(self.items[1].albumartist, self.items[2].artist) @@ -672,14 +673,12 @@ class DuplicateCheckTest(unittest.TestCase): task = importer.ImportTask(path='a path', toppath='top path', items=[item]) - task.set_match(artist, album, None, None) + task.set_candidates(artist, album, None, None) if asis: task.set_choice(importer.action.ASIS) else: - task.set_choice(( - AlbumInfo(album, None, artist, None, None), - [item] - )) + info = AlbumInfo(album, None, artist, None, None) + task.set_choice(AlbumMatch(0, info, {}, set(), set())) return task def _item_task(self, asis, artist=None, title=None, existing=False): @@ -696,7 +695,7 @@ class DuplicateCheckTest(unittest.TestCase): item.title = title task.set_choice(importer.action.ASIS) else: - task.set_choice(TrackInfo(title, None, artist)) + task.set_choice(TrackMatch(0, TrackInfo(title, None, artist))) return task def test_duplicate_album_apply(self): diff --git a/test/test_ui.py b/test/test_ui.py index 8180d5ecc..3c3ddf81d 100644 --- a/test/test_ui.py +++ b/test/test_ui.py @@ -462,7 +462,7 @@ class AutotagTest(unittest.TestCase): 'path', [_common.item()], ) - task.set_match('artist', 'album', [], autotag.RECOMMEND_NONE) + task.set_candidates('artist', 'album', [], autotag.RECOMMEND_NONE) res = commands.choose_match(task, _common.iconfig(None, quiet=False)) self.assertEqual(res, result) self.assertTrue('No match' in self.io.getoutput()) @@ -647,72 +647,66 @@ class ShowChangeTest(unittest.TestCase): def setUp(self): self.io = _common.DummyIO() self.io.install() + + self.items = [_common.item()] + self.items[0].track = 1 + self.items[0].path = '/path/to/file.mp3' + self.info = autotag.AlbumInfo( + 'the album', 'album id', 'the artist', 'artist id', [ + autotag.TrackInfo('the title', 'track id', index=1) + ]) + def tearDown(self): self.io.restore() - def _items_and_info(self): - items = [_common.item()] - items[0].track = 1 - items[0].path = '/path/to/file.mp3' - info = autotag.AlbumInfo( - 'the album', 'album id', 'the artist', 'artist id', [ - autotag.TrackInfo('the title', 'track id') - ]) - return items, info + def _show_change(self, items=None, info=None, + cur_artist='the artist', cur_album='the album', + dist=0.1): + items = items or self.items + info = info or self.info + mapping = dict(zip(items, info.tracks)) + commands.show_change( + cur_artist, + cur_album, + autotag.AlbumMatch(0.1, info, mapping, set(), set()), + color=False, + ) + return self.io.getoutput().lower() def test_null_change(self): - items, info = self._items_and_info() - commands.show_change('the artist', 'the album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + msg = self._show_change() self.assertTrue('similarity: 90' in msg) self.assertTrue('tagging:' in msg) def test_album_data_change(self): - items, info = self._items_and_info() - commands.show_change('another artist', 'another album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + msg = self._show_change(cur_artist='another artist', + cur_album='another album') self.assertTrue('correcting tags from:' in msg) def test_item_data_change(self): - items, info = self._items_and_info() - items[0].title = 'different' - commands.show_change('the artist', 'the album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + self.items[0].title = 'different' + msg = self._show_change() self.assertTrue('different -> the title' in msg) def test_item_data_change_with_unicode(self): - items, info = self._items_and_info() - items[0].title = u'caf\xe9' - commands.show_change('the artist', 'the album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + self.items[0].title = u'caf\xe9' + msg = self._show_change() self.assertTrue(u'caf\xe9 -> the title' in msg.decode('utf8')) def test_album_data_change_with_unicode(self): - items, info = self._items_and_info() - commands.show_change(u'caf\xe9', u'another album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + msg = self._show_change(cur_artist=u'caf\xe9', + cur_album=u'another album') self.assertTrue('correcting tags from:' in msg) def test_item_data_change_title_missing(self): - items, info = self._items_and_info() - items[0].title = '' - commands.show_change('the artist', 'the album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + self.items[0].title = '' + msg = self._show_change() self.assertTrue('file.mp3 -> the title' in msg) def test_item_data_change_title_missing_with_unicode_filename(self): - items, info = self._items_and_info() - items[0].title = '' - items[0].path = u'/path/to/caf\xe9.mp3'.encode('utf8') - commands.show_change('the artist', 'the album', - items, info, 0.1, color=False) - msg = self.io.getoutput().lower() + self.items[0].title = '' + self.items[0].path = u'/path/to/caf\xe9.mp3'.encode('utf8') + msg = self._show_change() self.assertTrue(u'caf\xe9.mp3 -> the title' in msg.decode('utf8')) class DefaultPathTest(unittest.TestCase):