From 3d30708839236e65e7e83ce2baf8dd0364451fb5 Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Tue, 22 Apr 2014 07:25:34 +0200
Subject: [PATCH 01/19] add test_lastgenre.py

---
 test/test_lastgenre.py | 44 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 test/test_lastgenre.py

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
new file mode 100644
index 000000000..67e1369e3
--- /dev/null
+++ b/test/test_lastgenre.py
@@ -0,0 +1,44 @@
+# This file is part of beets.
+# Copyright 2014, Fabrice Laporte.
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+
+"""Tests for the 'lastgenre' plugin."""
+
+from _common import unittest
+from beetsplug import lastgenre
+from beets import config
+
+class LastGenrePluginTest(unittest.TestCase):
+    def setUp(self):
+        """Set up configuration"""
+        lyrics.LastGenrePlugin()
+
+    def _setup_config(self, whitelist=set(), branches=None, count=1):
+        config['lastgenre']['whitelist'] = whitelist
+        if branches:
+            config['lastgenre']['branches'] = branches
+            config['lastgenre']['c14n'] = True
+        else:
+            config['lastgenre']['c14n'] = False
+        config['lastgenre']['count'] = count
+
+    def test_c14n():
+        _setup_config(set('blues'),
+                      [['blues'],
+                       ['blues', 'country blues'],
+                       ['blues', 'country blues', 'delta blues']])
+
+        self.assertEqual(lastgenre._strings_to_genre(['delta blues']),
+                         'blues')
+
+

From 499a7b868e12ee386193898fcffb88cad46d40aa Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <fabrice.laporte@balyo.com>
Date: Tue, 22 Apr 2014 12:59:08 +0200
Subject: [PATCH 02/19] Move the filtering logic of '_tags_for' function into a
 dedicated 'filter_tags' function.

---
 beetsplug/lastgenre/__init__.py | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index eea428dfa..c4f55dafa 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -66,25 +66,32 @@ def _tags_for(obj):
         log.debug(u'last.fm error: %s' % unicode(exc))
         return []
 
-    tags = []
+    res = filter_tags(res)
+    return res
+
+def filter_tags(tags):
+    """Filter tags based on their weights and whitelist content
+    """
+
+    res = []
     min_weight = config['lastgenre']['min_weight'].get(int)
     count = config['lastgenre']['count'].get(int)
 
     dbg = []
-    for el in res:
+    for el in tags:
         weight = int(el.weight or 0)
         tag = el.item.get_name().lower()
         if _is_allowed(tag):
-            if min_weight > -1 and min_weight > weight and len(tags) > 0:
-                return tags
-            tags.append(tag)
+            if min_weight > -1 and min_weight > weight and len(res) > 0:
+                return res
+            res.append(tag)
             dbg.append(u'{0} [{1}]'.format(tag, weight))
-            if len(tags) == count:
+            if len(res) == count:
                 break
     log.debug(u'lastfm.tag (min. {0}): {1}'.format(
         min_weight, u', '.join(dbg)
     ))
-    return tags
+    return res
 
 
 def _is_allowed(genre):

From 21f1607e8f30ab2f1a1cfe1feee2bd9487c7fd35 Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <fabrice.laporte@balyo.com>
Date: Tue, 22 Apr 2014 13:00:01 +0200
Subject: [PATCH 03/19] lastgenre: add tests functions

---
 test/test_lastgenre.py | 61 +++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 12 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 67e1369e3..79f77f83f 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -18,27 +18,64 @@ from _common import unittest
 from beetsplug import lastgenre
 from beets import config
 
+
 class LastGenrePluginTest(unittest.TestCase):
     def setUp(self):
         """Set up configuration"""
-        lyrics.LastGenrePlugin()
+        lastgenre.LastGenrePlugin()
 
     def _setup_config(self, whitelist=set(), branches=None, count=1):
-        config['lastgenre']['whitelist'] = whitelist
+        lastgenre.options['whitelist'] = whitelist
         if branches:
-            config['lastgenre']['branches'] = branches
-            config['lastgenre']['c14n'] = True
+            lastgenre.options['branches'] = branches
+            lastgenre.options['c14n'] = True
         else:
-            config['lastgenre']['c14n'] = False
+            lastgenre.options['c14n'] = False
+
         config['lastgenre']['count'] = count
 
-    def test_c14n():
-        _setup_config(set('blues'),
-                      [['blues'],
-                       ['blues', 'country blues'],
-                       ['blues', 'country blues', 'delta blues']])
+    def test_c14n(self):
+        """Resolve genres that belong to a canonicalization branch.
+        """
+        self._setup_config(whitelist=set(['blues']),
+                           branches=[['blues'], ['blues', 'country blues'],
+                                     ['blues', 'country blues',
+                                      'delta blues']])
 
-        self.assertEqual(lastgenre._strings_to_genre(['delta blues']),
-                         'blues')
+        self.assertEqual(lastgenre._strings_to_genre(['delta blues']), 'Blues')
+        self.assertEqual(lastgenre._strings_to_genre(['rhytm n blues']), '')
+
+    def test_whitelist(self):
+        """Keep only genres that are in the whitelist.
+        """
+        self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
+                           count=2)
+        self.assertEqual(lastgenre._strings_to_genre(['pop', 'blues']),
+                         'Blues')
+
+    def test_count(self):
+        """Keep the n first genres, as we expect them to be sorted from more to
+        less popular.
+        """
+        self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
+                           count=2)
+        self.assertEqual(lastgenre._strings_to_genre(
+                         ['jazz', 'pop', 'rock', 'blues']),
+                         'Jazz, Rock')
+
+    def test_count_c14n(self):
+        """Keep the n first genres, after having applied c14n when necessary
+        """
+        self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
+                           branches=[['blues'], ['blues', 'country blues']],
+                           count=2)
+        self.assertEqual(lastgenre._strings_to_genre(
+                         ['jazz', 'pop', 'country blues', 'rock']),
+                         'Jazz, Blues')
 
 
+def suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='suite')

From 4e5bb262a76dec8b330c95847474d60b912adb2f Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Thu, 24 Apr 2014 09:03:19 +0200
Subject: [PATCH 04/19] wip: rewrite setup_config

---
 test/test_lastgenre.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 79f77f83f..31637078d 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -24,8 +24,10 @@ class LastGenrePluginTest(unittest.TestCase):
         """Set up configuration"""
         lastgenre.LastGenrePlugin()
 
-    def _setup_config(self, whitelist=set(), branches=None, count=1):
-        lastgenre.options['whitelist'] = whitelist
+    def _setup_config(self, _whitelist=set(), _branches=None, count=1):
+
+        if _whitelist:
+            lastgenre.options['whitelist'] = _whitelist
         if branches:
             lastgenre.options['branches'] = branches
             lastgenre.options['c14n'] = True
@@ -73,6 +75,9 @@ class LastGenrePluginTest(unittest.TestCase):
                          ['jazz', 'pop', 'country blues', 'rock']),
                          'Jazz, Blues')
 
+    def test_default_c14n(self):
+        """c14n with default config files should work out-of-the-box
+        """
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

From 2e8e55736d35bfc3f8b64757a64db46619a4631e Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Fri, 25 Apr 2014 09:00:49 +0200
Subject: [PATCH 05/19] get rid of module level options variable

Conflicts:
	beetsplug/lastgenre/__init__.py
---
 beetsplug/lastgenre/__init__.py | 203 +++++++++++++++-----------------
 test/test_lastgenre.py          |  50 ++++----
 2 files changed, 117 insertions(+), 136 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index c4f55dafa..8952b1983 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -94,47 +94,6 @@ def filter_tags(tags):
     return res
 
 
-def _is_allowed(genre):
-    """Determine whether the genre is present in the whitelist,
-    returning a boolean.
-    """
-    if genre is None:
-        return False
-    if not options['whitelist'] or genre in options['whitelist']:
-        return True
-    return False
-
-
-def _strings_to_genre(tags):
-    """Given a list of strings, return a genre by joining them into a
-    single string and (optionally) canonicalizing each.
-    """
-    if not tags:
-        return None
-
-    if options.get('c14n'):
-        # Use the canonicalization tree.
-        out = []
-        for tag in tags:
-            for parent in find_parents(tag, options['branches']):
-                if _is_allowed(parent):
-                    out.append(parent)
-                    break
-        tags = out
-
-    tags = [t.title() for t in tags]
-    return config['lastgenre']['separator'].get(unicode).join(
-        tags[:config['lastgenre']['count'].get(int)]
-    )
-
-
-def fetch_genre(lastfm_obj):
-    """Return the genre for a pylast entity or None if no suitable genre
-    can be found. Ex. 'Electronic, House, Dance'
-    """
-    return _strings_to_genre(_tags_for(lastfm_obj))
-
-
 # Canonicalization tree processing.
 
 
@@ -168,63 +127,8 @@ def find_parents(candidate, branches):
     return [candidate]
 
 
-# Cached entity lookups.
-
-_genre_cache = {}
-
-
-def _cached_lookup(entity, method, *args):
-    """Get a genre based on the named entity using the callable `method`
-    whose arguments are given in the sequence `args`. The genre lookup
-    is cached based on the entity name and the arguments.
-    """
-    # Shortcut if we're missing metadata.
-    if any(not s for s in args):
-        return None
-
-    key = u'{0}.{1}'.format(entity, u'-'.join(unicode(a) for a in args))
-    if key in _genre_cache:
-        return _genre_cache[key]
-    else:
-        genre = fetch_genre(method(*args))
-        _genre_cache[key] = genre
-        return genre
-
-
-def fetch_album_genre(obj):
-    """Return the album genre for this Item or Album.
-    """
-    return _cached_lookup(u'album', LASTFM.get_album, obj.albumartist,
-                          obj.album)
-
-
-def fetch_album_artist_genre(obj):
-    """Return the album artist genre for this Item or Album.
-    """
-    return _cached_lookup(u'artist', LASTFM.get_artist, obj.albumartist)
-
-
-def fetch_artist_genre(item):
-    """Returns the track artist genre for this Item.
-    """
-    return _cached_lookup(u'artist', LASTFM.get_artist, item.artist)
-
-
-def fetch_track_genre(obj):
-    """Returns the track genre for this Item.
-    """
-    return _cached_lookup(u'track', LASTFM.get_track, obj.artist, obj.title)
-
-
 # Main plugin logic.
 
-options = {
-    'whitelist': None,
-    'branches': None,
-    'c14n': False,
-}
-
-
 class LastGenrePlugin(plugins.BeetsPlugin):
     def __init__(self):
         super(LastGenrePlugin, self).__init__()
@@ -241,32 +145,35 @@ class LastGenrePlugin(plugins.BeetsPlugin):
             'separator': u', ',
         })
 
+        self.setup()
+
+    def setup(self):
+        """Setup plugin from config options
+        """
         if self.config['auto']:
             self.import_stages = [self.imported]
 
+        self._genre_cache = {}
+
         # Read the whitelist file.
-        wl_filename = self.config['whitelist'].as_filename()
-        whitelist = set()
-        with open(wl_filename) as f:
+        self.whitelist  = set()
+        with open(self.config['whitelist'].as_filename()) as f:
             for line in f:
                 line = line.decode('utf8').strip().lower()
-                if line:
-                    whitelist.add(line)
-        options['whitelist'] = whitelist
+                if line and not line.startswith(u'#'):
+                    self.whitelist.add(line)
 
         # Read the genres tree for canonicalization if enabled.
+        self.c14n_branches = []
         c14n_filename = self.config['canonical'].get()
-        if c14n_filename is not None:
+        if c14n_filename :
             c14n_filename = c14n_filename.strip()
             if not c14n_filename:
                 c14n_filename = C14N_TREE
             c14n_filename = normpath(c14n_filename)
 
             genres_tree = yaml.load(open(c14n_filename, 'r'))
-            branches = []
-            flatten_tree(genres_tree, [], branches)
-            options['branches'] = branches
-            options['c14n'] = True
+            flatten_tree(genres_tree, [], self.c14n_branches)
 
     @property
     def sources(self):
@@ -281,6 +188,86 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         elif source == 'artist':
             return 'artist',
 
+    def _strings_to_genre(self, tags):
+        """Given a list of strings, return a genre by joining them into a
+        single string and (optionally) canonicalizing each.
+        """
+        if not tags:
+            return None
+
+        if self.c14n_branches:
+            # Use the canonicalization tree.
+            out = []
+            for tag in tags:
+                for parent in find_parents(tag, self.c14n_branches):
+                    if self._is_allowed(parent):
+                        out.append(parent)
+                        break
+            tags = out
+
+        tags = [t.title() for t in tags]
+        return config['lastgenre']['separator'].get(unicode).join(
+            tags[:config['lastgenre']['count'].get(int)]
+        )
+
+    def fetch_genre(self, lastfm_obj):
+        """Return the genre for a pylast entity or None if no suitable genre
+        can be found. Ex. 'Electronic, House, Dance'
+        """
+        return self._strings_to_genre(_tags_for(lastfm_obj))
+
+    def _is_allowed(self, genre):
+        """Determine whether the genre is present in the whitelist,
+        returning a boolean.
+        """
+        if genre is None:
+            return False
+        if not self.whitelist or genre in self.whitelist:
+            return True
+        return False
+
+    # Cached entity lookups.
+
+    def _cached_lookup(self, entity, method, *args):
+        """Get a genre based on the named entity using the callable `method`
+        whose arguments are given in the sequence `args`. The genre lookup
+        is cached based on the entity name and the arguments.
+        """
+        # Shortcut if we're missing metadata.
+        if any(not s for s in args):
+            return None
+
+        key = u'{0}.{1}'.format(entity, u'-'.join(unicode(a) for a in args))
+        if key in self._genre_cache:
+            return self._genre_cache[key]
+        else:
+            genre = fetch_genre(method(*args))
+            self._genre_cache[key] = genre
+            return genre
+
+    def fetch_album_genre(self, obj):
+        """Return the album genre for this Item or Album.
+        """
+        return self._cached_lookup(u'album', LASTFM.get_album, obj.albumartist,
+                              obj.album)
+
+    def fetch_album_artist_genre(obj):
+        """Return the album artist genre for this Item or Album.
+        """
+        return self._cached_lookup(u'artist', LASTFM.get_artist,
+                                   obj.albumartist)
+
+    def fetch_artist_genre(item):
+        """Returns the track artist genre for this Item.
+        """
+        return self._cached_lookup(u'artist', LASTFM.get_artist, item.artist)
+
+    def fetch_track_genre(obj):
+        """Returns the track genre for this Item.
+        """
+        return self._cached_lookup(u'track', LASTFM.get_track, obj.artist,
+                                   obj.title)
+
     def _get_genre(self, obj):
         """Get the genre string for an Album or Item object based on
         self.sources. Return a `(genre, source)` pair. The
@@ -335,7 +322,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         # Filter the existing genre.
         if obj.genre:
-            result = _strings_to_genre([obj.genre])
+            result = self.strings_to_genre([obj.genre])
             if result:
                 return result, 'original'
 
diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 31637078d..b51f27d49 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -14,45 +14,43 @@
 
 """Tests for the 'lastgenre' plugin."""
 
+import logging
 from _common import unittest
-from beetsplug import lastgenre
+from beetsplug.lastgenre import LastGenrePlugin
 from beets import config
 
+log = logging.getLogger('beets')
+
+lastGenrePlugin = LastGenrePlugin()
 
 class LastGenrePluginTest(unittest.TestCase):
-    def setUp(self):
-        """Set up configuration"""
-        lastgenre.LastGenrePlugin()
-
-    def _setup_config(self, _whitelist=set(), _branches=None, count=1):
-
-        if _whitelist:
-            lastgenre.options['whitelist'] = _whitelist
-        if branches:
-            lastgenre.options['branches'] = branches
-            lastgenre.options['c14n'] = True
-        else:
-            lastgenre.options['c14n'] = False
 
+    def _setup_config(self, whitelist=set(), canonical=None, count=1):
+        config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
 
+        if whitelist:
+            lastGenrePlugin.whitelist = whitelist
+
+
     def test_c14n(self):
         """Resolve genres that belong to a canonicalization branch.
         """
-        self._setup_config(whitelist=set(['blues']),
-                           branches=[['blues'], ['blues', 'country blues'],
-                                     ['blues', 'country blues',
-                                      'delta blues']])
+        # default whitelist and c14n
+        self._setup_config(canonical='')
+        self.assertEqual(lastGenrePlugin._strings_to_genre(['delta blues']), 'Blues')
+        self.assertEqual(lastGenrePlugin._strings_to_genre(['i got blues']), '')
 
-        self.assertEqual(lastgenre._strings_to_genre(['delta blues']), 'Blues')
-        self.assertEqual(lastgenre._strings_to_genre(['rhytm n blues']), '')
+        # custom whitelist
+        self._setup_config(canonical='', whitelist=set(['rock']))
+        self.assertEqual(lastGenrePlugin._strings_to_genre(['delta blues']), '')
 
     def test_whitelist(self):
         """Keep only genres that are in the whitelist.
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            count=2)
-        self.assertEqual(lastgenre._strings_to_genre(['pop', 'blues']),
+        self.assertEqual(lastGenrePlugin._strings_to_genre(['pop', 'blues']),
                          'Blues')
 
     def test_count(self):
@@ -61,7 +59,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            count=2)
-        self.assertEqual(lastgenre._strings_to_genre(
+        self.assertEqual(lastGenrePlugin._strings_to_genre(
                          ['jazz', 'pop', 'rock', 'blues']),
                          'Jazz, Rock')
 
@@ -69,16 +67,12 @@ class LastGenrePluginTest(unittest.TestCase):
         """Keep the n first genres, after having applied c14n when necessary
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
-                           branches=[['blues'], ['blues', 'country blues']],
+                           canonical='',
                            count=2)
-        self.assertEqual(lastgenre._strings_to_genre(
+        self.assertEqual(lastGenrePlugin._strings_to_genre(
                          ['jazz', 'pop', 'country blues', 'rock']),
                          'Jazz, Blues')
 
-    def test_default_c14n(self):
-        """c14n with default config files should work out-of-the-box
-        """
-
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 

From d0d3c18da257057db88276f3013333b43b21f8a7 Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Sat, 26 Apr 2014 00:27:56 +0200
Subject: [PATCH 06/19] lastgenre: rewrite filtering logic to make tests pass

- remove filter_tags() as genres should not be removed this soon while
c14n has not been applied
- group all filtering logic in the function _resolve_genres (formerly
_strings_to_genre)
---
 beetsplug/lastgenre/__init__.py | 79 +++++++++++++--------------------
 test/test_lastgenre.py          | 31 ++++++++-----
 2 files changed, 51 insertions(+), 59 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index 8952b1983..8f47d9a4b 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -66,31 +66,8 @@ def _tags_for(obj):
         log.debug(u'last.fm error: %s' % unicode(exc))
         return []
 
-    res = filter_tags(res)
-    return res
+    res = [el.item.get_name().lower() for el in res]
 
-def filter_tags(tags):
-    """Filter tags based on their weights and whitelist content
-    """
-
-    res = []
-    min_weight = config['lastgenre']['min_weight'].get(int)
-    count = config['lastgenre']['count'].get(int)
-
-    dbg = []
-    for el in tags:
-        weight = int(el.weight or 0)
-        tag = el.item.get_name().lower()
-        if _is_allowed(tag):
-            if min_weight > -1 and min_weight > weight and len(res) > 0:
-                return res
-            res.append(tag)
-            dbg.append(u'{0} [{1}]'.format(tag, weight))
-            if len(res) == count:
-                break
-    log.debug(u'lastfm.tag (min. {0}): {1}'.format(
-        min_weight, u', '.join(dbg)
-    ))
     return res
 
 
@@ -166,7 +143,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         # Read the genres tree for canonicalization if enabled.
         self.c14n_branches = []
         c14n_filename = self.config['canonical'].get()
-        if c14n_filename :
+        if c14n_filename is not None:
             c14n_filename = c14n_filename.strip()
             if not c14n_filename:
                 c14n_filename = C14N_TREE
@@ -188,24 +165,29 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         elif source == 'artist':
             return 'artist',
 
-    def _strings_to_genre(self, tags):
+    def _resolve_genres(self, tags):
         """Given a list of strings, return a genre by joining them into a
         single string and (optionally) canonicalizing each.
         """
         if not tags:
             return None
 
+        count = config['lastgenre']['count'].get(int)
         if self.c14n_branches:
-            # Use the canonicalization tree.
-            out = []
+            # Extend the list to consider tags parents in the c14n tree
+            tags_all = []
             for tag in tags:
-                for parent in find_parents(tag, self.c14n_branches):
-                    if self._is_allowed(parent):
-                        out.append(parent)
-                        break
-            tags = out
+                parents = [x for x in find_parents(tag, self.c14n_branches) if
+                           self._is_allowed(x)]
+                tags_all += parents
+                if len(tags_all) >= count:
+                    break
+            tags = tags_all
+
+        # c14n only adds allowed genres but we may have had forbidden genres in
+        # the original tags list
+        tags = [x.title() for x in tags if self._is_allowed(x)]
 
-        tags = [t.title() for t in tags]
         return config['lastgenre']['separator'].get(unicode).join(
             tags[:config['lastgenre']['count'].get(int)]
         )
@@ -214,7 +196,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         """Return the genre for a pylast entity or None if no suitable genre
         can be found. Ex. 'Electronic, House, Dance'
         """
-        return self._strings_to_genre(_tags_for(lastfm_obj))
+        return self._resolve_genres(_tags_for(lastfm_obj))
 
     def _is_allowed(self, genre):
         """Determine whether the genre is present in the whitelist,
@@ -241,7 +223,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         if key in self._genre_cache:
             return self._genre_cache[key]
         else:
-            genre = fetch_genre(method(*args))
+            genre = self.fetch_genre(method(*args))
             self._genre_cache[key] = genre
             return genre
 
@@ -249,20 +231,20 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         """Return the album genre for this Item or Album.
         """
         return self._cached_lookup(u'album', LASTFM.get_album, obj.albumartist,
-                              obj.album)
+                                   obj.album)
 
-    def fetch_album_artist_genre(obj):
+    def fetch_album_artist_genre(self, obj):
         """Return the album artist genre for this Item or Album.
         """
         return self._cached_lookup(u'artist', LASTFM.get_artist,
                                    obj.albumartist)
 
-    def fetch_artist_genre(item):
+    def fetch_artist_genre(self, item):
         """Returns the track artist genre for this Item.
         """
         return self._cached_lookup(u'artist', LASTFM.get_artist, item.artist)
 
-    def fetch_track_genre(obj):
+    def fetch_track_genre(self, obj):
         """Returns the track genre for this Item.
         """
         return self._cached_lookup(u'track', LASTFM.get_track, obj.artist,
@@ -279,20 +261,21 @@ class LastGenrePlugin(plugins.BeetsPlugin):
             - fallback
             - None
         """
+
         # Shortcut to existing genre if not forcing.
-        if not self.config['force'] and _is_allowed(obj.genre):
+        if not self.config['force'] and self._is_allowed(obj.genre):
             return obj.genre, 'keep'
 
         # Track genre (for Items only).
         if isinstance(obj, library.Item):
             if 'track' in self.sources:
-                result = fetch_track_genre(obj)
+                result = self.fetch_track_genre(obj)
                 if result:
                     return result, 'track'
 
         # Album genre.
         if 'album' in self.sources:
-            result = fetch_album_genre(obj)
+            result = self.fetch_album_genre(obj)
             if result:
                 return result, 'album'
 
@@ -300,18 +283,18 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         if 'artist' in self.sources:
             result = None
             if isinstance(obj, library.Item):
-                result = fetch_artist_genre(obj)
+                result = self.fetch_artist_genre(obj)
             elif obj.albumartist != 'Various Artists':
-                result = fetch_album_artist_genre(obj)
+                result = self.fetch_album_artist_genre(obj)
             else:
                 # For "Various Artists", pick the most popular track genre.
                 item_genres = []
                 for item in obj.items():
                     item_genre = None
                     if 'track' in self.sources:
-                        item_genre = fetch_track_genre(item)
+                        item_genre = self.fetch_track_genre(item)
                     if not item_genre:
-                        item_genre = fetch_artist_genre(item)
+                        item_genre = self.fetch_artist_genre(item)
                     if item_genre:
                         item_genres.append(item_genre)
                 if item_genres:
@@ -322,7 +305,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         # Filter the existing genre.
         if obj.genre:
-            result = self.strings_to_genre([obj.genre])
+            result = self._resolve_genres([obj.genre])
             if result:
                 return result, 'original'
 
diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index b51f27d49..84580a47d 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -15,42 +15,47 @@
 """Tests for the 'lastgenre' plugin."""
 
 import logging
+import os
 from _common import unittest
-from beetsplug.lastgenre import LastGenrePlugin
+from beetsplug import lastgenre
 from beets import config
 
 log = logging.getLogger('beets')
 
-lastGenrePlugin = LastGenrePlugin()
+lastGenrePlugin = lastgenre.LastGenrePlugin()
+
 
 class LastGenrePluginTest(unittest.TestCase):
 
     def _setup_config(self, whitelist=set(), canonical=None, count=1):
         config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
-
+        config['lastgenre']['whitelist'] = \
+            os.path.join(os.path.dirname(lastgenre.__file__), 'genres.txt')
+        lastGenrePlugin.setup()
         if whitelist:
             lastGenrePlugin.whitelist = whitelist
 
-
     def test_c14n(self):
         """Resolve genres that belong to a canonicalization branch.
         """
         # default whitelist and c14n
-        self._setup_config(canonical='')
-        self.assertEqual(lastGenrePlugin._strings_to_genre(['delta blues']), 'Blues')
-        self.assertEqual(lastGenrePlugin._strings_to_genre(['i got blues']), '')
+        self._setup_config(canonical=' ')
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         'Blues')
+        self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']), '')
 
         # custom whitelist
         self._setup_config(canonical='', whitelist=set(['rock']))
-        self.assertEqual(lastGenrePlugin._strings_to_genre(['delta blues']), '')
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         '')
 
     def test_whitelist(self):
         """Keep only genres that are in the whitelist.
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            count=2)
-        self.assertEqual(lastGenrePlugin._strings_to_genre(['pop', 'blues']),
+        self.assertEqual(lastGenrePlugin._resolve_genres(['pop', 'blues']),
                          'Blues')
 
     def test_count(self):
@@ -59,7 +64,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            count=2)
-        self.assertEqual(lastGenrePlugin._strings_to_genre(
+        self.assertEqual(lastGenrePlugin._resolve_genres(
                          ['jazz', 'pop', 'rock', 'blues']),
                          'Jazz, Rock')
 
@@ -69,10 +74,14 @@ class LastGenrePluginTest(unittest.TestCase):
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            canonical='',
                            count=2)
-        self.assertEqual(lastGenrePlugin._strings_to_genre(
+
+        # thanks to c14n, 'blues' superseeds 'country blues' and takes the
+        # second slot
+        self.assertEqual(lastGenrePlugin._resolve_genres(
                          ['jazz', 'pop', 'country blues', 'rock']),
                          'Jazz, Blues')
 
+
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 

From e7af3319f81ebb8929404461f4004368eb923084 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sat, 26 Apr 2014 20:59:39 -0700
Subject: [PATCH 07/19] lastgenre: Restore min_weight option

---
 beetsplug/lastgenre/__init__.py | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index 8f47d9a4b..44ddd78ff 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -49,10 +49,12 @@ PYLAST_EXCEPTIONS = (
 
 # Core genre identification routine.
 
-def _tags_for(obj):
-    """Given a pylast entity (album or track), returns a list of
-    tag names for that entity. Returns an empty list if the entity is
+def _tags_for(obj, min_weight=None):
+    """Given a pylast entity (album or track), return a list of
+    tag names for that entity. Return an empty list if the entity is
     not found or another error occurs.
+
+    If `min_weight` is specified, tags are filtered by weight.
     """
     try:
         # Work around an inconsistency in pylast where
@@ -66,6 +68,11 @@ def _tags_for(obj):
         log.debug(u'last.fm error: %s' % unicode(exc))
         return []
 
+    # Filter by weight (optionally).
+    if min_weight:
+        res = [el for el in res if (el.weight or 0) >= min_weight]
+
+    # Get strings from tags.
     res = [el.item.get_name().lower() for el in res]
 
     return res
@@ -133,7 +140,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         self._genre_cache = {}
 
         # Read the whitelist file.
-        self.whitelist  = set()
+        self.whitelist = set()
         with open(self.config['whitelist'].as_filename()) as f:
             for line in f:
                 line = line.decode('utf8').strip().lower()
@@ -196,7 +203,8 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         """Return the genre for a pylast entity or None if no suitable genre
         can be found. Ex. 'Electronic, House, Dance'
         """
-        return self._resolve_genres(_tags_for(lastfm_obj))
+        min_weight = self.config['min_weight'].get(int)
+        return self._resolve_genres(_tags_for(lastfm_obj, min_weight))
 
     def _is_allowed(self, genre):
         """Determine whether the genre is present in the whitelist,

From 35b6602795c7fdd71dec09f311a8a6b8a6576414 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sat, 26 Apr 2014 21:00:59 -0700
Subject: [PATCH 08/19] Use self.config instead of global config

---
 beetsplug/lastgenre/__init__.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index 44ddd78ff..dcf5b2fe2 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -179,7 +179,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         if not tags:
             return None
 
-        count = config['lastgenre']['count'].get(int)
+        count = self.config['count'].get(int)
         if self.c14n_branches:
             # Extend the list to consider tags parents in the c14n tree
             tags_all = []
@@ -195,8 +195,8 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         # the original tags list
         tags = [x.title() for x in tags if self._is_allowed(x)]
 
-        return config['lastgenre']['separator'].get(unicode).join(
-            tags[:config['lastgenre']['count'].get(int)]
+        return self.config['separator'].get(unicode).join(
+            tags[:self.config['count'].get(int)]
         )
 
     def fetch_genre(self, lastfm_obj):

From e399173f7e5e02715d2bd98de318e276158987eb Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Sun, 27 Apr 2014 15:59:15 +0200
Subject: [PATCH 09/19] add a test that would fail with the restricted
 whitelist approach

If no option is set, valid genres should not be rejected. Which is an argument
to have a wide default whitelist.
---
 test/test_lastgenre.py | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 84580a47d..b4fa9cc4d 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -36,19 +36,19 @@ class LastGenrePluginTest(unittest.TestCase):
         if whitelist:
             lastGenrePlugin.whitelist = whitelist
 
-    def test_c14n(self):
-        """Resolve genres that belong to a canonicalization branch.
+    def test_defaults(self):
+        """Tests whitelist and c14n options with default filepaths
         """
         # default whitelist and c14n
-        self._setup_config(canonical=' ')
+        self._setup_config(canonical='')
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
                          'Blues')
         self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']), '')
 
-        # custom whitelist
-        self._setup_config(canonical='', whitelist=set(['rock']))
+        # default whitelist and no c14n
+        self._setup_config()
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
-                         '')
+                         'delta blues')
 
     def test_whitelist(self):
         """Keep only genres that are in the whitelist.
@@ -58,6 +58,10 @@ class LastGenrePluginTest(unittest.TestCase):
         self.assertEqual(lastGenrePlugin._resolve_genres(['pop', 'blues']),
                          'Blues')
 
+        self._setup_config(canonical='', whitelist=set(['rock']))
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         '')
+
     def test_count(self):
         """Keep the n first genres, as we expect them to be sorted from more to
         less popular.

From e442749cd4dff1de3b0d320e0010f2af63f95555 Mon Sep 17 00:00:00 2001
From: "Fabrice L." <kraymer@gmail.com>
Date: Sun, 27 Apr 2014 16:05:16 +0200
Subject: [PATCH 10/19] add a test that would fail with the restricted
 whitelist approach

If no option is set, valid genres should not be rejected. Which is an argument
to have a wide default whitelist.
---
 test/test_lastgenre.py | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 84580a47d..b4fa9cc4d 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -36,19 +36,19 @@ class LastGenrePluginTest(unittest.TestCase):
         if whitelist:
             lastGenrePlugin.whitelist = whitelist
 
-    def test_c14n(self):
-        """Resolve genres that belong to a canonicalization branch.
+    def test_defaults(self):
+        """Tests whitelist and c14n options with default filepaths
         """
         # default whitelist and c14n
-        self._setup_config(canonical=' ')
+        self._setup_config(canonical='')
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
                          'Blues')
         self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']), '')
 
-        # custom whitelist
-        self._setup_config(canonical='', whitelist=set(['rock']))
+        # default whitelist and no c14n
+        self._setup_config()
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
-                         '')
+                         'delta blues')
 
     def test_whitelist(self):
         """Keep only genres that are in the whitelist.
@@ -58,6 +58,10 @@ class LastGenrePluginTest(unittest.TestCase):
         self.assertEqual(lastGenrePlugin._resolve_genres(['pop', 'blues']),
                          'Blues')
 
+        self._setup_config(canonical='', whitelist=set(['rock']))
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         '')
+
     def test_count(self):
         """Keep the n first genres, as we expect them to be sorted from more to
         less popular.

From 817b8cc96e07d9dee01b81fe658d0222051e557b Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Sun, 27 Apr 2014 22:26:09 +0200
Subject: [PATCH 11/19] Allowing the whitelist to be disabled

By default it is disabled, setting the value to the empty
string will use the built-in whitelist (same behaviour than
c14n).
---
 beetsplug/lastgenre/__init__.py | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index dcf5b2fe2..fb68fb247 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -38,7 +38,6 @@ from beets import library
 log = logging.getLogger('beets')
 
 LASTFM = pylast.LastFMNetwork(api_key=plugins.LASTFM_KEY)
-C14N_TREE = os.path.join(os.path.dirname(__file__), 'genres-tree.yaml')
 
 PYLAST_EXCEPTIONS = (
     pylast.WSError,
@@ -113,12 +112,16 @@ def find_parents(candidate, branches):
 
 # Main plugin logic.
 
+WHITELIST = os.path.join(os.path.dirname(__file__), 'genres.txt')
+C14N_TREE = os.path.join(os.path.dirname(__file__), 'genres-tree.yaml')
+
+
 class LastGenrePlugin(plugins.BeetsPlugin):
     def __init__(self):
         super(LastGenrePlugin, self).__init__()
 
         self.config.add({
-            'whitelist': os.path.join(os.path.dirname(__file__), 'genres.txt'),
+            'whitelist': None,
             'min_weight': 10,
             'count': 1,
             'fallback': None,
@@ -139,13 +142,20 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         self._genre_cache = {}
 
-        # Read the whitelist file.
+        # Read the whitelist file if enabled.
         self.whitelist = set()
-        with open(self.config['whitelist'].as_filename()) as f:
-            for line in f:
-                line = line.decode('utf8').strip().lower()
-                if line and not line.startswith(u'#'):
-                    self.whitelist.add(line)
+        wl_filename = self.config['whitelist'].get()
+        if wl_filename is not None:
+            wl_filename = wl_filename.strip()
+            if not wl_filename:
+                wl_filename = WHITELIST
+            wl_filename = normpath(wl_filename)
+            with open(wl_filename, 'r') as f:
+                # with open(self.config['whitelist'].as_filename()) as f:
+                for line in f:
+                    line = line.decode('utf8').strip().lower()
+                    if line and not line.startswith(u'#'):
+                        self.whitelist.add(line)
 
         # Read the genres tree for canonicalization if enabled.
         self.c14n_branches = []

From 0b3c5ddbe0535662a543511eab9b0f545a2e672e Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Sun, 27 Apr 2014 22:27:20 +0200
Subject: [PATCH 12/19] Canonicalization only return most common genre

---
 beetsplug/lastgenre/__init__.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index fb68fb247..581772519 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -194,13 +194,18 @@ class LastGenrePlugin(plugins.BeetsPlugin):
             # Extend the list to consider tags parents in the c14n tree
             tags_all = []
             for tag in tags:
-                parents = [x for x in find_parents(tag, self.c14n_branches) if
-                           self._is_allowed(x)]
+                # Add parents that are in the whitelist, or add the oldest
+                # ancestor if no whitelist
+                if self.whitelist:
+                    parents = [x for x in find_parents(tag, self.c14n_branches)
+                               if self._is_allowed(x)]
+                else:
+                    parents = [find_parents(tag, self.c14n_branches)[-1]]
+
                 tags_all += parents
                 if len(tags_all) >= count:
                     break
             tags = tags_all
-
         # c14n only adds allowed genres but we may have had forbidden genres in
         # the original tags list
         tags = [x.title() for x in tags if self._is_allowed(x)]

From 0613cb7d138cdb61f86f199bf52678d70befbaf0 Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Sun, 27 Apr 2014 22:31:31 +0200
Subject: [PATCH 13/19] Update _setup_config

Give an empty string to obtain default whitelist, None for no whitelist
or a stringlist for a custom one.
---
 test/test_lastgenre.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index b4fa9cc4d..fb2952799 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -27,11 +27,11 @@ lastGenrePlugin = lastgenre.LastGenrePlugin()
 
 class LastGenrePluginTest(unittest.TestCase):
 
-    def _setup_config(self, whitelist=set(), canonical=None, count=1):
+    def _setup_config(self, whitelist=None, canonical=None, count=1):
         config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
-        config['lastgenre']['whitelist'] = \
-            os.path.join(os.path.dirname(lastgenre.__file__), 'genres.txt')
+        if whitelist == '':
+            config['lastgenre']['whitelist'] = whitelist
         lastGenrePlugin.setup()
         if whitelist:
             lastGenrePlugin.whitelist = whitelist

From d1cc95984e7ac2b8f96beaa75b380fffe1efbe7e Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Sun, 27 Apr 2014 22:32:31 +0200
Subject: [PATCH 14/19] Rename/add tests

Rewrite tests now that empty whitelist is allowed.
---
 test/test_lastgenre.py | 52 +++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index fb2952799..635acbe2c 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -36,21 +36,39 @@ class LastGenrePluginTest(unittest.TestCase):
         if whitelist:
             lastGenrePlugin.whitelist = whitelist
 
-    def test_defaults(self):
-        """Tests whitelist and c14n options with default filepaths
+    def test_default(self):
+        """Fetch genres with whitelist and c14n deactivated
         """
-        # default whitelist and c14n
-        self._setup_config(canonical='')
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
-                         'Blues')
-        self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']), '')
-
-        # default whitelist and no c14n
         self._setup_config()
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
-                         'delta blues')
+                         'Delta Blues')
 
-    def test_whitelist(self):
+    def test_c14n_only(self):
+        """Default c14n tree funnels up to most common genre except for *wrong*
+        genres that stay unchanged.
+        """
+        self._setup_config(canonical='', count=99)
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         'Blues')
+        self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']),
+                         'Iota Blues')
+
+    def test_whitelist_only(self):
+        """Default whitelist rejects *wrong* (non existing) genres.
+        """
+        self._setup_config(whitelist='')
+        self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']),
+                         '')
+
+    def test_whitelist_c14n(self):
+        """Default whitelist and c14n both activated result in all parents
+        genres being selected (from specific to common).
+        """
+        self._setup_config(canonical='', whitelist='', count=99)
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         'Delta Blues, Country Blues, Blues')
+
+    def test_whitelist_custom(self):
         """Keep only genres that are in the whitelist.
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
@@ -58,10 +76,6 @@ class LastGenrePluginTest(unittest.TestCase):
         self.assertEqual(lastGenrePlugin._resolve_genres(['pop', 'blues']),
                          'Blues')
 
-        self._setup_config(canonical='', whitelist=set(['rock']))
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
-                         '')
-
     def test_count(self):
         """Keep the n first genres, as we expect them to be sorted from more to
         less popular.
@@ -78,13 +92,19 @@ class LastGenrePluginTest(unittest.TestCase):
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            canonical='',
                            count=2)
-
         # thanks to c14n, 'blues' superseeds 'country blues' and takes the
         # second slot
         self.assertEqual(lastGenrePlugin._resolve_genres(
                          ['jazz', 'pop', 'country blues', 'rock']),
                          'Jazz, Blues')
 
+    def test_c14n_whitelist(self):
+        """Genres first pass through c14n and are then filtered
+        """
+        self._setup_config(canonical='', whitelist=set(['rock']))
+        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+                         '')
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

From d5dbaeff7ab9b0d84210e61bd07804c482ef065e Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 27 Apr 2014 13:54:32 -0700
Subject: [PATCH 15/19] lastgenre: Restore default whitelist

---
 beetsplug/lastgenre/__init__.py | 2 +-
 test/test_lastgenre.py          | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index 581772519..d15850d2e 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -121,7 +121,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         super(LastGenrePlugin, self).__init__()
 
         self.config.add({
-            'whitelist': None,
+            'whitelist': WHITELIST,
             'min_weight': 10,
             'count': 1,
             'fallback': None,
diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 89fb6eec9..047773f31 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -15,7 +15,6 @@
 """Tests for the 'lastgenre' plugin."""
 
 import logging
-import os
 from _common import unittest
 from beetsplug import lastgenre
 from beets import config
@@ -30,7 +29,7 @@ class LastGenrePluginTest(unittest.TestCase):
     def _setup_config(self, whitelist=None, canonical=None, count=1):
         config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
-        if whitelist == '':
+        if not whitelist:  # Either None or default ('').
             config['lastgenre']['whitelist'] = whitelist
         lastGenrePlugin.setup()
         if whitelist:

From ef3c1cd1ff52c01667a4ad95445d404518a6b514 Mon Sep 17 00:00:00 2001
From: Fabrice Laporte <kraymer@gmail.com>
Date: Mon, 28 Apr 2014 10:31:22 +0200
Subject: [PATCH 16/19] use booleans for whitelist and c14n options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

homogenise options setup using booleans, while keeping the empty string
(synonym of ‘true’) for backward compatibility.
---
 beetsplug/lastgenre/__init__.py | 18 +++++++-----------
 test/test_lastgenre.py          | 16 ++++++++--------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index d15850d2e..7ee2024b3 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -125,7 +125,7 @@ class LastGenrePlugin(plugins.BeetsPlugin):
             'min_weight': 10,
             'count': 1,
             'fallback': None,
-            'canonical': None,
+            'canonical': 'false',
             'source': 'album',
             'force': True,
             'auto': True,
@@ -144,14 +144,12 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         # Read the whitelist file if enabled.
         self.whitelist = set()
-        wl_filename = self.config['whitelist'].get()
-        if wl_filename is not None:
-            wl_filename = wl_filename.strip()
-            if not wl_filename:
+        wl_filename = self.config['whitelist'].get().strip()
+        if wl_filename != 'false':
+            if wl_filename in ('true', ''):
                 wl_filename = WHITELIST
             wl_filename = normpath(wl_filename)
             with open(wl_filename, 'r') as f:
-                # with open(self.config['whitelist'].as_filename()) as f:
                 for line in f:
                     line = line.decode('utf8').strip().lower()
                     if line and not line.startswith(u'#'):
@@ -159,13 +157,11 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         # Read the genres tree for canonicalization if enabled.
         self.c14n_branches = []
-        c14n_filename = self.config['canonical'].get()
-        if c14n_filename is not None:
-            c14n_filename = c14n_filename.strip()
-            if not c14n_filename:
+        c14n_filename = self.config['canonical'].get().strip()
+        if c14n_filename != 'false':
+            if c14n_filename in ('true', ''):
                 c14n_filename = C14N_TREE
             c14n_filename = normpath(c14n_filename)
-
             genres_tree = yaml.load(open(c14n_filename, 'r'))
             flatten_tree(genres_tree, [], self.c14n_branches)
 
diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 047773f31..910f85f0a 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -26,13 +26,13 @@ lastGenrePlugin = lastgenre.LastGenrePlugin()
 
 class LastGenrePluginTest(unittest.TestCase):
 
-    def _setup_config(self, whitelist=None, canonical=None, count=1):
+    def _setup_config(self, whitelist='false', canonical='false', count=1):
         config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
-        if not whitelist:  # Either None or default ('').
+        if whitelist in ('true', 'false'):
             config['lastgenre']['whitelist'] = whitelist
         lastGenrePlugin.setup()
-        if whitelist:
+        if whitelist not in ('true', 'false'):
             lastGenrePlugin.whitelist = whitelist
 
     def test_default(self):
@@ -46,7 +46,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """Default c14n tree funnels up to most common genre except for *wrong*
         genres that stay unchanged.
         """
-        self._setup_config(canonical='', count=99)
+        self._setup_config(canonical='true', count=99)
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
                          'Blues')
         self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']),
@@ -55,7 +55,7 @@ class LastGenrePluginTest(unittest.TestCase):
     def test_whitelist_only(self):
         """Default whitelist rejects *wrong* (non existing) genres.
         """
-        self._setup_config(whitelist='')
+        self._setup_config(whitelist='true')
         self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']),
                          '')
 
@@ -63,7 +63,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """Default whitelist and c14n both activated result in all parents
         genres being selected (from specific to common).
         """
-        self._setup_config(canonical='', whitelist='', count=99)
+        self._setup_config(canonical='true', whitelist='true', count=99)
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
                          'Delta Blues, Country Blues, Blues')
 
@@ -93,7 +93,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """Keep the n first genres, after having applied c14n when necessary
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
-                           canonical='',
+                           canonical='true',
                            count=2)
         # thanks to c14n, 'blues' superseeds 'country blues' and takes the
         # second slot
@@ -104,7 +104,7 @@ class LastGenrePluginTest(unittest.TestCase):
     def test_c14n_whitelist(self):
         """Genres first pass through c14n and are then filtered
         """
-        self._setup_config(canonical='', whitelist=set(['rock']))
+        self._setup_config(canonical='true', whitelist=set(['rock']))
         self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
                          '')
 

From f6337d2d1f64cf1e95113f34964e7eab5de1aeb9 Mon Sep 17 00:00:00 2001
From: "Fabrice L." <kraymer@gmail.com>
Date: Mon, 28 Apr 2014 10:44:53 +0200
Subject: [PATCH 17/19] Update lastgenre.rst

---
 docs/plugins/lastgenre.rst | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst
index 7a4d19a51..eb3b6210c 100644
--- a/docs/plugins/lastgenre.rst
+++ b/docs/plugins/lastgenre.rst
@@ -27,6 +27,8 @@ internal whitelist, but you can set your own in the config file using the
     lastgenre:
         whitelist: /path/to/genres.txt
 
+... or go for no whitelist altogether by setting it to "false".  
+
 The genre list file should contain one genre per line. Blank lines are ignored.
 For the curious, the default genre list is generated by a `script that scrapes
 Wikipedia`_.
@@ -58,9 +60,9 @@ leaves of the tree represent the most specific genres.
 To enable canonicalization, set the ``canonical`` configuration value::
 
     lastgenre:
-        canonical: ''
+        canonical: 'true'
 
-Setting this value to the empty string will use a built-in canonicalization
+Setting this value to 'true' will use a built-in canonicalization
 tree. You can also set it to a path, just like the ``whitelist`` config value,
 to use your own tree.
 
@@ -108,9 +110,6 @@ the ``min_weight`` config option::
     lastgenre:
         min_weight: 15
 
-However, if no tag with a weight greater then ``min_weight`` is found, the
-plugin uses the next-most-popular tag.
-
 
 Running Manually
 ----------------

From add309b57d81701077b42f178abcf2be655b1d39 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 28 Apr 2014 22:35:32 -0700
Subject: [PATCH 18/19] Isolate lastgenre tests

We now appropriately set up and tear down the fixture for the lastgenre tests.
(This was causing unpredictable failures elsewhere before.)
---
 test/test_lastgenre.py | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index 910f85f0a..a6cf537a8 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -14,32 +14,35 @@
 
 """Tests for the 'lastgenre' plugin."""
 
-import logging
 from _common import unittest
 from beetsplug import lastgenre
 from beets import config
 
-log = logging.getLogger('beets')
-
-lastGenrePlugin = lastgenre.LastGenrePlugin()
+from helper import TestHelper
 
 
-class LastGenrePluginTest(unittest.TestCase):
+class LastGenrePluginTest(unittest.TestCase, TestHelper):
+    def setUp(self):
+        self.setup_beets()
+        self.plugin = lastgenre.LastGenrePlugin()
+
+    def tearDown(self):
+        self.teardown_beets()
 
     def _setup_config(self, whitelist='false', canonical='false', count=1):
         config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
         if whitelist in ('true', 'false'):
             config['lastgenre']['whitelist'] = whitelist
-        lastGenrePlugin.setup()
+        self.plugin.setup()
         if whitelist not in ('true', 'false'):
-            lastGenrePlugin.whitelist = whitelist
+            self.plugin.whitelist = whitelist
 
     def test_default(self):
         """Fetch genres with whitelist and c14n deactivated
         """
         self._setup_config()
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+        self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          'Delta Blues')
 
     def test_c14n_only(self):
@@ -47,16 +50,16 @@ class LastGenrePluginTest(unittest.TestCase):
         genres that stay unchanged.
         """
         self._setup_config(canonical='true', count=99)
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+        self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          'Blues')
-        self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']),
+        self.assertEqual(self.plugin._resolve_genres(['iota blues']),
                          'Iota Blues')
 
     def test_whitelist_only(self):
         """Default whitelist rejects *wrong* (non existing) genres.
         """
         self._setup_config(whitelist='true')
-        self.assertEqual(lastGenrePlugin._resolve_genres(['iota blues']),
+        self.assertEqual(self.plugin._resolve_genres(['iota blues']),
                          '')
 
     def test_whitelist_c14n(self):
@@ -64,7 +67,7 @@ class LastGenrePluginTest(unittest.TestCase):
         genres being selected (from specific to common).
         """
         self._setup_config(canonical='true', whitelist='true', count=99)
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+        self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          'Delta Blues, Country Blues, Blues')
 
     def test_whitelist_custom(self):
@@ -72,11 +75,11 @@ class LastGenrePluginTest(unittest.TestCase):
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            count=2)
-        self.assertEqual(lastGenrePlugin._resolve_genres(['pop', 'blues']),
+        self.assertEqual(self.plugin._resolve_genres(['pop', 'blues']),
                          'Blues')
 
         self._setup_config(canonical='', whitelist=set(['rock']))
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+        self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          '')
 
     def test_count(self):
@@ -85,7 +88,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
                            count=2)
-        self.assertEqual(lastGenrePlugin._resolve_genres(
+        self.assertEqual(self.plugin._resolve_genres(
                          ['jazz', 'pop', 'rock', 'blues']),
                          'Jazz, Rock')
 
@@ -97,7 +100,7 @@ class LastGenrePluginTest(unittest.TestCase):
                            count=2)
         # thanks to c14n, 'blues' superseeds 'country blues' and takes the
         # second slot
-        self.assertEqual(lastGenrePlugin._resolve_genres(
+        self.assertEqual(self.plugin._resolve_genres(
                          ['jazz', 'pop', 'country blues', 'rock']),
                          'Jazz, Blues')
 
@@ -105,7 +108,7 @@ class LastGenrePluginTest(unittest.TestCase):
         """Genres first pass through c14n and are then filtered
         """
         self._setup_config(canonical='true', whitelist=set(['rock']))
-        self.assertEqual(lastGenrePlugin._resolve_genres(['delta blues']),
+        self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          '')
 
 

From 233f71a4574b5a10d1182a3a63ce5cd23c4268f4 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 28 Apr 2014 23:00:00 -0700
Subject: [PATCH 19/19] Use real booleans for whitelist/canonical options

As opposed to the strings "true" and "false".
---
 beetsplug/lastgenre/__init__.py | 25 ++++++++++--------------
 docs/plugins/lastgenre.rst      |  8 ++++----
 test/test_lastgenre.py          | 34 +++++++++++++++++++++++++--------
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py
index 7ee2024b3..1d1d59fb5 100644
--- a/beetsplug/lastgenre/__init__.py
+++ b/beetsplug/lastgenre/__init__.py
@@ -15,11 +15,6 @@
 """Gets genres for imported music based on Last.fm tags.
 
 Uses a provided whitelist file to determine which tags are valid genres.
-The genre whitelist can be specified like so in .beetsconfig:
-
-    [lastgenre]
-    whitelist=/path/to/genres.txt
-
 The included (default) genre list was produced by scraping Wikipedia.
 The scraper script used is available here:
 https://gist.github.com/1241307
@@ -121,11 +116,11 @@ class LastGenrePlugin(plugins.BeetsPlugin):
         super(LastGenrePlugin, self).__init__()
 
         self.config.add({
-            'whitelist': WHITELIST,
+            'whitelist': True,
             'min_weight': 10,
             'count': 1,
             'fallback': None,
-            'canonical': 'false',
+            'canonical': False,
             'source': 'album',
             'force': True,
             'auto': True,
@@ -144,10 +139,10 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         # Read the whitelist file if enabled.
         self.whitelist = set()
-        wl_filename = self.config['whitelist'].get().strip()
-        if wl_filename != 'false':
-            if wl_filename in ('true', ''):
-                wl_filename = WHITELIST
+        wl_filename = self.config['whitelist'].get()
+        if wl_filename in (True, ''):  # Indicates the default whitelist.
+            wl_filename = WHITELIST
+        if wl_filename:
             wl_filename = normpath(wl_filename)
             with open(wl_filename, 'r') as f:
                 for line in f:
@@ -157,10 +152,10 @@ class LastGenrePlugin(plugins.BeetsPlugin):
 
         # Read the genres tree for canonicalization if enabled.
         self.c14n_branches = []
-        c14n_filename = self.config['canonical'].get().strip()
-        if c14n_filename != 'false':
-            if c14n_filename in ('true', ''):
-                c14n_filename = C14N_TREE
+        c14n_filename = self.config['canonical'].get()
+        if c14n_filename in (True, ''):  # Default tree.
+            c14n_filename = C14N_TREE
+        if c14n_filename:
             c14n_filename = normpath(c14n_filename)
             genres_tree = yaml.load(open(c14n_filename, 'r'))
             flatten_tree(genres_tree, [], self.c14n_branches)
diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst
index eb3b6210c..58f3432be 100644
--- a/docs/plugins/lastgenre.rst
+++ b/docs/plugins/lastgenre.rst
@@ -27,7 +27,7 @@ internal whitelist, but you can set your own in the config file using the
     lastgenre:
         whitelist: /path/to/genres.txt
 
-... or go for no whitelist altogether by setting it to "false".  
+…or go for no whitelist altogether by setting the option to `false`.
 
 The genre list file should contain one genre per line. Blank lines are ignored.
 For the curious, the default genre list is generated by a `script that scrapes
@@ -39,7 +39,7 @@ Wikipedia`_.
 
 By default, beets will always fetch new genres, even if the files already have
 once. To instead leave genres in place in when they pass the whitelist, set
-the ``force`` option to "no".
+the ``force`` option to `no`.
 
 If no genre is found, the file will be left unchanged. To instead specify a
 fallback genre, use the ``fallback`` configuration option. You can, of
@@ -60,9 +60,9 @@ leaves of the tree represent the most specific genres.
 To enable canonicalization, set the ``canonical`` configuration value::
 
     lastgenre:
-        canonical: 'true'
+        canonical: true
 
-Setting this value to 'true' will use a built-in canonicalization
+Setting this value to `true` will use a built-in canonicalization
 tree. You can also set it to a path, just like the ``whitelist`` config value,
 to use your own tree.
 
diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py
index a6cf537a8..8fa5dc217 100644
--- a/test/test_lastgenre.py
+++ b/test/test_lastgenre.py
@@ -29,13 +29,15 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper):
     def tearDown(self):
         self.teardown_beets()
 
-    def _setup_config(self, whitelist='false', canonical='false', count=1):
+    def _setup_config(self, whitelist=False, canonical=False, count=1):
         config['lastgenre']['canonical'] = canonical
         config['lastgenre']['count'] = count
-        if whitelist in ('true', 'false'):
+        if isinstance(whitelist, (bool, basestring)):
+            # Filename, default, or disabled.
             config['lastgenre']['whitelist'] = whitelist
         self.plugin.setup()
-        if whitelist not in ('true', 'false'):
+        if not isinstance(whitelist, (bool, basestring)):
+            # Explicit list of genres.
             self.plugin.whitelist = whitelist
 
     def test_default(self):
@@ -49,7 +51,7 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper):
         """Default c14n tree funnels up to most common genre except for *wrong*
         genres that stay unchanged.
         """
-        self._setup_config(canonical='true', count=99)
+        self._setup_config(canonical=True, count=99)
         self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          'Blues')
         self.assertEqual(self.plugin._resolve_genres(['iota blues']),
@@ -58,7 +60,7 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper):
     def test_whitelist_only(self):
         """Default whitelist rejects *wrong* (non existing) genres.
         """
-        self._setup_config(whitelist='true')
+        self._setup_config(whitelist=True)
         self.assertEqual(self.plugin._resolve_genres(['iota blues']),
                          '')
 
@@ -66,7 +68,7 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper):
         """Default whitelist and c14n both activated result in all parents
         genres being selected (from specific to common).
         """
-        self._setup_config(canonical='true', whitelist='true', count=99)
+        self._setup_config(canonical=True, whitelist=True, count=99)
         self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          'Delta Blues, Country Blues, Blues')
 
@@ -96,7 +98,7 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper):
         """Keep the n first genres, after having applied c14n when necessary
         """
         self._setup_config(whitelist=set(['blues', 'rock', 'jazz']),
-                           canonical='true',
+                           canonical=True,
                            count=2)
         # thanks to c14n, 'blues' superseeds 'country blues' and takes the
         # second slot
@@ -107,10 +109,26 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper):
     def test_c14n_whitelist(self):
         """Genres first pass through c14n and are then filtered
         """
-        self._setup_config(canonical='true', whitelist=set(['rock']))
+        self._setup_config(canonical=True, whitelist=set(['rock']))
         self.assertEqual(self.plugin._resolve_genres(['delta blues']),
                          '')
 
+    def test_empty_string_enables_canonical(self):
+        """For backwards compatibility, setting the `canonical` option
+        to the empty string enables it using the default tree.
+        """
+        self._setup_config(canonical='', count=99)
+        self.assertEqual(self.plugin._resolve_genres(['delta blues']),
+                         'Blues')
+
+    def test_empty_string_enables_whitelist(self):
+        """Again for backwards compatibility, setting the `whitelist`
+        option to the empty string enables the default set of genres.
+        """
+        self._setup_config(whitelist='')
+        self.assertEqual(self.plugin._resolve_genres(['iota blues']),
+                         '')
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)