Hide unneccessary columns in artwork infodialog and update unit tests

picard/ui/infodialog.py

@@ -45,6 +45,7 @@ from PyQt6 import (
 
 from picard import log
 from picard.album import Album
+from picard.config import get_config
 from picard.coverart.image import CoverArtImageIOError
 from picard.coverart.utils import translated_types_as_string
 from picard.file import File
@@ -99,10 +100,11 @@ class ArtworkTable(QtWidgets.QTableWidget):
     V_SIZE = 230
 
     NUM_ROWS = 0
-    NUM_COLS = 2
+    NUM_COLS = 3
 
     _columns = {}
     _labels = ()
+    _tooltips = {}
     artwork_columns = ()
 
     def __init__(self, parent=None):
@@ -116,6 +118,8 @@ class ArtworkTable(QtWidgets.QTableWidget):
         v_header.setDefaultSectionSize(self.V_SIZE)
 
         self.setHorizontalHeaderLabels(self._labels)
+        for colname, index in self._columns.items():
+            self.horizontalHeaderItem(index).setToolTip(self._tooltips.get(colname, None))
 
     def get_column_index(self, name):
         return self._columns[name]
@@ -127,43 +131,22 @@ class ArtworkTableSimple(ArtworkTable):
     _columns = {
         'type': 0,
         'new': 1,
+        'external': 2,
     }
 
-    _labels = (_("Type"), _("Cover"),)
+    _labels = (_("Type"), _("New Tags Cover"), _("New External Cover"),)
-    artwork_columns = ('new',)
+    artwork_columns = ('new', 'external',)
+    _tooltips = {
+        'new': _("New cover art embedded into tags"),
+        'external': _("New cover art saved as a separate file"),
+    }
 
     def __init__(self, parent=None):
         super().__init__(parent=parent)
         self.setColumnWidth(self.get_column_index('type'), self.TYPE_COLUMN_SIZE)
 
 
-class ArtworkTableSimpleExternal(ArtworkTableSimple):
-    NUM_COLS = 3
-
-    _columns = {
-        'type': 0,
-        'new': 1,
-        'external': 2,
-    }
-
-    _labels = (_("Type"), _("Cover"), _("External Cover"),)
-    artwork_columns = ('new', 'external',)
-
-
 class ArtworkTableExisting(ArtworkTable):
-    NUM_COLS = 3
-
-    _columns = {
-        'orig': 0,
-        'type': 1,
-        'new': 2,
-    }
-
-    _labels = (_("Existing Cover"), _("Type"), _("New Cover"),)
-    artwork_columns = ('orig', 'new',)
-
-
-class ArtworkTableExistingExternal(ArtworkTable):
     NUM_COLS = 4
 
     _columns = {
@@ -173,8 +156,12 @@ class ArtworkTableExistingExternal(ArtworkTable):
         'external': 3,
     }
 
-    _labels = (_("Existing Cover"), _("Type"), _("New Cover"), _("New External Cover"),)
+    _labels = (_("Existing Cover"), _("Type"), _("New Tags Cover"), _("New External Cover"),)
     artwork_columns = ('orig', 'new', 'external',)
+    _tooltips = {
+        'new': _("New cover art embedded into tags"),
+        'external': _("New cover art saved as a separate file"),
+    }
 
 
 class ArtworkRow:
@@ -201,20 +188,17 @@ class InfoDialog(PicardDialog):
         self.new_images = sorted(obj.metadata.images) or []
         self.orig_images = []
         artworktable_class = ArtworkTableSimple
-        has_external_images = any(image.external_file_coverart for image in self.new_images)
-        if has_external_images:
-            artworktable_class = ArtworkTableSimpleExternal
 
+        has_new_external_images = any(image.external_file_coverart for image in self.new_images)
+        has_new_different_images = obj.orig_metadata.images != obj.metadata.images
         has_orig_images = hasattr(obj, 'orig_metadata') and obj.orig_metadata.images
-        if has_orig_images and obj.orig_metadata.images != obj.metadata.images:
+        if has_orig_images and (has_new_different_images or has_new_external_images):
             is_track = isinstance(obj, Track)
             is_linked_file = isinstance(obj, File) and isinstance(obj.parent_item, Track)
             is_album_with_files = isinstance(obj, Album) and obj.get_num_total_files() > 0
             if is_track or is_linked_file or is_album_with_files:
                 self.orig_images = sorted(obj.orig_metadata.images)
                 artworktable_class = ArtworkTableExisting
-                if has_external_images:
-                    artworktable_class = ArtworkTableExistingExternal
 
         self.ui.setupUi(self)
         self.ui.buttonBox.addButton(
@@ -365,6 +349,13 @@ class InfoDialog(PicardDialog):
         self._display_artwork_rows()
         self.artwork_table.itemDoubleClicked.connect(self.show_item)
         self.artwork_table.verticalHeader().resizeSections(QtWidgets.QHeaderView.ResizeMode.ResizeToContents)
+        config = get_config()
+        for colname in self.artwork_table.artwork_columns:
+            tags_image_not_used = colname == 'new' and not config.setting['save_images_to_tags']
+            file_image_not_used = colname == 'external' and not config.setting['save_images_to_files']
+            if tags_image_not_used or file_image_not_used:
+                col_index = self.artwork_table.get_column_index(colname)
+                self.artwork_table.setColumnHidden(col_index, True)
 
     def tab_hide(self, widget):
         tab = self.ui.tabWidget

test/test_coverart_processing.py

@@ -18,6 +18,9 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
+from copy import copy
+import itertools
+
 from PyQt6.QtCore import QBuffer
 from PyQt6.QtGui import QImage
 
@@ -46,13 +49,15 @@ def create_fake_image(width, height, image_format):
 
 
 class ImageFiltersTest(PicardTestCase):
-    def test_filter_by_size(self):
+    def setUp(self):
         settings = {
             'filter_cover_by_size': True,
             'cover_minimum_width': 500,
             'cover_minimum_height': 500
         }
         self.set_config_values(settings)
+
+    def test_filter_by_size(self):
         image1 = create_fake_image(400, 600, "png")
         image2 = create_fake_image(500, 500, "jpeg")
         image3 = create_fake_image(600, 600, "bmp")
@@ -61,12 +66,6 @@ class ImageFiltersTest(PicardTestCase):
         self.assertTrue(size_filter(image3))
 
     def test_filter_by_size_metadata(self):
-        settings = {
-            'filter_cover_by_size': True,
-            'cover_minimum_width': 500,
-            'cover_minimum_height': 500
-        }
-        self.set_config_values(settings)
         image_metadata1 = {'width': 400, 'height': 600}
         image_metadata2 = {'width': 500, 'height': 500}
         image_metadata3 = {'width': 600, 'height': 600}
@@ -76,14 +75,21 @@ class ImageFiltersTest(PicardTestCase):
 
 
 class ImageProcessorsTest(PicardTestCase):
-    def test_resize(self):
+    def setUp(self):
-        processor = ResizeImage()
+        self.settings = {
-        settings = {
+            'enabled_plugins': [],
             'resize_images_saved_to_tags': True,
             'cover_tags_maximum_width': 500,
-            'cover_tags_maximum_height': 500
+            'cover_tags_maximum_height': 500,
+            'resize_images_saved_to_file': True,
+            'cover_file_maximum_width': 750,
+            'cover_file_maximum_height': 750,
+            'save_images_to_tags': True,
+            'save_images_to_files': True,
         }
-        self.set_config_values(settings)
+        self.set_config_values(self.settings)
+
+    def test_resize(self):
         sizes = [
             (500, 500),
             (1000, 500),
@@ -98,6 +104,7 @@ class ImageProcessorsTest(PicardTestCase):
             (500, 500),
             (400, 400)
         ]
+        processor = ResizeImage()
         for size, expected_size in zip(sizes, expected_sizes):
             image = ProcessingImage(create_fake_image(size[0], size[1], "jpg"))
             processor.run(image, ProcessingTarget.TAGS)
@@ -108,18 +115,6 @@ class ImageProcessorsTest(PicardTestCase):
             self.assertEqual(new_size, (image.info.width, image.info.height))
 
     def test_image_processors(self):
-        settings = {
-            'enabled_plugins': [],
-            'resize_images_saved_to_tags': True,
-            'cover_tags_maximum_width': 500,
-            'cover_tags_maximum_height': 500,
-            'resize_images_saved_to_file': True,
-            'cover_file_maximum_width': 750,
-            'cover_file_maximum_height': 750,
-            'save_images_to_tags': True,
-            'save_images_to_files': True,
-        }
-        self.set_config_values(settings)
         sizes = [
             (1000, 1000),
             (1000, 500),
@@ -130,16 +125,28 @@ class ImageProcessorsTest(PicardTestCase):
             ((500, 250), (750, 375)),
             ((500, 500), (600, 600)),
         ]
-        for size, expected_size in zip(sizes, expected_sizes):
+        settings = copy(self.settings)
-            coverartimage = CoverArtImage()
+        self.target_combinations = itertools.product([True, False], repeat=2)
-            image = create_fake_image(size[0], size[1], "jpg")
+        for save_to_tags, save_to_file in self.target_combinations:
-            run_image_processors(image, coverartimage)
+            settings['save_images_to_tags'] = save_to_tags
-            tags_size = (coverartimage.width, coverartimage.height)
+            settings['save_images_to_files'] = save_to_file
-            file_size = (coverartimage.external_file_coverart.width, coverartimage.external_file_coverart.height)
+            self.set_config_values(settings)
-            extension = coverartimage.extension[1:]
+            for size, expected_size in zip(sizes, expected_sizes):
-            self.assertEqual(tags_size, expected_size[0])
+                coverartimage = CoverArtImage()
-            self.assertEqual(file_size, expected_size[1])
+                image = create_fake_image(size[0], size[1], "jpg")
-            self.assertEqual(extension, "jpg")
+                run_image_processors(image, coverartimage)
+                tags_size = (coverartimage.width, coverartimage.height)
+                expected_size_tags = expected_size[0] if save_to_tags else size
+                self.assertEqual(tags_size, expected_size_tags)
+                if save_to_file:
+                    external_cover = coverartimage.external_file_coverart
+                    file_size = (external_cover.width, external_cover.height)
+                    self.assertEqual(file_size, expected_size[1])
+                else:
+                    self.assertIsNone(coverartimage.external_file_coverart)
+                extension = coverartimage.extension[1:]
+                self.assertEqual(extension, "jpg")
+        self.set_config_values(self.settings)
 
     def test_identification_error(self):
         image = create_fake_image(0, 0, "jpg")