From 06b9af1511c24933a6d4b52a64f6325f4e0b3dc9 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 17:35:20 +0200 Subject: [PATCH 01/20] CheckboxListItem: accept parent parameter - change imports to match the style we have elsewhere --- picard/ui/checkbox_list_item.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/picard/ui/checkbox_list_item.py b/picard/ui/checkbox_list_item.py index 70b8563e8..d32c63ab3 100644 --- a/picard/ui/checkbox_list_item.py +++ b/picard/ui/checkbox_list_item.py @@ -21,17 +21,19 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -from PyQt6.QtCore import Qt -from PyQt6.QtWidgets import QListWidgetItem +from PyQt6 import ( + QtCore, + QtWidgets, +) -class CheckboxListItem(QListWidgetItem): +class CheckboxListItem(QtWidgets.QListWidgetItem): - def __init__(self, text='', checked=False): - super().__init__(text) - self.setFlags(self.flags() | Qt.ItemFlag.ItemIsUserCheckable) - self.setCheckState(Qt.CheckState.Checked if checked else Qt.CheckState.Unchecked) + def __init__(self, text='', checked=False, parent=None): + super().__init__(text, parent=parent) + self.setFlags(self.flags() | QtCore.Qt.ItemFlag.ItemIsUserCheckable) + self.setCheckState(QtCore.Qt.CheckState.Checked if checked else QtCore.Qt.CheckState.Unchecked) @property def checked(self): - return self.checkState() == Qt.CheckState.Checked + return self.checkState() == QtCore.Qt.CheckState.Checked From ab9e7ff5d92c990f6ea57925bcbe3dea4aa35855 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 17:41:03 +0200 Subject: [PATCH 02/20] Move CheckboxListItem to ui/widgets --- picard/ui/options/cover.py | 2 +- picard/ui/{ => widgets}/checkbox_list_item.py | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename picard/ui/{ => widgets}/checkbox_list_item.py (100%) diff --git a/picard/ui/options/cover.py b/picard/ui/options/cover.py index fb5afd9b5..8009bfe8c 100644 --- a/picard/ui/options/cover.py +++ b/picard/ui/options/cover.py @@ -38,11 +38,11 @@ from picard.i18n import ( gettext as _, ) -from picard.ui.checkbox_list_item import CheckboxListItem from picard.ui.forms.ui_options_cover import Ui_CoverOptionsPage from picard.ui.moveable_list_view import MoveableListView from picard.ui.options import OptionsPage from picard.ui.util import qlistwidget_items +from picard.ui.widgets.checkbox_list_item import CheckboxListItem class CoverOptionsPage(OptionsPage): diff --git a/picard/ui/checkbox_list_item.py b/picard/ui/widgets/checkbox_list_item.py similarity index 100% rename from picard/ui/checkbox_list_item.py rename to picard/ui/widgets/checkbox_list_item.py From 8bff6a2425960feda3f5680dfd6cc7195d82fe56 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 17:45:37 +0200 Subject: [PATCH 03/20] BaseAction: match proto, pass parent to parent class if provided --- picard/extension_points/item_actions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/extension_points/item_actions.py b/picard/extension_points/item_actions.py index 977d8e71a..dd15cecf6 100644 --- a/picard/extension_points/item_actions.py +++ b/picard/extension_points/item_actions.py @@ -54,8 +54,8 @@ class BaseAction(QtGui.QAction): NAME = "Unknown" MENU = [] - def __init__(self): - super().__init__(self.NAME, None) + def __init__(self, parent=None): + super().__init__(self.NAME, parent=parent) self.tagger = QtCore.QCoreApplication.instance() self.triggered.connect(self.__callback) From aed4885e26cbb4fd937c60d203502c9d803ac22d Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 18:27:31 +0200 Subject: [PATCH 04/20] Introduce HashableItem class --- picard/ui/__init__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/picard/ui/__init__.py b/picard/ui/__init__.py index 99bd0f613..b48a042eb 100644 --- a/picard/ui/__init__.py +++ b/picard/ui/__init__.py @@ -212,6 +212,18 @@ class PicardDialog(QtWidgets.QDialog, PreserveGeometry): # With py3, QObjects are no longer hashable unless they have # an explicit __hash__ implemented. # See: http://python.6.x6.nabble.com/QTreeWidgetItem-is-not-hashable-in-Py3-td5212216.html +class HashableItem: + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.id = uuid.uuid4() + + def __eq__(self, other): + return self.id == other.id + + def __hash__(self): + return hash(str(self.id)) + + class HashableTreeWidgetItem(QtWidgets.QTreeWidgetItem): def __init__(self, *args, **kwargs): From 5b4ecfca79581b4163c36cd998e1bd0ef972094a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 18:29:17 +0200 Subject: [PATCH 05/20] HashableTreeWidgetItem, HashableTreeWidgetItem: inherit from HashableItem --- picard/ui/__init__.py | 26 ++++---------------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/picard/ui/__init__.py b/picard/ui/__init__.py index b48a042eb..c40c242fc 100644 --- a/picard/ui/__init__.py +++ b/picard/ui/__init__.py @@ -224,27 +224,9 @@ class HashableItem: return hash(str(self.id)) -class HashableTreeWidgetItem(QtWidgets.QTreeWidgetItem): - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.id = uuid.uuid4() - - def __eq__(self, other): - return self.id == other.id - - def __hash__(self): - return hash(str(self.id)) +class HashableTreeWidgetItem(HashableItem, QtWidgets.QTreeWidgetItem): + pass -class HashableListWidgetItem(QtWidgets.QListWidgetItem): - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.id = uuid.uuid4() - - def __eq__(self, other): - return self.id == other.id - - def __hash__(self): - return hash(str(self.id)) +class HashableListWidgetItem(HashableItem, QtWidgets.QListWidgetItem): + pass From 4ccb1c14f4ff7e709dc190bfa566e78b8762e75a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 18:33:16 +0200 Subject: [PATCH 06/20] HashableItem: id -> __id --- picard/ui/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/__init__.py b/picard/ui/__init__.py index c40c242fc..75fb46432 100644 --- a/picard/ui/__init__.py +++ b/picard/ui/__init__.py @@ -215,13 +215,13 @@ class PicardDialog(QtWidgets.QDialog, PreserveGeometry): class HashableItem: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.id = uuid.uuid4() + self.__id = uuid.uuid4() def __eq__(self, other): - return self.id == other.id + return self.__id == other.__id def __hash__(self): - return hash(str(self.id)) + return hash(str(self.__id)) class HashableTreeWidgetItem(HashableItem, QtWidgets.QTreeWidgetItem): From 70ee6d818c81054b4ba1483f0534a8ea5322da2f Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 18:38:32 +0200 Subject: [PATCH 07/20] HashableItem: no need to convert uuid4 to str, it can be hashed directly https://github.com/python/cpython/blob/5130731c9e779b97d00a24f54cdce73ce9975dfd/Lib/uuid.py#L268 --- picard/ui/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/__init__.py b/picard/ui/__init__.py index 75fb46432..f9e24af7b 100644 --- a/picard/ui/__init__.py +++ b/picard/ui/__init__.py @@ -221,7 +221,7 @@ class HashableItem: return self.__id == other.__id def __hash__(self): - return hash(str(self.__id)) + return hash(self.__id) class HashableTreeWidgetItem(HashableItem, QtWidgets.QTreeWidgetItem): From 6b5d673e1107a42d2493f5dea010ecda95bb3ab6 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 18:39:48 +0200 Subject: [PATCH 08/20] HashableItem: cache the hash() result --- picard/ui/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/picard/ui/__init__.py b/picard/ui/__init__.py index f9e24af7b..dbfd74b3a 100644 --- a/picard/ui/__init__.py +++ b/picard/ui/__init__.py @@ -216,12 +216,13 @@ class HashableItem: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.__id = uuid.uuid4() + self.__hash = hash(self.__id) def __eq__(self, other): return self.__id == other.__id def __hash__(self): - return hash(self.__id) + return self.__hash class HashableTreeWidgetItem(HashableItem, QtWidgets.QTreeWidgetItem): From 2ec8248ea006ea518e62bc32cbcc145296125d72 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 22:12:21 +0200 Subject: [PATCH 09/20] PicardDialog.__init__(): call parent class __init__() before anything --- picard/ui/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/__init__.py b/picard/ui/__init__.py index dbfd74b3a..f811a9e72 100644 --- a/picard/ui/__init__.py +++ b/picard/ui/__init__.py @@ -182,8 +182,8 @@ class PicardDialog(QtWidgets.QDialog, PreserveGeometry): ready_for_display = QtCore.pyqtSignal() def __init__(self, parent=None): - self.tagger = QtCore.QCoreApplication.instance() super().__init__(parent=parent, f=self.flags) + self.tagger = QtCore.QCoreApplication.instance() self.__shown = False self.ready_for_display.connect(self.restore_geometry) From e7dff30b2d9a755b97942ff9080c8a079b55d3be Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Wed, 29 May 2024 22:44:43 +0200 Subject: [PATCH 10/20] TreeItem: make arguments explicit - sortable becomes optional - parent is now a keyword argument - obj is mandatory (unchanged) - fix up callers accordingly - introduce a post_init() method to allow post initialization without duplicating __init__() --- picard/ui/itemviews/__init__.py | 25 ++++++++++++++----------- picard/ui/itemviews/basetreeview.py | 2 +- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/picard/ui/itemviews/__init__.py b/picard/ui/itemviews/__init__.py index 409bf0c72..37116572d 100644 --- a/picard/ui/itemviews/__init__.py +++ b/picard/ui/itemviews/__init__.py @@ -241,10 +241,10 @@ class FileTreeView(BaseTreeView): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.unmatched_files = ClusterItem(self.tagger.unclustered_files, False, self) + self.unmatched_files = ClusterItem(self.tagger.unclustered_files, parent=self) self.unmatched_files.update() self.unmatched_files.setExpanded(True) - self.clusters = ClusterItem(self.tagger.clusters, False, self) + self.clusters = ClusterItem(self.tagger.clusters, parent=self) self.set_clusters_text() self.clusters.setExpanded(True) self.tagger.cluster_added.connect(self.add_file_cluster) @@ -282,10 +282,10 @@ class AlbumTreeView(BaseTreeView): def add_album(self, album): if isinstance(album, NatAlbum): - item = NatAlbumItem(album, True) + item = NatAlbumItem(album, sortable=True) self.insertTopLevelItem(0, item) else: - item = AlbumItem(album, True, self) + item = AlbumItem(album, sortable=True, parent=self) item.setIcon(ITEM_ICON_COLUMN, AlbumItem.icon_cd) for i, column in enumerate(DEFAULT_COLUMNS): font = item.font(i) @@ -301,13 +301,17 @@ class AlbumTreeView(BaseTreeView): class TreeItem(QtWidgets.QTreeWidgetItem): - def __init__(self, obj, sortable, *args): - super().__init__(*args) + def __init__(self, obj, sortable=False, parent=None): + super().__init__(parent) self.obj = obj if obj is not None: obj.item = self self.sortable = sortable self._sortkeys = {} + self.post_init() + + def post_init(self): + pass def setText(self, column, text): self._sortkeys[column] = None @@ -352,8 +356,7 @@ class TreeItem(QtWidgets.QTreeWidgetItem): class ClusterItem(TreeItem): - def __init__(self, *args): - super().__init__(*args) + def post_init(self): self.setIcon(ITEM_ICON_COLUMN, ClusterItem.icon_dir) def update(self, update_selection=True): @@ -375,7 +378,7 @@ class ClusterItem(TreeItem): # to be certain about item order in the cluster (addChildren adds in reverse order). # Benchmarked performance was not noticeably different. for file in files: - item = FileItem(file, True) + item = FileItem(file, sortable=True) self.addChild(item) item.update() @@ -412,7 +415,7 @@ class AlbumItem(TreeItem): if newnum > oldnum: # add new items items = [] for i in range(oldnum, newnum): - item = TrackItem(album.tracks[i], False) + item = TrackItem(album.tracks[i]) item.setHidden(False) # Workaround to make sure the parent state gets updated items.append(item) # insertChildren behaves differently if sorting is disabled / enabled, which results @@ -520,7 +523,7 @@ class TrackItem(TreeItem): if newnum > oldnum: # add new items items = [] for i in range(newnum - 1, oldnum - 1, -1): - item = FileItem(track.files[i], False) + item = FileItem(track.files[i]) item.update(update_track=False, update_selection=update_selection) items.append(item) self.addChildren(items) diff --git a/picard/ui/itemviews/basetreeview.py b/picard/ui/itemviews/basetreeview.py index 077bd849d..f1c06e4bc 100644 --- a/picard/ui/itemviews/basetreeview.py +++ b/picard/ui/itemviews/basetreeview.py @@ -633,7 +633,7 @@ class BaseTreeView(QtWidgets.QTreeWidget): if parent_item is None: parent_item = self.clusters from picard.ui.itemviews import ClusterItem - cluster_item = ClusterItem(cluster, not cluster.special, parent_item) + cluster_item = ClusterItem(cluster, sortable=not cluster.special, parent=parent_item) if cluster.hide_if_empty and not cluster.files: cluster_item.update() cluster_item.setHidden(True) From 9684df3b7608e38c6366ff0333bd9c294473bdaf Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 10:40:21 +0200 Subject: [PATCH 11/20] display_caa_types_selector() -> CAATypesSelectorDialog.display() class method - make parameters explicit, rather than using **kwargs --- picard/coverart/providers/caa.py | 4 ++-- picard/ui/caa_types_selector.py | 34 +++++++++++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/picard/coverart/providers/caa.py b/picard/coverart/providers/caa.py index 4333445a7..dfee27bd9 100644 --- a/picard/coverart/providers/caa.py +++ b/picard/coverart/providers/caa.py @@ -65,7 +65,7 @@ from picard.i18n import ( ) from picard.webservice import ratecontrol -from picard.ui.caa_types_selector import display_caa_types_selector +from picard.ui.caa_types_selector import CAATypesSelectorDialog from picard.ui.forms.ui_provider_options_caa import Ui_CaaOptions @@ -168,7 +168,7 @@ class ProviderOptionsCaa(ProviderOptions): def select_caa_types(self): known_types = {t['name']: translate_caa_type(t['name']) for t in CAA_TYPES} - (types, types_to_omit, ok) = display_caa_types_selector( + (types, types_to_omit, ok) = CAATypesSelectorDialog.display( parent=self, types_include=self.caa_image_types, types_exclude=self.caa_image_types_to_omit, diff --git a/picard/ui/caa_types_selector.py b/picard/ui/caa_types_selector.py index ef5b56a31..9b1488e0c 100644 --- a/picard/ui/caa_types_selector.py +++ b/picard/ui/caa_types_selector.py @@ -164,8 +164,13 @@ class CAATypesSelectorDialog(PicardDialog): help_url = 'doc_cover_art_types' def __init__( - self, parent=None, types_include=None, types_exclude=None, - default_include=None, default_exclude=None, known_types=None + self, + parent=None, + types_include=None, + types_exclude=None, + default_include=None, + default_exclude=None, + known_types=None, ): super().__init__(parent=parent) if types_include is None: @@ -355,8 +360,23 @@ class CAATypesSelectorDialog(PicardDialog): self.arrows_exclude.button_remove.setEnabled(has_items_exclude and has_selected_exclude) self.arrows_exclude.button_remove_all.setEnabled(has_items_exclude) - -def display_caa_types_selector(**kwargs): - dialog = CAATypesSelectorDialog(**kwargs) - result = dialog.exec() - return (dialog.included, dialog.excluded, result == QtWidgets.QDialog.DialogCode.Accepted) + @classmethod + def display( + cls, + parent=None, + types_include=None, + types_exclude=None, + default_include=None, + default_exclude=None, + known_types=None, + ): + dialog = cls( + parent=parent, + types_include=types_include, + types_exclude=types_exclude, + default_include=default_include, + default_exclude=default_exclude, + known_types=known_types, + ) + result = dialog.exec() + return (dialog.included, dialog.excluded, result == QtWidgets.QDialog.DialogCode.Accepted) From 414b8d83ce480f90e6cb5431d59fbc8e3aa01318 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 10:45:15 +0200 Subject: [PATCH 12/20] LogViewCommon: explicit parameters --- picard/ui/logview.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/logview.py b/picard/ui/logview.py index e9aa0b43e..ff6e6b82c 100644 --- a/picard/ui/logview.py +++ b/picard/ui/logview.py @@ -72,8 +72,8 @@ class LogViewDialog(PicardDialog): class LogViewCommon(LogViewDialog): - def __init__(self, log_tail, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, log_tail, title, parent=None): + super().__init__(title, parent=parent) self.displaying = False self.log_tail = log_tail self._init_doc() From a0cc95fff6ca96a9a0cd80cd952f1f7e9c62cbcd Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 10:54:27 +0200 Subject: [PATCH 13/20] TipSlider, ClickableSlider: pass parent explicitely --- picard/ui/options/releases.py | 6 +++--- picard/ui/playertoolbar.py | 2 +- picard/ui/widgets/__init__.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/picard/ui/options/releases.py b/picard/ui/options/releases.py index f00f94607..94d34d96d 100644 --- a/picard/ui/options/releases.py +++ b/picard/ui/options/releases.py @@ -64,8 +64,8 @@ class TipSlider(ClickableSlider): _minimum = 0 _maximum = 100 - def __init__(self, *args): - super().__init__(*args) + def __init__(self, parent=None): + super().__init__(parent=parent) self.style = QtWidgets.QApplication.style() self.opt = QtWidgets.QStyleOptionSlider() @@ -117,7 +117,7 @@ class ReleaseTypeScore: self.label = QtWidgets.QLabel(self.group) self.label.setText(label) self.layout.addWidget(self.label, row, column, 1, 1) - self.slider = TipSlider(self.group) + self.slider = TipSlider(parent=self.group) self.layout.addWidget(self.slider, row, column + 1, 1, 1) self.reset() diff --git a/picard/ui/playertoolbar.py b/picard/ui/playertoolbar.py index b8fc9edba..d0aea0cba 100644 --- a/picard/ui/playertoolbar.py +++ b/picard/ui/playertoolbar.py @@ -306,7 +306,7 @@ class PlaybackProgressSlider(QtWidgets.QWidget): tool_font = QtWidgets.QApplication.font('QToolButton') - self.progress_slider = ClickableSlider(self) + self.progress_slider = ClickableSlider(parent=self) self.progress_slider.setOrientation(QtCore.Qt.Orientation.Horizontal) self.progress_slider.setEnabled(False) self.progress_slider.setMinimumWidth(30) diff --git a/picard/ui/widgets/__init__.py b/picard/ui/widgets/__init__.py index 68d38a6f3..e1997e3d0 100644 --- a/picard/ui/widgets/__init__.py +++ b/picard/ui/widgets/__init__.py @@ -162,7 +162,7 @@ class SliderPopover(Popover): self.label.setAlignment(QtCore.Qt.AlignmentFlag.AlignCenter) vbox.addWidget(self.label) - self.slider = ClickableSlider(self) + self.slider = ClickableSlider(parent=self) self.slider.setOrientation(QtCore.Qt.Orientation.Horizontal) self.slider.setValue(int(value)) self.slider.valueChanged.connect(self.value_changed) From 1650f29d1a3b5a2e82e0f77eb0e56aea1b1f0476 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 11:45:43 +0200 Subject: [PATCH 14/20] CAATypesSelectorDialog: reduce number of arguments, use defaults - default_include, default_exclude, known_types are from defaults, unlikely to change - so move part of the code from caller to CAATypesSelectorDialog() class - make types_include & types_exclude sets, as they are only used using `in` - move parent parameter at the end --- picard/coverart/providers/caa.py | 10 +------- picard/ui/caa_types_selector.py | 44 ++++++++++++++------------------ 2 files changed, 20 insertions(+), 34 deletions(-) diff --git a/picard/coverart/providers/caa.py b/picard/coverart/providers/caa.py index dfee27bd9..05ed459a1 100644 --- a/picard/coverart/providers/caa.py +++ b/picard/coverart/providers/caa.py @@ -55,10 +55,6 @@ from picard.coverart.providers.provider import ( CoverArtProvider, ProviderOptions, ) -from picard.coverart.utils import ( - CAA_TYPES, - translate_caa_type, -) from picard.i18n import ( N_, gettext as _, @@ -167,14 +163,10 @@ class ProviderOptionsCaa(ProviderOptions): self.ui.select_caa_types.setEnabled(enabled) def select_caa_types(self): - known_types = {t['name']: translate_caa_type(t['name']) for t in CAA_TYPES} (types, types_to_omit, ok) = CAATypesSelectorDialog.display( - parent=self, types_include=self.caa_image_types, types_exclude=self.caa_image_types_to_omit, - default_include=DEFAULT_CAA_IMAGE_TYPE_INCLUDE, - default_exclude=DEFAULT_CAA_IMAGE_TYPE_EXCLUDE, - known_types=known_types, + parent=self, ) if ok: self.caa_image_types = types diff --git a/picard/ui/caa_types_selector.py b/picard/ui/caa_types_selector.py index 9b1488e0c..796a24e99 100644 --- a/picard/ui/caa_types_selector.py +++ b/picard/ui/caa_types_selector.py @@ -37,6 +37,14 @@ from PyQt6 import ( QtWidgets, ) +from picard.const.defaults import ( + DEFAULT_CAA_IMAGE_TYPE_EXCLUDE, + DEFAULT_CAA_IMAGE_TYPE_INCLUDE, +) +from picard.coverart.utils import ( + CAA_TYPES, + translate_caa_type, +) from picard.i18n import ( N_, gettext as _, @@ -153,33 +161,25 @@ class CAATypesSelectorDialog(PicardDialog): """Display dialog box to select the CAA image types to include and exclude from download and use. Keyword Arguments: - parent {[type]} -- Parent of the QDialog object being created (default: {None}) types_include {[string]} -- List of CAA image types to include (default: {None}) types_exclude {[string]} -- List of CAA image types to exclude (default: {None}) - default_include {[string]} -- List of CAA image types to include by default (default: {None}) - default_exclude {[string]} -- List of CAA image types to exclude by default (default: {None}) - known_types {{string: string}} -- Dict. of all known CAA image types, unique name as key, translated title as value (default: {None}) + parent {[type]} -- Parent of the QDialog object being created (default: {None}) """ help_url = 'doc_cover_art_types' def __init__( self, - parent=None, types_include=None, types_exclude=None, - default_include=None, - default_exclude=None, - known_types=None, + parent=None, ): super().__init__(parent=parent) - if types_include is None: - types_include = [] - if types_exclude is None: - types_exclude = [] - self._default_include = default_include or [] - self._default_exclude = default_exclude or [] - self._known_types = known_types or {} + types_include = set(types_include or ()) + types_exclude = set(types_exclude or ()) + self._default_include = DEFAULT_CAA_IMAGE_TYPE_INCLUDE + self._default_exclude = DEFAULT_CAA_IMAGE_TYPE_EXCLUDE + self._known_types = {t['name']: translate_caa_type(t['name']) for t in CAA_TYPES} self.setWindowTitle(_("Cover art types")) self.setWindowModality(QtCore.Qt.WindowModality.WindowModal) @@ -306,8 +306,8 @@ class CAATypesSelectorDialog(PicardDialog): 'excludes' lists to determine the appropriate list for each type. Arguments: - includes -- list of standard image types to place in the "Include" listbox - excludes -- list of standard image types to place in the "Exclude" listbox + includes -- set of standard image types to place in the "Include" listbox + excludes -- set of standard image types to place in the "Exclude" listbox """ self.list_include.clear() self.list_exclude.clear() @@ -363,20 +363,14 @@ class CAATypesSelectorDialog(PicardDialog): @classmethod def display( cls, - parent=None, types_include=None, types_exclude=None, - default_include=None, - default_exclude=None, - known_types=None, + parent=None, ): dialog = cls( - parent=parent, types_include=types_include, types_exclude=types_exclude, - default_include=default_include, - default_exclude=default_exclude, - known_types=known_types, + parent=parent, ) result = dialog.exec() return (dialog.included, dialog.excluded, result == QtWidgets.QDialog.DialogCode.Accepted) From 04c1e6e1b7cbfb756b882a921cc0b75b3fdf210b Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 12:02:04 +0200 Subject: [PATCH 15/20] CAATypesSelectorDialog: included/excluded -> return tuple - excluded can be an empty tuple, `'none'` isn't a valid type anyway - adapt debug output (display [] instead of set()) --- picard/coverart/providers/caa.py | 2 +- picard/ui/caa_types_selector.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/coverart/providers/caa.py b/picard/coverart/providers/caa.py index 05ed459a1..fb56da58b 100644 --- a/picard/coverart/providers/caa.py +++ b/picard/coverart/providers/caa.py @@ -274,7 +274,7 @@ class CoverArtProviderCaa(CoverArtProvider): self.error("CAA JSON error: %s" % (http.errorString())) else: if self.restrict_types: - log.debug("CAA types: included: %s, excluded: %s", self.included_types, self.excluded_types) + log.debug("CAA types: included: %s, excluded: %s", list(self.included_types), list(self.excluded_types)) try: config = get_config() for image in data['images']: diff --git a/picard/ui/caa_types_selector.py b/picard/ui/caa_types_selector.py index 796a24e99..f18bd588f 100644 --- a/picard/ui/caa_types_selector.py +++ b/picard/ui/caa_types_selector.py @@ -324,11 +324,11 @@ class CAATypesSelectorDialog(PicardDialog): @property def included(self): - return list(self.list_include.all_items_data()) or ['front'] + return tuple(self.list_include.all_items_data()) or ('front', ) @property def excluded(self): - return list(self.list_exclude.all_items_data()) or ['none'] + return tuple(self.list_exclude.all_items_data()) def _on_list_clicked(self, lists, index): for temp_list in lists: From f4c1f958d9d16c7e4bbac25ab04edcaeb99b57e3 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 13:09:51 +0200 Subject: [PATCH 16/20] Drop LockableObject, unused --- picard/dataobj.py | 5 +++-- picard/util/__init__.py | 22 ---------------------- 2 files changed, 3 insertions(+), 24 deletions(-) diff --git a/picard/dataobj.py b/picard/dataobj.py index 27a4735fc..184cd0a00 100644 --- a/picard/dataobj.py +++ b/picard/dataobj.py @@ -28,11 +28,12 @@ from collections import Counter +from PyQt6 import QtCore + from picard.config import get_config -from picard.util import LockableObject -class DataObject(LockableObject): +class DataObject(QtCore.QObject): def __init__(self, obj_id): super().__init__() diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 0fd9b0be0..cb84b1d9f 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -126,28 +126,6 @@ class ReadWriteLockContext: return self._entered > 0 -class LockableObject(QtCore.QObject): - """Read/write lockable object.""" - - def __init__(self): - super().__init__() - self.__context = ReadWriteLockContext() - - def lock_for_read(self): - """Lock the object for read operations.""" - self.__context.lock_for_read() - return self.__context - - def lock_for_write(self): - """Lock the object for write operations.""" - self.__context.lock_for_write() - return self.__context - - def unlock(self): - """Unlock the object.""" - self.__context.unlock() - - def process_events_iter(iterable, interval=0.1): """ Creates an iterator over iterable that calls QCoreApplication.processEvents() From 6a633dcd108db698c17564e85e59974bd12ad274 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 13:24:00 +0200 Subject: [PATCH 17/20] Album: it inherits from MetadataItem, but never calls MetadataItem.__init__() - this doesn't seem to cause any issue, but that's bad practice --- picard/album.py | 1 + 1 file changed, 1 insertion(+) diff --git a/picard/album.py b/picard/album.py index 5ba888a51..2dad87332 100644 --- a/picard/album.py +++ b/picard/album.py @@ -129,6 +129,7 @@ class Album(DataObject, MetadataItem): def __init__(self, album_id, discid=None): DataObject.__init__(self, album_id) + MetadataItem.__init__(self) self.tagger = QtCore.QCoreApplication.instance() self.tracks = [] self.loaded = False From c3a815dba678407209fbe8c36cf3b0830b3ad986 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 14:29:40 +0200 Subject: [PATCH 18/20] Add a tagger property to MetadataItem It also adds a setter for it to detect duplicated setting of this property --- picard/album.py | 6 +----- picard/cluster.py | 1 - picard/file.py | 1 - picard/item.py | 13 +++++++++++++ picard/track.py | 1 - 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/picard/album.py b/picard/album.py index 2dad87332..e1966f00b 100644 --- a/picard/album.py +++ b/picard/album.py @@ -47,10 +47,7 @@ from collections import ( from enum import IntEnum import traceback -from PyQt6 import ( - QtCore, - QtNetwork, -) +from PyQt6 import QtNetwork from picard import log from picard.cluster import Cluster @@ -130,7 +127,6 @@ class Album(DataObject, MetadataItem): def __init__(self, album_id, discid=None): DataObject.__init__(self, album_id) MetadataItem.__init__(self) - self.tagger = QtCore.QCoreApplication.instance() self.tracks = [] self.loaded = False self.load_task = None diff --git a/picard/cluster.py b/picard/cluster.py index 3035db6d8..9b9f2ac7b 100644 --- a/picard/cluster.py +++ b/picard/cluster.py @@ -102,7 +102,6 @@ class Cluster(FileList): def __init__(self, name, artist="", special=False, related_album=None, hide_if_empty=False): super().__init__() - self.tagger = QtCore.QCoreApplication.instance() self.item = None self.metadata['album'] = name self.metadata['albumartist'] = artist diff --git a/picard/file.py b/picard/file.py index ce2e28cf6..4f75cac97 100644 --- a/picard/file.py +++ b/picard/file.py @@ -163,7 +163,6 @@ class File(QtCore.QObject, MetadataItem): def __init__(self, filename): super().__init__() - self.tagger = QtCore.QCoreApplication.instance() self.filename = filename self.base_filename = os.path.basename(filename) self._state = File.UNDEFINED diff --git a/picard/item.py b/picard/item.py index db448bb13..6f0f09869 100644 --- a/picard/item.py +++ b/picard/item.py @@ -169,6 +169,19 @@ class MetadataItem(Item): self.iter_children_items_metadata_ignore_attrs = {} self.suspend_metadata_images_update = IgnoreUpdatesContext() + @property + def tagger(self): + return QtCore.QCoreApplication.instance() + + @tagger.setter + def tagger(self, value): + # We used to set tagger property in subclasses, but that's not needed anymore + assert value == QtCore.QCoreApplication.instance() + import inspect + stack = inspect.stack() + f = stack[1] + log.warning("MetadataItem.tagger property set at %s:%d in %s", f.filename, f.lineno, f.function) + def update_metadata_images(self): if not self.suspend_metadata_images_update and self.can_show_coverart: if self.update_metadata_images_from_children(): diff --git a/picard/track.py b/picard/track.py index 7d42fd3ff..a25be46f1 100644 --- a/picard/track.py +++ b/picard/track.py @@ -129,7 +129,6 @@ class Track(DataObject, FileListItem): def __init__(self, track_id, album=None): DataObject.__init__(self, track_id) FileListItem.__init__(self) - self.tagger = QtCore.QCoreApplication.instance() self.album = album self.scripted_metadata = Metadata() self._track_artists = [] From 7e3a5d9c58674f93f473848fb14c9c15ad5e32a1 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 30 May 2024 19:17:01 +0200 Subject: [PATCH 19/20] Declare class ImageListState at module level --- picard/item.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/picard/item.py b/picard/item.py index 6f0f09869..50b881ab8 100644 --- a/picard/item.py +++ b/picard/item.py @@ -31,6 +31,7 @@ from picard import log from picard.i18n import ngettext from picard.metadata import Metadata from picard.util import IgnoreUpdatesContext +from picard.util.imagelist import ImageList class Item: @@ -158,6 +159,23 @@ class Item: number_of_images) % number_of_images +class ImageListState: + def __init__(self): + self.images = {} + self.has_common_images = True + self.first_obj = True + + def process_images(self, src_obj_metadata): + src_dict = src_obj_metadata.images.hash_dict() + prev_len = len(self.images) + self.images.update(src_dict) + if len(self.images) != prev_len: + if not self.first_obj: + self.has_common_images = False + if self.first_obj: + self.first_obj = False + + class MetadataItem(Item): metadata_images_changed = QtCore.pyqtSignal() @@ -254,24 +272,6 @@ class MetadataItem(Item): Returns: bool: True, if images where changed, False otherwise """ - from picard.util.imagelist import ImageList - - class ImageListState: - def __init__(self): - self.images = {} - self.has_common_images = True - self.first_obj = True - - def process_images(self, src_obj_metadata): - src_dict = src_obj_metadata.images.hash_dict() - prev_len = len(self.images) - self.images.update(src_dict) - if len(self.images) != prev_len: - if not self.first_obj: - self.has_common_images = False - if self.first_obj: - self.first_obj = False - changed = False for metadata_attr in self.update_children_metadata_attrs: From 1a853363b59b40e42b91c8957144b4ff0c0d1495 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 31 May 2024 14:10:36 +0200 Subject: [PATCH 20/20] UpdateCheckManager: no need to inherit from QtCore.QObject and pass tagger object from caller - _parent is actually the main window, so derive it from tagger --- picard/tagger.py | 2 +- picard/util/checkupdate.py | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/picard/tagger.py b/picard/tagger.py index 398a86910..31ab964c5 100644 --- a/picard/tagger.py +++ b/picard/tagger.py @@ -381,7 +381,7 @@ class Tagger(QtWidgets.QApplication): # Load release version information if self.autoupdate_enabled: - self.updatecheckmanager = UpdateCheckManager(parent=self.window) + self.updatecheckmanager = UpdateCheckManager(self) @property def is_wayland(self): diff --git a/picard/util/checkupdate.py b/picard/util/checkupdate.py index 90540a7e5..76076890b 100644 --- a/picard/util/checkupdate.py +++ b/picard/util/checkupdate.py @@ -23,7 +23,6 @@ from functools import partial -from PyQt6 import QtCore from PyQt6.QtWidgets import QMessageBox from picard import ( @@ -47,12 +46,10 @@ from picard.version import ( ) -class UpdateCheckManager(QtCore.QObject): +class UpdateCheckManager: - def __init__(self, parent=None): - super().__init__(parent=parent) - self.tagger = QtCore.QCoreApplication.instance() - self._parent = parent + def __init__(self, tagger): + self.tagger = tagger self._available_versions = {} self._show_always = False self._update_level = 0 @@ -107,7 +104,7 @@ class UpdateCheckManager(QtCore.QObject): log.error(_("Error loading Picard releases list: {error_message}").format(error_message=reply.errorString(),)) if self._show_always: QMessageBox.information( - self._parent, + self.tagger.window, _("Picard Update"), _("Unable to retrieve the latest version information from the website.\n({url})").format( url=PLUGINS_API['urls']['releases'], @@ -141,7 +138,7 @@ class UpdateCheckManager(QtCore.QObject): high_version = test_version if key: if QMessageBox.information( - self._parent, + self.tagger.window, _("Picard Update"), _("A new version of Picard is available.\n\n" "This version: {picard_old_version}\n" @@ -161,7 +158,7 @@ class UpdateCheckManager(QtCore.QObject): else: update_level = N_("unknown") QMessageBox.information( - self._parent, + self.tagger.window, _("Picard Update"), _("There is no update currently available for your subscribed update level: {update_level}\n\n" "Your version: {picard_old_version}\n").format(