From d1c88bbd25d7161d0d7f2d09f637693c3da143a7 Mon Sep 17 00:00:00 2001 From: Joseph Bushell Date: Thu, 1 Aug 2024 09:10:28 +0100 Subject: [PATCH 1/5] consider value of no_convert as one query rather than splitting --- beetsplug/convert.py | 11 ++++----- test/plugins/test_convert.py | 46 ++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index fe9d91d25..9b0e17a89 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -89,12 +89,11 @@ def should_transcode(item, fmt): """Determine whether the item should be transcoded as part of conversion (i.e., its bitrate is high or it has the wrong format). """ - no_convert_queries = config["convert"]["no_convert"].as_str_seq() - if no_convert_queries: - for query_string in no_convert_queries: - query, _ = parse_query_string(query_string, Item) - if query.match(item): - return False + no_convert_query = config["convert"]["no_convert"].as_str() + if no_convert_query: + query, _ = parse_query_string(no_convert_query, Item) + if query.match(item): + return False if ( config["convert"]["never_convert_lossy_files"] and item.format.lower() not in LOSSLESS_FORMATS diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 725318eac..10c3b95b9 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -335,3 +335,49 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): self.run_convert_path(item.path) converted = os.path.join(self.convert_dest, b"converted.ogg") self.assertNoFileTag(converted, "mp3") + + +@_common.slow_test() +class NoConvertTest(ConvertTestCase, ConvertCommand): + """Test the effect of the `no_convert` option.""" + + def setUp(self): + super().setUp() + + self.convert_dest = os.path.join(self.temp_dir, b"convert_dest") + self.config["convert"] = { + "dest": self.convert_dest, + "no_convert": "format:OGG", + "paths": {"default": "converted"}, + "format": "mp3", + "formats": { + "mp3": self.tagged_copy_cmd("mp3"), + }, + } + + def test_no_transcode_when_no_convert_set(self): + [item] = self.add_item_fixtures(ext="ogg") + with control_stdin("y"): + self.run_convert_path(item.path) + converted = os.path.join(self.convert_dest, b"converted.mp3") + self.assertFalse(os.path.exists(converted)) + + def test_no_transcode_when_multi_no_convert_set(self): + self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" + [item] = self.add_item_fixtures(ext="ogg") + item.bitrate = 128 + item.store() + with control_stdin("y"): + self.run_convert_path(item.path) + converted = os.path.join(self.convert_dest, b"converted.mp3") + self.assertFalse(os.path.exists(converted)) + + def test_transcode_when_multi_no_convert_set_partial_match(self): + self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" + [item] = self.add_item_fixtures(ext="ogg") + item.bitrate = 320 + item.store() + with control_stdin("y"): + self.run_convert_path(item.path) + converted = os.path.join(self.convert_dest, b"converted.mp3") + self.assertTrue(os.path.exists(converted)) From a73919b4ba4f5a67cea6baef83b48b6805d8b52a Mon Sep 17 00:00:00 2001 From: Joseph Bushell Date: Fri, 16 Aug 2024 21:08:56 +0100 Subject: [PATCH 2/5] add test for no_convert when using OR query --- test/plugins/test_convert.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 10c3b95b9..6604c2dd2 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -362,7 +362,7 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): converted = os.path.join(self.convert_dest, b"converted.mp3") self.assertFalse(os.path.exists(converted)) - def test_no_transcode_when_multi_no_convert_set(self): + def test_no_transcode_when_and_query_no_convert_set(self): self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" [item] = self.add_item_fixtures(ext="ogg") item.bitrate = 128 @@ -372,7 +372,7 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): converted = os.path.join(self.convert_dest, b"converted.mp3") self.assertFalse(os.path.exists(converted)) - def test_transcode_when_multi_no_convert_set_partial_match(self): + def test_transcode_when_and_query_no_convert_set_partial_match(self): self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" [item] = self.add_item_fixtures(ext="ogg") item.bitrate = 320 @@ -381,3 +381,13 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): self.run_convert_path(item.path) converted = os.path.join(self.convert_dest, b"converted.mp3") self.assertTrue(os.path.exists(converted)) + + def test_no_transcode_when_or_query_no_convert_set_partial_match(self): + self.config["convert"]["no_convert"] = "format:OGG , bitrate:..256" + [item] = self.add_item_fixtures(ext="ogg") + item.bitrate = 320 + item.store() + with control_stdin("y"): + self.run_convert_path(item.path) + converted = os.path.join(self.convert_dest, b"converted.mp3") + self.assertFalse(os.path.exists(converted)) From bba11be9f7e32a9b32a3a0b30d6d814ba8aa5bc9 Mon Sep 17 00:00:00 2001 From: Joseph Bushell Date: Wed, 25 Sep 2024 19:51:50 +0100 Subject: [PATCH 3/5] update test assertions --- test/plugins/test_convert.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 6604c2dd2..3075f1bfc 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -360,7 +360,7 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): with control_stdin("y"): self.run_convert_path(item.path) converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFalse(os.path.exists(converted)) + self.assertNotExists(converted) def test_no_transcode_when_and_query_no_convert_set(self): self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" @@ -370,7 +370,7 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): with control_stdin("y"): self.run_convert_path(item.path) converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFalse(os.path.exists(converted)) + self.assertNotExists(converted) def test_transcode_when_and_query_no_convert_set_partial_match(self): self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" @@ -380,7 +380,7 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): with control_stdin("y"): self.run_convert_path(item.path) converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertTrue(os.path.exists(converted)) + self.assertExists(converted) def test_no_transcode_when_or_query_no_convert_set_partial_match(self): self.config["convert"]["no_convert"] = "format:OGG , bitrate:..256" @@ -390,4 +390,4 @@ class NoConvertTest(ConvertTestCase, ConvertCommand): with control_stdin("y"): self.run_convert_path(item.path) converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertFalse(os.path.exists(converted)) + self.assertNotExists(converted) From 2e6e1809e3404ef939bac8a62c276b54176076fe Mon Sep 17 00:00:00 2001 From: Joseph Bushell Date: Sat, 26 Oct 2024 18:15:44 +0100 Subject: [PATCH 4/5] Update docs/changelog.rst MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Šarūnas Nejus --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index a2e15dd31..3e4266db1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -47,6 +47,10 @@ Bug fixes: * :doc:`plugins/lyrics`: Update ``tekstowo`` backend to fetch lyrics directly since recent updates to their website made it unsearchable. :bug:`5456` +* :doc:`plugins/convert`: Fixed the convert plugin ``no_convert`` option so + that it no longer treats "and" and "or" queries the same. To maintain + previous behaviour add commas between your query keywords. For help see + :ref:`combiningqueries`. For packagers: From 4b78abd939869d4e5e661fe241627da96f56e128 Mon Sep 17 00:00:00 2001 From: Joseph Bushell Date: Sat, 26 Oct 2024 19:09:56 +0100 Subject: [PATCH 5/5] create seperate in_no_convert function, update tests --- beetsplug/convert.py | 15 ++++---- test/plugins/test_convert.py | 68 +++++++++--------------------------- 2 files changed, 26 insertions(+), 57 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 9b0e17a89..bb06488f9 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -84,17 +84,20 @@ def get_format(fmt=None): return (command.encode("utf-8"), extension.encode("utf-8")) +def in_no_convert(item: Item) -> bool: + no_convert_query = config["convert"]["no_convert"].as_str() + if no_convert_query: + query, _ = parse_query_string(no_convert_query, Item) + return query.match(item) + else: + return False + def should_transcode(item, fmt): """Determine whether the item should be transcoded as part of conversion (i.e., its bitrate is high or it has the wrong format). """ - no_convert_query = config["convert"]["no_convert"].as_str() - if no_convert_query: - query, _ = parse_query_string(no_convert_query, Item) - if query.match(item): - return False - if ( + if in_no_convert(item) or ( config["convert"]["never_convert_lossy_files"] and item.format.lower() not in LOSSLESS_FORMATS ): diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 3075f1bfc..9057b2472 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -18,10 +18,13 @@ import os.path import re import sys import unittest +import pytest from mediafile import MediaFile from beets import util +from beetsplug import convert +from beets.library import Item from beets.test import _common from beets.test.helper import ( AsIsImporterMixin, @@ -337,57 +340,20 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): self.assertNoFileTag(converted, "mp3") -@_common.slow_test() -class NoConvertTest(ConvertTestCase, ConvertCommand): +class TestNoConvert: """Test the effect of the `no_convert` option.""" - def setUp(self): - super().setUp() + @pytest.mark.parametrize( + "config_value, should_skip", + [ + ("", False), + ("bitrate:320", False), + ("bitrate:320 format:ogg", False), + ("bitrate:320 , format:ogg", True), + ], + ) - self.convert_dest = os.path.join(self.temp_dir, b"convert_dest") - self.config["convert"] = { - "dest": self.convert_dest, - "no_convert": "format:OGG", - "paths": {"default": "converted"}, - "format": "mp3", - "formats": { - "mp3": self.tagged_copy_cmd("mp3"), - }, - } - - def test_no_transcode_when_no_convert_set(self): - [item] = self.add_item_fixtures(ext="ogg") - with control_stdin("y"): - self.run_convert_path(item.path) - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertNotExists(converted) - - def test_no_transcode_when_and_query_no_convert_set(self): - self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" - [item] = self.add_item_fixtures(ext="ogg") - item.bitrate = 128 - item.store() - with control_stdin("y"): - self.run_convert_path(item.path) - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertNotExists(converted) - - def test_transcode_when_and_query_no_convert_set_partial_match(self): - self.config["convert"]["no_convert"] = "format:OGG bitrate:..256" - [item] = self.add_item_fixtures(ext="ogg") - item.bitrate = 320 - item.store() - with control_stdin("y"): - self.run_convert_path(item.path) - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertExists(converted) - - def test_no_transcode_when_or_query_no_convert_set_partial_match(self): - self.config["convert"]["no_convert"] = "format:OGG , bitrate:..256" - [item] = self.add_item_fixtures(ext="ogg") - item.bitrate = 320 - item.store() - with control_stdin("y"): - self.run_convert_path(item.path) - converted = os.path.join(self.convert_dest, b"converted.mp3") - self.assertNotExists(converted) + def test_no_convert_skip(self, config_value, should_skip): + item = Item(format="ogg", bitrate=256) + convert.config["convert"]["no_convert"] = config_value + assert convert.in_no_convert(item) == should_skip