From cef9a3311c6ba169bcd1f63dacb9dcf58e1e0ddf Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 30 Jun 2018 22:27:29 +0200 Subject: [PATCH 1/7] LastGenre: allow prefer_specific without canonical This improves lastgenre's behaviour when the configuration option `prefer_specific` is set but `canonical` is not. Previously it would not set any tags. Now it does apply tags, sorted using the canonicalization tree, but not canonicalized. For this the default tree is loaded even when `canonical` is not set. An extra check is added to only use it for canonicalization when `canonical` is set. --- beetsplug/lastgenre/__init__.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 4374310ba..6c195f6c4 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -138,9 +138,18 @@ class LastGenrePlugin(plugins.BeetsPlugin): # Read the genres tree for canonicalization if enabled. self.c14n_branches = [] c14n_filename = self.config['canonical'].get() - if c14n_filename in (True, ''): # Default tree. + self.canonicalize = c14n_filename is not False + + # Default tree + if c14n_filename in (True, ''): c14n_filename = C14N_TREE + elif not self.canonicalize and self.config['prefer_specific'].get(): + # prefer_specific requires a tree, load default tree + c14n_filename = C14N_TREE + + # Read the tree if c14n_filename: + self._log.debug('Loading canonicalization tree {0}', c14n_filename) c14n_filename = normpath(c14n_filename) with codecs.open(c14n_filename, 'r', encoding='utf-8') as f: genres_tree = yaml.load(f) @@ -186,7 +195,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): return None count = self.config['count'].get(int) - if self.c14n_branches: + if self.c14n_branches and self.canonicalize: # Extend the list to consider tags parents in the c14n tree tags_all = [] for tag in tags: From 0f847aefcbb443f39b9898c2e9a196aefa146e75 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 30 Jun 2018 22:37:28 +0200 Subject: [PATCH 2/7] LastGenre tests: setup_config: add prefer_specific Allow to specify prefer_specific in lastgenre tests configuration setup. --- test/test_lastgenre.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py index a70c65ca1..6c7e12261 100644 --- a/test/test_lastgenre.py +++ b/test/test_lastgenre.py @@ -36,9 +36,11 @@ 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, + prefer_specific=False): config['lastgenre']['canonical'] = canonical config['lastgenre']['count'] = count + config['lastgenre']['prefer_specific'] = prefer_specific if isinstance(whitelist, (bool, six.string_types)): # Filename, default, or disabled. config['lastgenre']['whitelist'] = whitelist From 96120d5c6d2f0542945df6ea8ed4a64e583ec5ba Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 30 Jun 2018 22:38:56 +0200 Subject: [PATCH 3/7] LastGenre test: prefer_specific without canonical This tests commit cef9a331. --- test/test_lastgenre.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py index 6c7e12261..b2d1bb51f 100644 --- a/test/test_lastgenre.py +++ b/test/test_lastgenre.py @@ -138,6 +138,21 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper): self.assertEqual(self.plugin._resolve_genres(['iota blues']), u'') + def test_prefer_specific_loads_tree(self): + """When prefer_specific is enabled but canonical is not the + tree still has to be loaded. + """ + self._setup_config(prefer_specific=True, canonical=False) + self.assertNotEqual(self.plugin.c14n_branches, []) + + def test_prefer_specific_without_canonical(self): + """When prefer_specific is enabled but canonical is not the + tree still has to be loaded. + """ + self._setup_config(prefer_specific=True, canonical=False) + self.assertEqual(self.plugin._resolve_genres(['delta blues']), + u'Delta Blues') + def test_no_duplicate(self): """Remove duplicated genres. """ From c3a3f92fe1234525f9c94f9048ffffb3eb6c9c21 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 30 Jun 2018 22:40:17 +0200 Subject: [PATCH 4/7] fix :ref: in changelog.rst --- docs/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index bb21f21be..ea9d10bfd 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,7 +23,7 @@ Fixes: are missing. Thanks to :user:`autrimpo`. :bug:`2757` -* Display the artist credit when matching albums if the ref:`artist_credit` +* Display the artist credit when matching albums if the :ref:`artist_credit` configuration option is set. :bug:`2953` From f016ea7e05c6d0ac0beb6c3a1f815908d47c000f Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 30 Jun 2018 22:40:34 +0200 Subject: [PATCH 5/7] update changelog add changelog entry for cef9a331, lastgenre: allow `prefer_specific` without `canonical`. --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ea9d10bfd..640add8fa 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,6 +26,9 @@ Fixes: * Display the artist credit when matching albums if the :ref:`artist_credit` configuration option is set. :bug:`2953` +* LastGenre: Allow to set the configuration option ``prefer_specific`` + without setting ``canonical``. + :bug:`2973` 1.4.7 (May 29, 2018) From ceabc72feb22860b6cac1e77e27efc155282ad94 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sun, 1 Jul 2018 09:15:05 +0200 Subject: [PATCH 6/7] lastgenre: remove unnecessary check In _resolv_genres wo do not have to explicitly check if self.c14n_branches is not empty. The tree will have been loaded when self.canonicalize is truthy. Also if self.c14n_branches would be empty the canonicalization is a no-op anyway... --- beetsplug/lastgenre/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index 6c195f6c4..2f660206e 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -195,7 +195,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): return None count = self.config['count'].get(int) - if self.c14n_branches and self.canonicalize: + if self.canonicalize: # Extend the list to consider tags parents in the c14n tree tags_all = [] for tag in tags: From 154e6cfca8378e7be9c81d45262a14a3e81d1b68 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sun, 1 Jul 2018 09:19:36 +0200 Subject: [PATCH 7/7] improve tests prefer_specific without canonical This improves the tests for the changes introduced in commit cef9a331 LastGenre: allow prefer_specific without canonical. --- test/test_lastgenre.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/test_lastgenre.py b/test/test_lastgenre.py index b2d1bb51f..a8e6318b7 100644 --- a/test/test_lastgenre.py +++ b/test/test_lastgenre.py @@ -146,12 +146,12 @@ class LastGenrePluginTest(unittest.TestCase, TestHelper): self.assertNotEqual(self.plugin.c14n_branches, []) def test_prefer_specific_without_canonical(self): - """When prefer_specific is enabled but canonical is not the - tree still has to be loaded. + """Prefer_specific works without canonical. """ - self._setup_config(prefer_specific=True, canonical=False) - self.assertEqual(self.plugin._resolve_genres(['delta blues']), - u'Delta Blues') + self._setup_config(prefer_specific=True, canonical=False, count=4) + self.assertEqual(self.plugin._resolve_genres( + ['math rock', 'post-rock']), + u'Post-Rock, Math Rock') def test_no_duplicate(self): """Remove duplicated genres.