cleanup for partial match feature (#260)

- Plugins are sent the unadulterated, None-ridden ordered items lists. Changed
  the lastid plugin to accommodate this.
- Make colorization optional in partial album warnings.
- Fix some tests.

beets/autotag/match.py

@@ -33,7 +33,7 @@ ALBUM_WEIGHT = 3.0
 # The weight of the entire distance calculated for a given track.
 TRACK_WEIGHT = 1.0
 # The weight of a missing track.
-MISSING_WEIGHT = 0.3
+MISSING_WEIGHT = 0.9
 # 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).
@@ -171,8 +171,11 @@ def current_metadata(items):
 
 def order_items(items, trackinfo):
     """Orders the items based on how they match some canonical track
-    information. This always produces a result if the numbers of tracks
-    match.
+    information. Returns a list of Items whose length is equal to the
+    length of ``trackinfo``. This always produces a result if the
+    numbers of items is at most the number of TrackInfo objects
+    (otherwise, returns None). In the case of a partial match, the
+    returned list may contain None in some positions.
     """
     # Make sure lengths match: If there is less items, it might just be that
     # there is some tracks missing.
@@ -282,16 +285,7 @@ def distance(items, album_info):
             dist_max += MISSING_WEIGHT
 
     # Plugin distances.
-    # In order not to break compatibility, send purged lists
-    purged_items, purged_tracks = [], []
-    for i, t in zip(items, album_info.tracks):
-        if i:
-            purged_items.append(i)
-            purged_tracks.append(t)
-    purged_album_info = copy.copy(album_info)
-    purged_album_info.tracks = purged_tracks
-
-    plugin_d, plugin_dm = plugins.album_distance(purged_items, purged_album_info)
+    plugin_d, plugin_dm = plugins.album_distance(items, album_info)
     dist += plugin_d
     dist_max += plugin_dm
 
@@ -366,7 +360,8 @@ def validate_candidate(items, tuple_dict, info):
 
     # Make sure the album has the correct number of tracks.
     if len(items) > len(info.tracks):
-        log.debug('Track count mismatch.')
+        log.debug('Too many items to match: %i > %i.' %
+                  (len(items), len(info.tracks)))
         return
 
     # Put items in order.

beets/importer.py

@@ -637,7 +637,7 @@ def apply_choices(config):
             # Add new ones.
             if task.is_album:
                 # Add an album.
-                album = lib.add_album([i for i in task.items if i])
+                album = lib.add_album(items)
                 task.album_id = album.id
             else:
                 # Add tracks.

beets/ui/commands.py

@@ -101,7 +101,7 @@ DEFAULT_IGNORE                = ['.AppleDouble', '._*', '*~', '.DS_Store']
 
 VARIOUS_ARTISTS = u'Various Artists'
 
-PARTIAL_MATCH_STRING = ui.colorize('green', u'(Partial match !)')
+PARTIAL_MATCH_MESSAGE = u'(partial match!)'
 
 # Importer utilities and support.
 
@@ -126,17 +126,24 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True):
     """
     def show_album(artist, album, partial=False):
         if artist:
-            album_description = '    %s - %s' % (artist, album)
+            album_description = u'    %s - %s' % (artist, album)
         elif album:
-            album_description = '    %s' % album
+            album_description = u'    %s' % album
         else:
-            album_description = '    (unknown album)'
+            album_description = u'    (unknown album)'
 
-        # Add a suffix indicating a partial match
+        # Add a suffix if this is a partial match.
         if partial:
-            print_('%s %s' % (album_description, PARTIAL_MATCH_STRING))
+            warning = PARTIAL_MATCH_MESSAGE
         else:
-            print_(album_description)
+            warning = None
+        if color and warning:
+            warning = ui.colorize('yellow', warning)
+
+        out = album_description
+        if warning:
+            out += u' ' + warning
+        print_(out)
 
     # Record if the match is partial or not.
     partial_match = None in items
@@ -158,11 +165,14 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True):
         show_album(artist_l, album_l)
         print_("To:")
         show_album(artist_r, album_r)
-    elif partial_match:
-        print_("Tagging: %s - %s %s" % (info.artist, info.album,
-                                        PARTIAL_MATCH_STRING))
     else:
-        print_("Tagging: %s - %s" % (info.artist, info.album))
+        message = u"Tagging: %s - %s" % (info.artist, info.album)
+        if partial_match:
+            warning = PARTIAL_MATCH_MESSAGE
+            if color:
+                warning = ui.colorize('yellow', PARTIAL_MATCH_MESSAGE)
+            message += u' ' + warning
+        print_(message)
 
     # Distance/similarity.
     print_('(Similarity: %s)' % dist_string(dist, color))
@@ -198,8 +208,10 @@ def show_change(cur_artist, cur_album, items, info, dist, color=True):
         elif cur_track != new_track:
             print_(u" * %s (%s -> %s)" % (item.title, cur_track, new_track))
     for i, track_info in missing_tracks:
-        print_(ui.colorize('red', u' * Missing track:  %s (%d)' % \
-                           (track_info.title, i+1)))
+        line = u' * Missing track: %s (%d)' % (track_info.title, i+1)
+        if color:
+            line = ui.colorize('yellow', line)
+        print_(line)
 
 def show_item_change(item, info, dist, color):
     """Print out the change that would occur by tagging `item` with the
@@ -327,9 +339,12 @@ def choose_candidate(candidates, singleton, rec, color, timid,
 
                     line += ' (%s)' % dist_string(dist, color)
 
-                    # Pointing out the partial matches.
+                    # Point out the partial matches.
                     if None in items:
-                        line += ' %s' % PARTIAL_MATCH_STRING
+                        warning = PARTIAL_MATCH_MESSAGE
+                        if color:
+                            warning = ui.colorize('yellow', warning)
+                        line += u' %s' % warning
 
                     print_(line)
                                             

beetsplug/lastid.py

@@ -103,7 +103,9 @@ class LastIdPlugin(plugins.BeetsPlugin):
         return dist * DISTANCE_SCALE, dist_max * DISTANCE_SCALE
 
     def album_distance(self, items, info):
-        last_artist, last_artist_id = get_cur_artist(items)
+        last_artist, last_artist_id = get_cur_artist(
+            [item for item in items if item]
+        )
 
         # Compare artist to MusicBrainz metadata.
         dist, dist_max = 0.0, 0.0

test/test_autotag.py

@@ -313,16 +313,29 @@ class OrderingTest(unittest.TestCase):
         ordered = match.order_items(items, trackinfo)
         self.assertEqual(ordered, None)
 
-    def test_order_works_with_missing_tracks(self)
+    def test_order_works_with_missing_tracks(self):
+        items = []
+        items.append(self.item('one', 1))
+        items.append(self.item('three', 3))
+        trackinfo = []
+        trackinfo.append(TrackInfo('one', None))
+        trackinfo.append(TrackInfo('two', None))
+        trackinfo.append(TrackInfo('three', None))
+        ordered = match.order_items(items, trackinfo)
+        self.assertEqual(ordered[0].title, 'one')
+        self.assertEqual(ordered[1], None)
+        self.assertEqual(ordered[2].title, 'three')
+
+    def test_order_returns_none_for_extra_tracks(self):
         items = []
         items.append(self.item('one', 1))
         items.append(self.item('two', 2))
+        items.append(self.item('three', 3))
         trackinfo = []
         trackinfo.append(TrackInfo('one', None))
+        trackinfo.append(TrackInfo('three', None))
         ordered = match.order_items(items, trackinfo)
-        self.assertEqual(ordered[0].title, 'one')
-        self.assertEqual(ordered[1].title, 'two')
-        self.assertEqual(ordered[2], None)
+        self.assertEqual(ordered, None)
 
     def test_order_corrects_when_track_names_are_entirely_wrong(self):
         # A real-world test case contributed by a user.