From 233f71a4574b5a10d1182a3a63ce5cd23c4268f4 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Mon, 28 Apr 2014 23:00:00 -0700 Subject: [PATCH] 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__)