diff --git a/picard/ui/itemviews/__init__.py b/picard/ui/itemviews/__init__.py index abf1d38266..3e2e07b59f 100644 --- a/picard/ui/itemviews/__init__.py +++ b/picard/ui/itemviews/__init__.py @@ -55,7 +55,7 @@ from picard import log from picard.album import NatAlbum -from picard.cluster import ClusterList, UnclusteredFiles +from picard.cluster import Cluster, ClusterList from picard.file import ( File, FileErrorType, @@ -73,14 +73,14 @@ from picard.ui.columns import ( ColumnAlign, ColumnSortType, + ImageColumn, ) from picard.ui.itemviews.basetreeview import BaseTreeView from picard.ui.itemviews.columns import ( ALBUMVIEW_COLUMNS, FILEVIEW_COLUMNS, - IconColumn, ) -from picard.ui.itemviews.match_quality_column import MatchQualityColumn +from picard.ui.itemviews.custom_columns import DelegateColumn def get_match_color(similarity, basecolor): @@ -408,11 +408,12 @@ def update_colums_text(self, color=None, bgcolor=None): self.setForeground(i, color) if bgcolor is not None: self.setBackground(i, bgcolor) - if isinstance(column, IconColumn): - self.setSizeHint(i, column.size) - elif isinstance(column, MatchQualityColumn): - # Progress columns are handled by delegate, just set size hint + if isinstance(column, ImageColumn): self.setSizeHint(i, column.size) + elif isinstance(column, DelegateColumn): + # Delegate columns are handled by delegate, just set size hint + if hasattr(column, 'size'): + self.setSizeHint(i, column.size) else: if column.align == ColumnAlign.RIGHT: self.setTextAlignment(i, QtCore.Qt.AlignmentFlag.AlignRight | QtCore.Qt.AlignmentFlag.AlignVCenter) @@ -420,11 +421,14 @@ def update_colums_text(self, color=None, bgcolor=None): if isinstance(column, CustomColumn): # Invalidate caches for this object to reflect tag changes column.invalidate_cache(self.obj) - # Hide custom column values for container rows like - # "Clusters" and "Unclustered Files". - # These represent unrelated collections and should not - # display per-entity values. - if isinstance(self.obj, ClusterList | UnclusteredFiles): + # Hide custom column values for container/group rows, but preserve Title and status icon. + # - ClusterList: Represents the "Clusters" root. Title is set elsewhere. + # - Special Cluster instances (e.g. "Unclustered Files"): Should show their Title + # but no other per-entity values in custom columns. + is_group_row = isinstance(self.obj, ClusterList) or ( + isinstance(self.obj, Cluster) and getattr(self.obj, 'special', False) + ) + if is_group_row and (column.key != 'title' and not column.status_icon): self.setText(i, "") continue diff --git a/picard/ui/itemviews/basetreeview.py b/picard/ui/itemviews/basetreeview.py index 7e0f1eb035..c493c0006d 100644 --- a/picard/ui/itemviews/basetreeview.py +++ b/picard/ui/itemviews/basetreeview.py @@ -91,8 +91,8 @@ from picard.ui.collectionmenu import CollectionMenu from picard.ui.enums import MainAction from picard.ui.filter import Filter +from picard.ui.itemviews.custom_columns import DelegateColumn from picard.ui.itemviews.events import header_events -from picard.ui.itemviews.match_quality_column import MatchQualityColumn, MatchQualityColumnDelegate from picard.ui.ratingwidget import RatingWidget from picard.ui.scriptsmenu import ScriptsMenu from picard.ui.util import menu_builder @@ -481,11 +481,14 @@ def _init_header(self): header = ConfigurableColumnsHeader(self.columns, parent=self) self.setHeader(header) - # Set up delegate for progress columns - progress_delegate = MatchQualityColumnDelegate(self) + # Set up delegates for delegate columns + delegate_instances = {} for i, column in enumerate(self.columns): - if isinstance(column, MatchQualityColumn): - self.setItemDelegateForColumn(i, progress_delegate) + if isinstance(column, DelegateColumn): + delegate_class = column.delegate_class + if delegate_class not in delegate_instances: + delegate_instances[delegate_class] = delegate_class(self) + self.setItemDelegateForColumn(i, delegate_instances[delegate_class]) self.restore_default_columns() self.restore_state() diff --git a/picard/ui/itemviews/columns.py b/picard/ui/itemviews/columns.py index 3383cf9a5d..f165751670 100644 --- a/picard/ui/itemviews/columns.py +++ b/picard/ui/itemviews/columns.py @@ -45,8 +45,6 @@ from PyQt6 import QtCore -from picard.album import AlbumStatus -from picard.const.sys import IS_LINUX from picard.i18n import N_ from picard.util import icontheme @@ -55,144 +53,169 @@ ColumnAlign, Columns, ColumnSortType, - DefaultColumn, - ImageColumn, ) -from picard.ui.itemviews.match_quality_column import MatchQualityColumn - - -def _sortkey_length(obj): - return obj.metadata.length or 0 - - -def _sortkey_filesize(obj): - try: - return int(obj.metadata['~filesize'] or obj.orig_metadata['~filesize']) - except ValueError: - return 0 - - -def _sortkey_bitrate(obj): - try: - return float(obj.metadata['~bitrate'] or obj.orig_metadata['~bitrate'] or 0) - except (ValueError, TypeError): - return 0 - - -def _sortkey_match_quality(obj): - """Sort key for match quality column - sort by completion percentage.""" - if hasattr(obj, 'get_num_matched_tracks') and hasattr(obj, 'tracks'): - # Album object - # Check if album is still loading - if so, return 0 to avoid premature sorting - if hasattr(obj, 'status') and obj.status == AlbumStatus.LOADING: - return 0.0 - - total = len(obj.tracks) if obj.tracks else 0 - if total > 0: - # Column sorting is reversed on Linux - multiplier = -1 if IS_LINUX else 1 - matched = obj.get_num_matched_tracks() - return matched / total * multiplier - return 0.0 - # For track objects, return 0 since we don't show icons at track level - return 0.0 - - -class IconColumn(ImageColumn): - _header_icon = None - header_icon_func = None - header_icon_size = QtCore.QSize(0, 0) - header_icon_border = 0 - - @property - def header_icon(self): - # icon cannot be set before QApplication is created - # so create it during runtime and cache it - # Avoid error: QPixmap: Must construct a QGuiApplication before a QPixmap - if self._header_icon is None: - self._header_icon = self.header_icon_func() - return self._header_icon - - def set_header_icon_size(self, width, height, border): - self.header_icon_size = QtCore.QSize(width, height) - self.header_icon_border = border - self.size = QtCore.QSize(width + 2 * border, height + 2 * border) - self.width = self.size.width() - - def paint(self, painter, rect): - icon = self.header_icon - if not icon: - return - h = self.header_icon_size.height() - w = self.header_icon_size.width() - border = self.header_icon_border - padding_v = (rect.height() - h) // 2 - target_rect = QtCore.QRect(rect.x() + border, rect.y() + padding_v, w, h) - painter.drawPixmap(target_rect, icon.pixmap(self.header_icon_size)) - - -_fingerprint_column = IconColumn(N_("Fingerprint status"), '~fingerprint') -_fingerprint_column.header_icon_func = lambda: icontheme.lookup('fingerprint-gray', icontheme.ICON_SIZE_MENU) -_fingerprint_column.set_header_icon_size(16, 16, 1) - -_match_quality_column = MatchQualityColumn(N_("Match"), '~match_quality', width=57) -_match_quality_column.sortable = True -_match_quality_column.sort_type = ColumnSortType.SORTKEY -_match_quality_column.sortkey = _sortkey_match_quality -_match_quality_column.is_default = True - - -# Common columns used by both views -_common_columns = ( - DefaultColumn(N_("Title"), 'title', sort_type=ColumnSortType.NAT, width=250, always_visible=True, status_icon=True), - DefaultColumn( +from picard.ui.itemviews.custom_columns import make_delegate_column, make_field_column, make_numeric_field_column +from picard.ui.itemviews.custom_columns.factory import make_icon_header_column +from picard.ui.itemviews.custom_columns.providers import LazyHeaderIconProvider +from picard.ui.itemviews.custom_columns.sorting_adapters import NumericSortAdapter +from picard.ui.itemviews.custom_columns.utils import ( + parse_bitrate, + parse_file_size, + parse_time_format, +) +from picard.ui.itemviews.match_quality_column import MatchQualityProvider + + +def create_match_quality_column(): + """Create a match quality delegate column with proper sorting. + + Returns + ------- + DelegateColumn + The configured match quality column. + """ + base_provider = MatchQualityProvider() + sorter = NumericSortAdapter(base_provider) + column = make_delegate_column( + N_("Match"), + '~match_quality', + base_provider, + width=57, + sort_type=ColumnSortType.SORTKEY, + size=QtCore.QSize(16, 16), + sort_provider=sorter, + ) + column.is_default = True + return column + + +def create_fingerprint_status_column(): + """Create the fingerprint status icon header column. + + Returns + ------- + IconColumn + The configured fingerprint status column. + """ + provider = LazyHeaderIconProvider(lambda: icontheme.lookup('fingerprint-gray', icontheme.ICON_SIZE_MENU)) + column = make_icon_header_column( + N_("Fingerprint status"), + '~fingerprint', + provider, + icon_width=16, + icon_height=16, + border=1, + ) + return column + + +def create_common_columns() -> tuple[Column, ...]: + """Create the built-in common columns using factories. + + Returns + ------- + tuple + Tuple of configured column objects for both views. + """ + # Title (status icon column) + title_col = make_field_column( + N_("Title"), + 'title', + sort_type=ColumnSortType.NAT, + width=250, + always_visible=True, + status_icon=True, + is_default=True, + ) + + # Length with numeric sort key from metadata.length + length_col = make_numeric_field_column( N_("Length"), '~length', - align=ColumnAlign.RIGHT, - sort_type=ColumnSortType.SORTKEY, - sortkey=_sortkey_length, + parse_time_format, width=50, - ), - DefaultColumn(N_("Artist"), 'artist', width=200), - Column(N_("Album Artist"), 'albumartist'), - Column(N_("Composer"), 'composer'), - Column(N_("Album"), 'album', sort_type=ColumnSortType.NAT), - Column(N_("Disc Subtitle"), 'discsubtitle', sort_type=ColumnSortType.NAT), - Column(N_("Track No."), 'tracknumber', align=ColumnAlign.RIGHT, sort_type=ColumnSortType.NAT), - Column(N_("Disc No."), 'discnumber', align=ColumnAlign.RIGHT, sort_type=ColumnSortType.NAT), - Column(N_("Catalog No."), 'catalognumber', sort_type=ColumnSortType.NAT), - Column(N_("Barcode"), 'barcode'), - Column(N_("Media"), 'media'), - Column( + align=ColumnAlign.RIGHT, + is_default=True, + ) + + # Artist + artist_col = make_field_column(N_("Artist"), 'artist', width=200, is_default=True) + + # Others (mostly field columns) + album_artist = make_field_column(N_("Album Artist"), 'albumartist') + composer = make_field_column(N_("Composer"), 'composer') + album = make_field_column(N_("Album"), 'album', sort_type=ColumnSortType.NAT) + discsubtitle = make_field_column(N_("Disc Subtitle"), 'discsubtitle', sort_type=ColumnSortType.NAT) + trackno = make_field_column(N_("Track No."), 'tracknumber', align=ColumnAlign.RIGHT, sort_type=ColumnSortType.NAT) + discno = make_field_column(N_("Disc No."), 'discnumber', align=ColumnAlign.RIGHT, sort_type=ColumnSortType.NAT) + catalognumber = make_field_column(N_("Catalog No."), 'catalognumber', sort_type=ColumnSortType.NAT) + barcode = make_field_column(N_("Barcode"), 'barcode') + media = make_field_column(N_("Media"), 'media') + + # Size with numeric sort key + size_col = make_numeric_field_column( N_("Size"), '~filesize', + parse_file_size, align=ColumnAlign.RIGHT, - sort_type=ColumnSortType.SORTKEY, - sortkey=_sortkey_filesize, - ), - Column(N_("File Type"), '~format', width=120), - Column( + ) + + # File Type + filetype = make_field_column(N_("File Type"), '~format', width=120) + + # Bitrate + bitrate = make_numeric_field_column( N_("Bitrate"), '~bitrate', - align=ColumnAlign.RIGHT, - sort_type=ColumnSortType.SORTKEY, - sortkey=_sortkey_bitrate, + parse_bitrate, width=80, - ), - Column(N_("Genre"), 'genre'), - _fingerprint_column, - Column(N_("Date"), 'date'), - Column(N_("Original Release Date"), 'originaldate'), - Column(N_("Release Date"), 'releasedate'), - Column(N_("Cover"), 'covercount'), - Column(N_("Cover Dimensions"), 'coverdimensions'), -) + align=ColumnAlign.RIGHT, + ) + + genre = make_field_column(N_("Genre"), 'genre') + + fingerprint = create_fingerprint_status_column() + + date = make_field_column(N_("Date"), 'date') + originaldate = make_field_column(N_("Original Release Date"), 'originaldate') + releasedate = make_field_column(N_("Release Date"), 'releasedate') + cover = make_field_column(N_("Cover"), 'covercount') + coverdims = make_field_column(N_("Cover Dimensions"), 'coverdimensions') + + return ( + title_col, + length_col, + artist_col, + album_artist, + composer, + album, + discsubtitle, + trackno, + discno, + catalognumber, + barcode, + media, + size_col, + filetype, + bitrate, + genre, + fingerprint, + date, + originaldate, + releasedate, + cover, + coverdims, + ) +# Common columns used by both views +_common_columns = create_common_columns() + # File view columns (without match quality column) FILEVIEW_COLUMNS = Columns(_common_columns, default_width=100) # Album view columns (with match quality column) # Insert `_match_quality_column` after Title, Length, Artist, Album Artist ALBUMVIEW_COLUMNS = Columns(_common_columns, default_width=100) +_match_quality_column = create_match_quality_column() ALBUMVIEW_COLUMNS.insert(ALBUMVIEW_COLUMNS.pos('albumartist') + 1, _match_quality_column) diff --git a/picard/ui/itemviews/custom_columns/__init__.py b/picard/ui/itemviews/custom_columns/__init__.py index 671c220433..cc54dfa665 100644 --- a/picard/ui/itemviews/custom_columns/__init__.py +++ b/picard/ui/itemviews/custom_columns/__init__.py @@ -22,17 +22,21 @@ from __future__ import annotations -from picard.ui.itemviews.custom_columns.column import CustomColumn +from picard.ui.itemviews.custom_columns.column import CustomColumn, DelegateColumn, IconColumn from picard.ui.itemviews.custom_columns.factory import ( _create_custom_column, make_callable_column, + make_delegate_column, make_field_column, + make_numeric_field_column, make_provider_column, make_script_column, make_transformed_column, ) from picard.ui.itemviews.custom_columns.protocols import ( ColumnValueProvider, + DelegateProvider, + HeaderIconProvider, SortKeyProvider, ) from picard.ui.itemviews.custom_columns.registry import registry @@ -52,13 +56,19 @@ __all__ = [ "ColumnValueProvider", + "DelegateProvider", "SortKeyProvider", + "HeaderIconProvider", "CustomColumn", + "DelegateColumn", + "IconColumn", "make_field_column", + "make_numeric_field_column", "make_provider_column", "make_script_column", "make_callable_column", "make_transformed_column", + "make_delegate_column", "registry", "_create_custom_column", # Sorting adapters diff --git a/picard/ui/itemviews/custom_columns/column.py b/picard/ui/itemviews/custom_columns/column.py index 68b08afad0..e2df9d1221 100644 --- a/picard/ui/itemviews/custom_columns/column.py +++ b/picard/ui/itemviews/custom_columns/column.py @@ -22,14 +22,19 @@ from __future__ import annotations +from PyQt6 import QtCore, QtGui + from picard.ui.columns import ( Column, ColumnAlign, ColumnSortType, + ImageColumn, ) from picard.ui.itemviews.custom_columns.protocols import ( CacheInvalidatable, ColumnValueProvider, + DelegateProvider, + HeaderIconProvider, SortKeyProvider, ) @@ -46,6 +51,8 @@ def __init__( align: ColumnAlign = ColumnAlign.LEFT, sort_type: ColumnSortType = ColumnSortType.TEXT, always_visible: bool = False, + *, + status_icon: bool = False, ): """Create custom column. @@ -79,7 +86,7 @@ def __init__( sort_type=sort_type, sortkey=sortkey_fn, always_visible=always_visible, - status_icon=False, + status_icon=status_icon, ) self.provider = provider @@ -94,3 +101,145 @@ def invalidate_cache(self, obj=None): # pragma: no cover - UI-driven if isinstance(self.provider, CacheInvalidatable): # type: ignore[attr-defined] self.provider.invalidate(obj) # noqa: PGH003 + + +class DelegateColumn(Column): + """A column that uses a delegate for custom rendering and optional sorting.""" + + def __init__( + self, + title: str, + key: str, + provider: DelegateProvider, + width: int | None = None, + align: ColumnAlign = ColumnAlign.LEFT, + sort_type: ColumnSortType = ColumnSortType.TEXT, + always_visible: bool = False, + size: QtCore.QSize | None = None, + *, + status_icon: bool = False, + sort_provider: SortKeyProvider | None = None, + ): + """Create delegate column. + + Parameters + ---------- + title + Display title of the column. + key + Internal key used to identify the column. + provider + Provide delegate class and data for rendering. + width + Optional fixed width in pixels. + align + Set text alignment. + sort_type + Set sorting behavior of the column. + always_visible + If True, hide the column visibility toggle. + size + Preferred delegate size. + """ + + sortkey_fn = None + if sort_type == ColumnSortType.SORTKEY: + if isinstance(sort_provider, SortKeyProvider): + sortkey_fn = sort_provider.sort_key # type: ignore[assignment] + elif isinstance(provider, SortKeyProvider): + # Fallback for backward compatibility if provider implements sorting + sortkey_fn = provider.sort_key # type: ignore[assignment] + + super().__init__( + title, + key, + width=width, + align=align, + sort_type=sort_type, + sortkey=sortkey_fn, + always_visible=always_visible, + status_icon=status_icon, + ) + self.delegate_provider = provider + self.size = size if size is not None else QtCore.QSize(16, 16) # Default icon size + + @property + def delegate_class(self): + """Get the delegate class for this column.""" + return self.delegate_provider.get_delegate_class() + + +class IconColumn(ImageColumn): + """A column that paints a header icon provided by a `HeaderIconProvider`. + + The header icon is constructed lazily via the provider to avoid creating + Qt objects before the application is initialized. + """ + + _header_icon: QtGui.QIcon | None = None + + def __init__(self, title: str, key: str, provider: HeaderIconProvider, *, width: int | None = None) -> None: + super().__init__( + title, + key, + width=width, + align=ColumnAlign.LEFT, + sort_type=ColumnSortType.TEXT, + sortkey=None, + always_visible=False, + status_icon=False, + ) + self._provider = provider + self.header_icon_size = QtCore.QSize(0, 0) + self.header_icon_border = 0 + self.size = QtCore.QSize(0, 0) + + @property + def header_icon(self) -> QtGui.QIcon | None: + """Get the header icon for this column. + + Returns + ------- + QtGui.QIcon | None + The header icon, or None if no icon is available. + """ + if self._header_icon is None: + self._header_icon = self._provider.get_icon() + return self._header_icon + + def set_header_icon_size(self, width: int, height: int, border: int) -> None: + """Set the header icon size and border. + + Parameters + ---------- + width : int + Width of the icon in pixels. + height : int + Height of the icon in pixels. + border : int + Border size around the icon in pixels. + """ + self.header_icon_size = QtCore.QSize(width, height) + self.header_icon_border = border + self.size = QtCore.QSize(width + 2 * border, height + 2 * border) + self.width = self.size.width() + + def paint(self, painter: QtGui.QPainter, rect: QtCore.QRect) -> None: # pragma: no cover - GUI rendering + """Paint the header icon in the specified rectangle. + + Parameters + ---------- + painter : QtGui.QPainter + The painter to use for drawing. + rect : QtCore.QRect + The rectangle where the icon should be painted. + """ + icon = self.header_icon + if not icon: + return + h = self.header_icon_size.height() + w = self.header_icon_size.width() + border = self.header_icon_border + padding_v = (rect.height() - h) // 2 + target_rect = QtCore.QRect(rect.x() + border, rect.y() + padding_v, w, h) + painter.drawPixmap(target_rect, icon.pixmap(self.header_icon_size)) diff --git a/picard/ui/itemviews/custom_columns/factory.py b/picard/ui/itemviews/custom_columns/factory.py index 612723baf3..768ee06a2a 100644 --- a/picard/ui/itemviews/custom_columns/factory.py +++ b/picard/ui/itemviews/custom_columns/factory.py @@ -24,6 +24,8 @@ from collections.abc import Callable +from PyQt6 import QtCore + from picard.item import Item from picard.script import ScriptParser @@ -31,9 +33,11 @@ ColumnAlign, ColumnSortType, ) -from picard.ui.itemviews.custom_columns.column import CustomColumn +from picard.ui.itemviews.custom_columns.column import CustomColumn, DelegateColumn, IconColumn from picard.ui.itemviews.custom_columns.protocols import ( ColumnValueProvider, + DelegateProvider, + HeaderIconProvider, SortKeyProvider, ) from picard.ui.itemviews.custom_columns.providers import ( @@ -42,6 +46,7 @@ TransformProvider, ) from picard.ui.itemviews.custom_columns.script_provider import ChainedValueProvider +from picard.ui.itemviews.custom_columns.sorting_adapters import NumericSortAdapter def _infer_sort_type(provider: ColumnValueProvider, sort_type: ColumnSortType | None) -> ColumnSortType: @@ -75,6 +80,8 @@ def _create_custom_column( align: ColumnAlign = ColumnAlign.LEFT, always_visible: bool = False, sort_type: ColumnSortType | None = None, + status_icon: bool = False, + is_default: bool = False, ) -> CustomColumn: """Create `CustomColumn`. @@ -93,7 +100,7 @@ def _create_custom_column( # downgrade to TEXT to avoid invalid configuration if inferred_sort_type == ColumnSortType.SORTKEY and not isinstance(provider, SortKeyProvider): inferred_sort_type = ColumnSortType.TEXT - return CustomColumn( + column = CustomColumn( title, key, provider, @@ -101,7 +108,10 @@ def _create_custom_column( align=align, sort_type=inferred_sort_type, always_visible=always_visible, + status_icon=status_icon, ) + column.is_default = is_default + return column def make_field_column( @@ -111,18 +121,35 @@ def make_field_column( width: int | None = None, align: ColumnAlign = ColumnAlign.LEFT, always_visible: bool = False, + sort_type: ColumnSortType = ColumnSortType.TEXT, + status_icon: bool = False, + is_default: bool = False, ) -> CustomColumn: - """Create column that displays a field via `obj.column(key)`. + """Create a column that displays a field via ``obj.column(key)``. Parameters ---------- - title, key, width, align, always_visible - Column configuration. + title : str + Display title of the column. + key : str + Internal key used to identify the column and field source. + width : int | None, optional + Fixed width in pixels. If ``None``, uses default behavior. + align : ColumnAlign, default ``ColumnAlign.LEFT`` + Text alignment for the cell contents. + always_visible : bool, default ``False`` + If ``True``, hides the column visibility toggle in the UI. + sort_type : ColumnSortType, default ``ColumnSortType.TEXT`` + Sorting behavior for the column. + status_icon : bool, default ``False`` + If ``True``, marks this as the single status icon column. + is_default : bool, default ``False`` + If ``True``, marks this column as visible by default. Returns ------- CustomColumn - The field column. + The configured field column. """ provider = FieldReferenceProvider(key) return _create_custom_column( @@ -132,7 +159,62 @@ def make_field_column( width=width, align=align, always_visible=always_visible, - sort_type=ColumnSortType.TEXT, + sort_type=sort_type, + status_icon=status_icon, + is_default=is_default, + ) + + +def make_numeric_field_column( + title: str, + key: str, + parser: Callable[[str], float] | None = None, + *, + width: int | None = None, + align: ColumnAlign = ColumnAlign.LEFT, + always_visible: bool = False, + status_icon: bool = False, + is_default: bool = False, +) -> CustomColumn: + """Create a field column with numeric sorting support. + + Parameters + ---------- + title : str + Display title of the column. + key : str + Internal key used to identify the column and field source. + parser : Callable[[str], float] | None, optional + Function to parse the field value to a numeric value for sorting. + If ``None``, uses the default ``float()`` parser. + width : int | None, optional + Fixed width in pixels. If ``None``, uses default behavior. + align : ColumnAlign, default ``ColumnAlign.LEFT`` + Text alignment for the cell contents. + always_visible : bool, default ``False`` + If ``True``, hides the column visibility toggle in the UI. + status_icon : bool, default ``False`` + If ``True``, marks this as the single status icon column. + is_default : bool, default ``False`` + If ``True``, marks this column as visible by default. + + Returns + ------- + CustomColumn + The configured numeric field column with proper sorting. + """ + base_provider = FieldReferenceProvider(key) + numeric_provider = NumericSortAdapter(base_provider, parser=parser) + return _create_custom_column( + title, + key, + numeric_provider, + width=width, + align=align, + always_visible=always_visible, + sort_type=ColumnSortType.SORTKEY, + status_icon=status_icon, + is_default=is_default, ) @@ -188,6 +270,7 @@ def make_callable_column( align: ColumnAlign = ColumnAlign.LEFT, always_visible: bool = False, sort_type: ColumnSortType | None = None, + status_icon: bool = False, ) -> CustomColumn: """Create column backed by a Python callable. @@ -210,6 +293,7 @@ def make_callable_column( align=align, always_visible=always_visible, sort_type=sort_type, + status_icon=status_icon, ) @@ -281,3 +365,66 @@ def make_provider_column( always_visible=always_visible, sort_type=sort_type, ) + + +def make_delegate_column( + title: str, + key: str, + provider: DelegateProvider, + *, + width: int | None = None, + align: ColumnAlign = ColumnAlign.LEFT, + always_visible: bool = False, + sort_type: ColumnSortType | None = None, + size: QtCore.QSize | None = None, + sort_provider: SortKeyProvider | None = None, +) -> DelegateColumn: + """Create column that uses a delegate for custom rendering. + + Parameters + ---------- + title, key, provider, width, align, always_visible, sort_type + Column configuration. + + Returns + ------- + DelegateColumn + The delegate column. + """ + resolved_sort_type = sort_type or ColumnSortType.TEXT + return DelegateColumn( + title, + key, + provider, + width=width, + align=align, + sort_type=resolved_sort_type, + always_visible=always_visible, + size=size, + sort_provider=sort_provider, + ) + + +def make_icon_header_column( + title: str, + key: str, + provider: HeaderIconProvider, + *, + icon_width: int, + icon_height: int, + border: int = 0, +) -> IconColumn: + """Create an icon header column from a header icon provider. + + Parameters + ---------- + title, key + Column configuration. + provider + Object implementing `HeaderIconProvider`. + icon_width, icon_height, border + Header icon sizing and border. + """ + column = IconColumn(title, key, provider, width=None) + column.set_header_icon_size(icon_width, icon_height, border) + return column diff --git a/picard/ui/itemviews/custom_columns/protocols.py b/picard/ui/itemviews/custom_columns/protocols.py index e0c8c49d6c..236e86aad3 100644 --- a/picard/ui/itemviews/custom_columns/protocols.py +++ b/picard/ui/itemviews/custom_columns/protocols.py @@ -27,6 +27,8 @@ runtime_checkable, ) +from PyQt6 import QtGui + from picard.item import Item @@ -78,3 +80,31 @@ def sort_key(self, obj: Item): # pragma: no cover - optional Sort key. """ ... + + +@runtime_checkable +class DelegateProvider(Protocol): + """Protocol for columns that require custom rendering via delegates.""" + + def get_delegate_class(self) -> type: + """Return the delegate class for custom rendering. + + Returns + ------- + type + The delegate class (subclass of QStyledItemDelegate). + """ + ... + + +@runtime_checkable +class HeaderIconProvider(Protocol): + """Protocol for providing a header icon for an icon column. + + Implementations should construct icons lazily (after QApplication exists) + and can internally cache the icon. + """ + + def get_icon(self) -> QtGui.QIcon: # pragma: no cover - QtGui.QIcon + """Return a QIcon used for the column header.""" + ... diff --git a/picard/ui/itemviews/custom_columns/providers.py b/picard/ui/itemviews/custom_columns/providers.py index 18027179d9..3d6b84a828 100644 --- a/picard/ui/itemviews/custom_columns/providers.py +++ b/picard/ui/itemviews/custom_columns/providers.py @@ -26,11 +26,13 @@ from dataclasses import dataclass import re +from PyQt6 import QtGui + from picard import log from picard.item import Item from picard.script.parser import normalize_tagname -from picard.ui.itemviews.custom_columns.protocols import ColumnValueProvider +from picard.ui.itemviews.custom_columns.protocols import ColumnValueProvider, HeaderIconProvider @dataclass(frozen=True) @@ -105,3 +107,23 @@ def evaluate(self, obj: Item) -> str: def __repr__(self) -> str: # pragma: no cover - debug helper func_name = getattr(self._func, "__name__", repr(self._func)) return f"{self.__class__.__name__}(func={func_name})" + + +class LazyHeaderIconProvider(HeaderIconProvider): + """Provide a header icon lazily via a callable. + + Parameters + ---------- + factory + A zero-argument callable that returns a `QtGui.QIcon` when invoked. + The icon is created only once and cached. + """ + + def __init__(self, factory: Callable[[], QtGui.QIcon]): + self._factory = factory + self._icon: QtGui.QIcon | None = None + + def get_icon(self) -> QtGui.QIcon: # pragma: no cover - Qt object + if self._icon is None: + self._icon = self._factory() + return self._icon diff --git a/picard/ui/itemviews/custom_columns/utils.py b/picard/ui/itemviews/custom_columns/utils.py new file mode 100644 index 0000000000..38816251f4 --- /dev/null +++ b/picard/ui/itemviews/custom_columns/utils.py @@ -0,0 +1,120 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# +# Copyright (C) 2025 The MusicBrainz Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the GNU General Public License Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +"""Utility functions for custom column parsing and formatting.""" + +from __future__ import annotations + +import re + + +def parse_time_format(time_str: str) -> float: + """Parse time format (e.g., '3:00', '1:02:59') to seconds. + + Parameters + ---------- + time_str : str + Time string in format MM:SS or HH:MM:SS. + + Returns + ------- + float + Time in seconds. + + Raises + ------ + ValueError + If the time format is invalid. + """ + if not time_str or time_str == "?:??": + return 0.0 + parts = time_str.split(':') + if len(parts) == 2: # MM:SS + minutes, seconds = map(float, parts) + return minutes * 60 + seconds + elif len(parts) == 3: # HH:MM:SS + hours, minutes, seconds = map(float, parts) + return hours * 3600 + minutes * 60 + seconds + else: + raise ValueError(f"Invalid time format: {time_str}") + + +def parse_file_size(size_str: str) -> float: + """Parse file size format (e.g., '1.0 MB', '1.0 MiB') to bytes. + + Parameters + ---------- + size_str : str + File size string with optional unit (B, KB, MB, GB, TB). + + Returns + ------- + float + Size in bytes. + + Raises + ------ + ValueError + If the size format is invalid. + """ + if not size_str: + return 0.0 + # Remove any parentheses content like "(1.0 MiB)" + size_str = size_str.split('(')[0].strip() + size_str = size_str.upper() + + # Extract number and unit + match = re.match(r'^([\d.]+)\s*([KMGT]?B?)$', size_str) + if not match: + raise ValueError(f"Invalid size format: {size_str}") + + number, unit = match.groups() + number = float(number) + + # Convert to bytes + multipliers = {'B': 1, 'KB': 1024, 'MB': 1024**2, 'GB': 1024**3, 'TB': 1024**4} + return number * multipliers.get(unit, 1) + + +def parse_bitrate(bitrate_str: str) -> float: + """Parse bitrate format (e.g., '320 kbps', '128 kbps') to kbps. + + Parameters + ---------- + bitrate_str : str + Bitrate string with numeric value. + + Returns + ------- + float + Bitrate in kbps. + + Raises + ------ + ValueError + If the bitrate format is invalid. + """ + if not bitrate_str: + return 0.0 + # Extract number from bitrate string + match = re.search(r'([\d.]+)', bitrate_str) + if not match: + raise ValueError(f"Invalid bitrate format: {bitrate_str}") + return float(match.group(1)) diff --git a/picard/ui/itemviews/match_quality_column.py b/picard/ui/itemviews/match_quality_column.py index 268257eb6e..da8450c485 100644 --- a/picard/ui/itemviews/match_quality_column.py +++ b/picard/ui/itemviews/match_quality_column.py @@ -21,11 +21,13 @@ from collections import namedtuple -from PyQt6 import QtCore, QtWidgets +from PyQt6 import QtCore, QtGui, QtWidgets from picard.i18n import gettext as _ +from picard.item import Item -from picard.ui.columns import ImageColumn +from picard.ui.itemviews.custom_columns import DelegateColumn +from picard.ui.itemviews.custom_columns.protocols import ColumnValueProvider, DelegateProvider # Structured container for delegate item data @@ -45,57 +47,58 @@ } -class MatchQualityColumn(ImageColumn): +class MatchQualityProvider(ColumnValueProvider, DelegateProvider): """Column that displays match quality using match icons at the release level.""" - def __init__(self, title, key, width=120): - super().__init__(title, key, width=width) - self.size = QtCore.QSize(16, 16) # Icon size - self.width = width + def __init__(self) -> None: + """Initialize the match quality provider.""" + self._delegate_class = MatchQualityColumnDelegate - def paint(self, painter, rect): - """Override paint method to prevent NotImplementedError from base class.""" - # This column is painted by the delegate, not the header - # So we just do nothing here to prevent the error - pass + def evaluate(self, obj: Item) -> str: + """Return the match percentage as a string for sorting. - def get_match_icon(self, obj): - """Get the appropriate match icon for the given object.""" - # Only show icons at the release (album) level, not track level + Parameters + ---------- + obj + The item to evaluate. + + Returns + ------- + str + The match percentage as a string (e.g., "0.75" for 75%). + """ if not hasattr(obj, 'get_num_matched_tracks') or not hasattr(obj, 'tracks'): - return None + return "0.0" # Album object total = len(obj.tracks) if obj.tracks else 0 if total == 0: - # Use pending icon for zero tracks - from picard.ui.itemviews import FileItem + return "0.0" - if hasattr(FileItem, 'match_pending_icons') and len(FileItem.match_pending_icons) > 5: - return FileItem.match_pending_icons[5] # match-pending-100.png - return None + get_num_matched_tracks = getattr(obj, 'get_num_matched_tracks', None) + if not callable(get_num_matched_tracks): + return "0.0" - # Calculate match percentage - matched = obj.get_num_matched_tracks() + matched = int(get_num_matched_tracks()) percentage = matched / total - - # Determine which icon to use based on percentage using thresholds - for threshold in sorted(THRESHOLD_TO_ICON_INDEX.keys(), reverse=True): - if percentage >= threshold: - icon_index = THRESHOLD_TO_ICON_INDEX[threshold] - break - - # Get the match icons from FileItem - from picard.ui.itemviews import FileItem - - if hasattr(FileItem, 'match_icons') and icon_index < len(FileItem.match_icons): - return FileItem.match_icons[icon_index] - - return None - - def get_match_stats(self, obj): - """Get comprehensive match statistics for the given object.""" + return str(percentage) + + def get_match_stats(self, obj: Item) -> dict[str, int] | None: + """Get comprehensive match statistics for the given object. + + Parameters + ---------- + obj + The item to evaluate. + + Returns + ------- + dict[str, int] | None + A dictionary with keys ``matched``, ``total``, ``unmatched``, + ``duplicates``, ``extra``, and ``missing``. Returns ``None`` if + stats are not available for the given object. + """ # Only show stats at the release (album) level, not track level if not hasattr(obj, 'get_num_matched_tracks') or not hasattr(obj, 'tracks'): return None @@ -141,11 +144,21 @@ def get_match_stats(self, obj): 'missing': missing, } + def get_delegate_class(self) -> type[QtWidgets.QStyledItemDelegate]: + """Return the delegate class for custom rendering. + + Returns + ------- + type[QtWidgets.QStyledItemDelegate] + The delegate class (subclass of QStyledItemDelegate). + """ + return self._delegate_class + class MatchQualityColumnDelegate(QtWidgets.QStyledItemDelegate): """Delegate for rendering match quality icons in tree items.""" - def __init__(self, parent=None): + def __init__(self, parent: QtCore.QObject | None = None) -> None: super().__init__(parent) def _get_item_data(self, index: QtCore.QModelIndex) -> MatchItemData | None: @@ -172,23 +185,67 @@ def _get_item_data(self, index: QtCore.QModelIndex) -> MatchItemData | None: return None column = columns[column_index] - if not isinstance(column, MatchQualityColumn): + if not isinstance(column, DelegateColumn): return None - stats = column.get_match_stats(obj) + # Get stats from the provider (not from delegate) + provider = column.delegate_provider + if hasattr(provider, '_base'): + provider = provider._base # Unwrap adapter + + stats = provider.get_match_stats(obj) if not stats: return None return MatchItemData(obj=obj, column=column, stats=stats) - def _format_tooltip_text(self, stats): - """Format stats into detailed tooltip text. + def _get_match_icon(self, obj: Item, column: DelegateColumn) -> QtGui.QIcon | None: + """Select the appropriate match icon using delegate-side logic.""" + # Compute stats using provider, but map to icons here (UI concern) + provider = column.delegate_provider + if hasattr(provider, '_base'): + provider = provider._base # Unwrap adapter - Args: - stats: Dictionary containing match statistics + stats = provider.get_match_stats(obj) + if not stats: + return None - Returns: - str: Formatted tooltip text + total = stats.get('total', 0) + if total == 0: + from picard.ui.itemviews import FileItem + + if hasattr(FileItem, 'match_pending_icons') and len(FileItem.match_pending_icons) > 5: + return FileItem.match_pending_icons[5] + return None + + matched = stats.get('matched', 0) + percentage = matched / total if total else 0.0 + + # Determine which icon index to use based on percentage using thresholds + icon_index = 0 + for threshold in sorted(THRESHOLD_TO_ICON_INDEX.keys(), reverse=True): + if percentage >= threshold: + icon_index = THRESHOLD_TO_ICON_INDEX[threshold] + break + + from picard.ui.itemviews import FileItem + + if hasattr(FileItem, 'match_icons') and icon_index < len(FileItem.match_icons): + return FileItem.match_icons[icon_index] + return None + + def _format_tooltip_text(self, stats: dict[str, int]) -> str: + """Format stats into a detailed tooltip string. + + Parameters + ---------- + stats + Dictionary containing integer match statistics. + + Returns + ------- + str + The formatted tooltip text. """ tooltip_parts = [] @@ -214,7 +271,23 @@ def _format_tooltip_text(self, stats): return "\n".join(tooltip_parts) - def paint(self, painter, option, index): + def paint( + self, + painter: QtGui.QPainter, + option: QtWidgets.QStyleOptionViewItem, + index: QtCore.QModelIndex, + ) -> None: + """Paint the match quality icon in the item cell. + + Parameters + ---------- + painter + The painter used for drawing. + option + The style options for the item. + index + The model index identifying the item. + """ # Initialize the style option self.initStyleOption(option, index) @@ -230,8 +303,8 @@ def paint(self, painter, option, index): if not item_data: return - # Get the match icon - icon = item_data.column.get_match_icon(item_data.obj) + # Get the match icon from the provider + icon = self._get_match_icon(item_data.obj, item_data.column) # Calculate layout icon_size = item_data.column.size @@ -243,8 +316,31 @@ def paint(self, painter, option, index): y = option.rect.y() + (option.rect.height() - icon_size.height()) // 2 icon.paint(painter, QtCore.QRect(x, y, icon_size.width(), icon_size.height())) - def helpEvent(self, event, view, option, index): - """Show tooltip with explanation of the stats.""" + def helpEvent( + self, + event: QtGui.QHelpEvent, + view: QtWidgets.QWidget, + option: QtWidgets.QStyleOptionViewItem, + index: QtCore.QModelIndex, + ) -> bool: + """Show a tooltip with an explanation of the stats. + + Parameters + ---------- + event + The help event providing cursor position. + view + The view showing the items. + option + The style options for the item. + index + The model index identifying the item. + + Returns + ------- + bool + ``True`` if the event was handled, else ``False``. + """ # Get item data item_data = self._get_item_data(index) if not item_data: @@ -257,6 +353,24 @@ def helpEvent(self, event, view, option, index): QtWidgets.QToolTip.showText(event.globalPos(), tooltip_text, view) return True - def sizeHint(self, option, index): + def sizeHint( + self, + option: QtWidgets.QStyleOptionViewItem, + index: QtCore.QModelIndex, + ) -> QtCore.QSize: + """Return the recommended size for the item cell. + + Parameters + ---------- + option + The style options for the item. + index + The model index identifying the item. + + Returns + ------- + QtCore.QSize + A size that accommodates both icon and text. + """ # Return a size that accommodates both icon and text return QtCore.QSize(57, 16) diff --git a/test/test_album_loading_sorting.py b/test/test_album_loading_sorting.py deleted file mode 100644 index 3612cabe01..0000000000 --- a/test/test_album_loading_sorting.py +++ /dev/null @@ -1,92 +0,0 @@ -# -*- coding: utf-8 -*- -# -# Picard, the next-generation MusicBrainz tagger -# -# Copyright (C) 2025 The MusicBrainz Team -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of the GNU General Public License -# as published by the GNU General Public License Foundation; either version 2 -# of the License, or (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, write to the Free Software -# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -from unittest.mock import Mock - -from picard.album import AlbumStatus -from picard.const.sys import IS_LINUX - -import pytest - -from picard.ui.itemviews.columns import _sortkey_match_quality - - -def _apply_platform_multiplier(value): - """Apply the same platform-specific multiplier logic as in _sortkey_match_quality.""" - multiplier = -1 if IS_LINUX else 1 - return value * multiplier - - -class TestAlbumLoadingSorting: - """Test that sorting works correctly during album loading.""" - - def test_sortkey_during_loading_returns_zero(self): - """Test that sort key returns 0.0 when album is still loading.""" - album = Mock() - album.status = AlbumStatus.LOADING - album.get_num_matched_tracks.return_value = 3 - album.tracks = [Mock(), Mock(), Mock(), Mock(), Mock()] # 5 tracks - - result = _sortkey_match_quality(album) - assert result == 0.0 - - def test_sortkey_after_loading_returns_correct_percentage(self): - """Test that sort key returns correct percentage after album is loaded.""" - album = Mock() - album.status = AlbumStatus.LOADED - album.get_num_matched_tracks.return_value = 3 - album.tracks = [Mock(), Mock(), Mock(), Mock(), Mock()] # 5 tracks - - result = _sortkey_match_quality(album) - expected = _apply_platform_multiplier(0.6) - assert result == expected - - def test_sortkey_no_status_attribute_works_normally(self): - """Test that objects without status attribute work normally.""" - album = Mock() - # No status attribute - album.get_num_matched_tracks.return_value = 2 - album.tracks = [Mock(), Mock(), Mock()] # 3 tracks - - result = _sortkey_match_quality(album) - expected = _apply_platform_multiplier(0.6666666666666666) - assert result == pytest.approx(expected, rel=1e-10) # 2/3 ≈ 0.666 - - def test_sortkey_error_status_works_normally(self): - """Test that albums with error status work normally.""" - album = Mock() - album.status = AlbumStatus.ERROR - album.get_num_matched_tracks.return_value = 1 - album.tracks = [Mock(), Mock()] # 2 tracks - - result = _sortkey_match_quality(album) - expected = _apply_platform_multiplier(0.5) - assert result == expected - - def test_sortkey_none_status_works_normally(self): - """Test that albums with None status work normally.""" - album = Mock() - album.status = None - album.get_num_matched_tracks.return_value = 4 - album.tracks = [Mock(), Mock(), Mock(), Mock(), Mock()] # 5 tracks - - result = _sortkey_match_quality(album) - expected = _apply_platform_multiplier(0.8) - assert result == expected diff --git a/test/test_itemviews_columns_split.py b/test/test_itemviews_columns_split.py index a01963a233..4f2d51947e 100644 --- a/test/test_itemviews_columns_split.py +++ b/test/test_itemviews_columns_split.py @@ -27,7 +27,7 @@ ALBUMVIEW_COLUMNS, FILEVIEW_COLUMNS, ) -from picard.ui.itemviews.match_quality_column import MatchQualityColumn +from picard.ui.itemviews.custom_columns import DelegateColumn @pytest.fixture @@ -74,11 +74,11 @@ def test_album_view_match_after_albumartist(album_keys: list[str]) -> None: assert idx_match == idx_albumartist + 1 -def test_album_view_match_is_match_quality_column_type() -> None: - # Ensure the actual column instance at the expected index is MatchQualityColumn +def test_album_view_match_is_delegate_column_type() -> None: + # Ensure the actual column instance at the expected index is DelegateColumn keys: list[str] = [c.key for c in ALBUMVIEW_COLUMNS] idx_match: int = _index_of(keys, "~match_quality") - assert isinstance(ALBUMVIEW_COLUMNS[idx_match], MatchQualityColumn) + assert isinstance(ALBUMVIEW_COLUMNS[idx_match], DelegateColumn) class _DummyObj: diff --git a/test/test_itemviews_match_quality.py b/test/test_itemviews_match_quality.py index a74aa2a0c8..0581e95da2 100644 --- a/test/test_itemviews_match_quality.py +++ b/test/test_itemviews_match_quality.py @@ -23,33 +23,24 @@ from PyQt6 import QtCore, QtGui, QtWidgets -from picard.album import Album, AlbumStatus -from picard.const.sys import IS_LINUX +from picard.album import Album from picard.track import Track import pytest from picard.ui.itemviews import TreeItem -from picard.ui.itemviews.columns import _sortkey_match_quality -from picard.ui.itemviews.match_quality_column import ( - MatchQualityColumn, - MatchQualityColumnDelegate, -) +from picard.ui.itemviews.columns import create_match_quality_column +from picard.ui.itemviews.custom_columns import DelegateColumn +from picard.ui.itemviews.match_quality_column import MatchQualityColumnDelegate, MatchQualityProvider -def _apply_platform_multiplier(value): - """Apply the same platform-specific multiplier logic as in _sortkey_match_quality.""" - multiplier = -1 if IS_LINUX else 1 - return value * multiplier - - -class TestMatchQualityColumn: - """Test the MatchQualityColumn class.""" +class TestMatchQualityProvider: + """Test the MatchQualityProvider class.""" @pytest.fixture - def match_quality_column(self) -> MatchQualityColumn: - """Create a MatchQualityColumn instance for testing.""" - return MatchQualityColumn("Match Quality", "~match_quality", width=120) + def provider(self) -> MatchQualityProvider: + """Create a MatchQualityProvider instance for testing.""" + return MatchQualityProvider() @pytest.fixture def mock_album(self) -> Mock: @@ -62,88 +53,49 @@ def mock_album(self) -> Mock: album.unmatched_files.files = [] return album - @pytest.fixture - def mock_track(self) -> Mock: - """Create a mock track.""" - track = Mock(spec=Track) - track.files = [] - return track - - def test_init(self, match_quality_column: MatchQualityColumn) -> None: - """Test MatchQualityColumn initialization.""" - assert match_quality_column.title == "Match Quality" - assert match_quality_column.key == "~match_quality" - assert match_quality_column.width == 120 - assert match_quality_column.size == QtCore.QSize(16, 16) - - def test_paint_does_nothing(self, match_quality_column: MatchQualityColumn) -> None: - """Test that paint method does nothing (handled by delegate).""" - painter = Mock() - rect = QtCore.QRect(0, 0, 100, 20) - # Should not raise any exception - match_quality_column.paint(painter, rect) + def test_init(self, provider: MatchQualityProvider) -> None: + """Test MatchQualityProvider initialization.""" + assert provider is not None + assert provider.get_delegate_class() == MatchQualityColumnDelegate @pytest.mark.parametrize( - ("matched", "total", "expected_icon_index"), + ("matched", "total", "expected_percentage"), [ - (0, 0, 5), # No tracks - use pending icon - (5, 10, 0), # 50% match - (6, 10, 1), # 60% match - (7, 10, 2), # 70% match - (8, 10, 3), # 80% match - (9, 10, 4), # 90% match - (10, 10, 5), # 100% match - (3, 10, 0), # 30% match - use worst icon + (0, 0, "0.0"), # No tracks + (5, 10, "0.5"), # 50% match + (6, 10, "0.6"), # 60% match + (7, 10, "0.7"), # 70% match + (8, 10, "0.8"), # 80% match + (9, 10, "0.9"), # 90% match + (10, 10, "1.0"), # 100% match + (3, 10, "0.3"), # 30% match ], ) - def test_get_match_icon_percentage_based( + def test_evaluate_percentage_calculation( self, - match_quality_column: MatchQualityColumn, + provider: MatchQualityProvider, mock_album: Mock, matched: int, total: int, - expected_icon_index: int, + expected_percentage: str, ) -> None: - """Test get_match_icon returns correct icon based on percentage.""" + """Test evaluate returns correct percentage as string.""" # Setup mock album mock_album.get_num_matched_tracks.return_value = matched mock_album.tracks = [Mock() for _ in range(total)] - # Mock FileItem.match_icons - patch the import inside the method - with patch("picard.ui.itemviews.FileItem") as mock_file_item: - mock_file_item.match_icons = [Mock() for _ in range(6)] - mock_file_item.match_pending_icons = [Mock() for _ in range(6)] - - icon = match_quality_column.get_match_icon(mock_album) - - if total == 0: - # Should use pending icon for zero tracks - assert icon == mock_file_item.match_pending_icons[5] - else: - # Should use match icon based on percentage - assert icon == mock_file_item.match_icons[expected_icon_index] + result = provider.evaluate(mock_album) + assert result == expected_percentage - def test_get_match_icon_no_album_attributes(self, match_quality_column: MatchQualityColumn) -> None: - """Test get_match_icon returns None for objects without album attributes.""" + def test_evaluate_no_album_attributes(self, provider: MatchQualityProvider) -> None: + """Test evaluate returns 0.0 for objects without album attributes.""" obj = Mock() # Remove album attributes del obj.get_num_matched_tracks del obj.tracks - icon = match_quality_column.get_match_icon(obj) - assert icon is None - - def test_get_match_icon_no_fileitem_icons(self, match_quality_column: MatchQualityColumn, mock_album: Mock) -> None: - """Test get_match_icon handles missing FileItem icons gracefully.""" - mock_album.get_num_matched_tracks.return_value = 5 - mock_album.tracks = [Mock() for _ in range(10)] - - with patch("picard.ui.itemviews.FileItem") as mock_file_item: - # Remove match_icons attribute - del mock_file_item.match_icons - - icon = match_quality_column.get_match_icon(mock_album) - assert icon is None + result = provider.evaluate(obj) + assert result == "0.0" @pytest.mark.parametrize( ("matched", "total", "unmatched", "duplicates", "extra", "missing"), @@ -156,7 +108,7 @@ def test_get_match_icon_no_fileitem_icons(self, match_quality_column: MatchQuali ) def test_get_match_stats( self, - match_quality_column: MatchQualityColumn, + provider: MatchQualityProvider, mock_album: Mock, matched: int, total: int, @@ -165,7 +117,7 @@ def test_get_match_stats( extra: int, missing: int, ) -> None: - """Test get_match_stats returns correct statistics.""" + """Test _get_match_stats returns correct statistics.""" # Setup mock album mock_album.get_num_matched_tracks.return_value = matched mock_album.get_num_unmatched_files.return_value = unmatched @@ -181,7 +133,7 @@ def test_get_match_stats( else: track.files = [Mock()] # Normal tracks - stats = match_quality_column.get_match_stats(mock_album) + stats = provider.get_match_stats(mock_album) assert stats is not None assert stats['matched'] == matched @@ -191,17 +143,175 @@ def test_get_match_stats( assert stats['extra'] == extra assert stats['missing'] == missing - def test_get_match_stats_no_album_attributes(self, match_quality_column: MatchQualityColumn) -> None: - """Test get_match_stats returns None for objects without album attributes.""" + def test_get_match_stats_no_album_attributes(self, provider: MatchQualityProvider) -> None: + """Test _get_match_stats returns None for objects without album attributes.""" obj = Mock() # Remove album attributes del obj.get_num_matched_tracks del obj.tracks - stats = match_quality_column.get_match_stats(obj) + stats = provider.get_match_stats(obj) assert stats is None +class TestDelegateColumn: + """Test the DelegateColumn class.""" + + @pytest.fixture + def provider(self) -> MatchQualityProvider: + """Create a MatchQualityProvider instance for testing.""" + return MatchQualityProvider() + + @pytest.fixture + def delegate_column(self, provider: MatchQualityProvider) -> DelegateColumn: + """Create a DelegateColumn instance for testing.""" + return DelegateColumn("Match Quality", "~match_quality", provider, width=57, size=QtCore.QSize(16, 16)) + + def test_init(self, delegate_column: DelegateColumn) -> None: + """Test DelegateColumn initialization.""" + assert delegate_column.title == "Match Quality" + assert delegate_column.key == "~match_quality" + assert delegate_column.width == 57 + assert delegate_column.size == QtCore.QSize(16, 16) + assert delegate_column.delegate_class == MatchQualityColumnDelegate + + def test_delegate_class_property(self, delegate_column: DelegateColumn) -> None: + """Test delegate_class property returns correct class.""" + assert delegate_column.delegate_class == MatchQualityColumnDelegate + + def test_size_attribute(self, delegate_column: DelegateColumn) -> None: + """Test size attribute is accessible for delegate compatibility.""" + assert hasattr(delegate_column, 'size') + assert delegate_column.size == QtCore.QSize(16, 16) + + def test_default_size(self, provider: MatchQualityProvider) -> None: + """Test DelegateColumn uses default size when none provided.""" + column = DelegateColumn("Test", "test", provider) + assert column.size == QtCore.QSize(16, 16) + + +class TestMatchQualityColumnFactory: + """Test the match quality column factory.""" + + def test_create_match_quality_column(self) -> None: + """Test create_match_quality_column returns properly configured column.""" + column = create_match_quality_column() + + # Check it's a DelegateColumn + assert isinstance(column, DelegateColumn) + + # Check basic properties + assert column.title == "Match" + assert column.key == "~match_quality" + assert column.width == 57 + assert column.size == QtCore.QSize(16, 16) + + # Check delegate class + assert column.delegate_class == MatchQualityColumnDelegate + + # Check it's marked as default + assert column.is_default is True + + def test_factory_uses_numeric_sorting(self) -> None: + """Test that the factory creates a column with numeric sorting.""" + column = create_match_quality_column() + + assert hasattr(column, 'sortkey') + + # Test that it can handle numeric evaluation + from unittest.mock import Mock + + mock_album = Mock() + mock_album.get_num_matched_tracks.return_value = 5 + mock_album.tracks = [Mock() for _ in range(10)] + + # The column's sortkey should return a tuple for sorting + sort_key = column.sortkey(mock_album) + assert isinstance(sort_key, tuple) + assert len(sort_key) == 2 + assert sort_key[0] == 0 # Numeric values get priority + assert sort_key[1] == 0.5 # The actual percentage + + +class TestMatchQualityColumn: + """Test the match quality column using the factory for regression testing.""" + + @pytest.fixture + def match_quality_column(self) -> DelegateColumn: + """Create a match quality column instance for testing using the factory.""" + return create_match_quality_column() + + @pytest.fixture + def mock_album(self) -> Mock: + """Create a mock album with tracks and files.""" + album = Mock(spec=Album) + album.tracks = [] + album.get_num_matched_tracks.return_value = 0 + album.get_num_unmatched_files.return_value = 0 + album.unmatched_files = Mock() + album.unmatched_files.files = [] + return album + + @pytest.fixture + def mock_track(self) -> Mock: + """Create a mock track.""" + track = Mock(spec=Track) + track.files = [] + return track + + def test_init(self, match_quality_column: DelegateColumn) -> None: + """Test match quality column initialization.""" + assert match_quality_column.title == "Match" + assert match_quality_column.key == "~match_quality" + assert match_quality_column.width == 57 + assert match_quality_column.size == QtCore.QSize(16, 16) + assert isinstance(match_quality_column, DelegateColumn) + + def test_delegate_class(self, match_quality_column: DelegateColumn) -> None: + """Test that the column has the correct delegate class.""" + assert match_quality_column.delegate_class == MatchQualityColumnDelegate + + def test_provider_evaluation(self, match_quality_column: DelegateColumn, mock_album: Mock) -> None: + """Test that the provider correctly evaluates match percentage.""" + # Setup mock album + mock_album.get_num_matched_tracks.return_value = 5 + mock_album.tracks = [Mock() for _ in range(10)] + + # Test that the provider evaluates correctly + result = match_quality_column.delegate_provider.evaluate(mock_album) + assert result == "0.5" # 5/10 = 0.5 + + def test_provider_stats(self, match_quality_column: DelegateColumn, mock_album: Mock) -> None: + """Test that the provider correctly calculates match statistics.""" + # Setup mock album + mock_album.get_num_matched_tracks.return_value = 3 + mock_album.get_num_unmatched_files.return_value = 1 + mock_album.tracks = [Mock() for _ in range(5)] + mock_album.unmatched_files.files = [Mock()] + + # Setup tracks with appropriate file counts + for i, track in enumerate(mock_album.tracks): + if i < 1: + track.files = [] # Missing track + elif i < 2: + track.files = [Mock(), Mock()] # Duplicate files + else: + track.files = [Mock()] # Normal tracks + + # Test the provider's get_match_stats method + provider = match_quality_column.delegate_provider + assert provider is not None + stats = provider.get_match_stats(mock_album) + + assert stats is not None + assert stats['matched'] == 3 + assert stats['total'] == 5 + assert stats['unmatched'] == 1 + assert stats['duplicates'] == 1 + assert stats['extra'] == 1 + assert stats['missing'] == 1 + + class TestMatchQualityColumnDelegate: """Test the MatchQualityColumnDelegate class.""" @@ -324,10 +434,10 @@ def test_paint_wrong_column_type( mock_option: Mock, mock_index: Mock, ) -> None: - """Test paint method when column is not MatchQualityColumn.""" + """Test paint method when column is not DelegateColumn.""" painter = Mock() mock_tree_widget.itemFromIndex.return_value = mock_item - mock_item.columns[2] = Mock() # Not MatchQualityColumn + mock_item.columns[2] = Mock() # Not DelegateColumn # Mock initStyleOption to avoid Qt type issues with patch.object(delegate, "parent", return_value=mock_tree_widget): @@ -349,11 +459,24 @@ def test_paint_without_stats( painter = Mock() mock_tree_widget.itemFromIndex.return_value = mock_item - # Mock the column to return None for stats - mock_column = Mock(spec=MatchQualityColumn) - mock_column.get_match_stats.return_value = None + # Mock the column with delegate_provider + mock_provider = Mock(spec=['get_match_stats']) # Only allow get_match_stats attribute + mock_provider.get_match_stats.return_value = None # No stats available + + mock_column = Mock(spec=DelegateColumn) + mock_column.delegate_provider = mock_provider + mock_column.size = QtCore.QSize(16, 16) # Add size attribute mock_item.columns[2] = mock_column + # Mock the index to return column 2 + mock_index.column.return_value = 2 + + # Mock the object to not have album attributes so get_match_stats returns None + mock_item.obj = Mock() + # Remove album attributes so get_match_stats returns None + del mock_item.obj.get_num_matched_tracks + del mock_item.obj.tracks + # Mock initStyleOption to avoid Qt type issues with patch.object(delegate, "parent", return_value=mock_tree_widget): with patch.object(delegate, "initStyleOption"): @@ -414,14 +537,14 @@ def test_helpEvent_without_columns( def test_helpEvent_wrong_column_type( self, delegate: MatchQualityColumnDelegate, mock_tree_widget: Mock, mock_item: Mock ) -> None: - """Test helpEvent method when column is not MatchQualityColumn.""" + """Test helpEvent method when column is not DelegateColumn.""" event = Mock() view = mock_tree_widget option = Mock() index = Mock() index.column.return_value = 2 mock_tree_widget.itemFromIndex.return_value = mock_item - mock_item.columns[2] = Mock() # Not MatchQualityColumn + mock_item.columns[2] = Mock() # Not DelegateColumn result = delegate.helpEvent(event, view, option, index) assert result is False @@ -438,8 +561,7 @@ def test_helpEvent_without_stats( mock_tree_widget.itemFromIndex.return_value = mock_item # Mock the column to return None for stats - mock_column = Mock(spec=MatchQualityColumn) - mock_column.get_match_stats.return_value = None + mock_column = Mock(spec=DelegateColumn) mock_item.columns[2] = mock_column result = delegate.helpEvent(event, view, option, index) @@ -457,12 +579,43 @@ def test_helpEvent_with_stats( index.column.return_value = 2 mock_tree_widget.itemFromIndex.return_value = mock_item + # Mock the album object with proper attributes + mock_album = Mock() + mock_album.get_num_matched_tracks.return_value = 5 + mock_album.get_num_unmatched_files.return_value = 2 + mock_album.tracks = [Mock() for _ in range(10)] # 10 tracks + mock_album.unmatched_files = Mock() + mock_album.unmatched_files.files = [Mock()] # 1 unmatched file + + # Setup tracks with appropriate file counts for stats calculation + for i, track in enumerate(mock_album.tracks): + if i < 2: # 2 missing tracks + track.files = [] + elif i < 3: # 1 track with duplicates + track.files = [Mock(), Mock()] + else: # Normal tracks + track.files = [Mock()] + + mock_item.obj = mock_album + # Set up delegate's parent relationship with patch.object(delegate, "parent", return_value=mock_tree_widget): - # Mock the column - mock_column = Mock(spec=MatchQualityColumn) - stats = {'matched': 5, 'total': 10, 'missing': 2, 'duplicates': 1, 'extra': 1, 'unmatched': 2} - mock_column.get_match_stats.return_value = stats + # Mock the column with delegate_provider + mock_provider = Mock() + mock_provider.get_match_stats.return_value = { + 'matched': 5, + 'total': 10, + 'missing': 2, + 'duplicates': 1, + 'extra': 1, + 'unmatched': 2, + } + # Ensure the provider is not wrapped by an adapter for this test + if hasattr(mock_provider, '_base'): + del mock_provider._base + + mock_column = Mock(spec=DelegateColumn) + mock_column.delegate_provider = mock_provider mock_item.columns[2] = mock_column with patch("picard.ui.itemviews.match_quality_column.QtWidgets.QToolTip") as mock_tooltip: @@ -495,7 +648,7 @@ def test_sizeHint(self, delegate: MatchQualityColumnDelegate) -> None: class TestItemViewsIntegration: - """Test integration of MatchQualityColumn with existing itemviews system.""" + """Test integration of new delegate column architecture with existing itemviews system.""" @pytest.fixture def mock_album(self) -> Mock: @@ -508,14 +661,14 @@ def mock_album(self) -> Mock: album.unmatched_files.files = [Mock()] return album - def test_match_quality_column_in_itemview_columns(self) -> None: - """Test that MatchQualityColumn is included in ITEMVIEW_COLUMNS (album view).""" + def test_delegate_column_in_album_view_columns(self) -> None: + """Test that DelegateColumn is included in ALBUMVIEW_COLUMNS (album view).""" from picard.ui.itemviews.columns import ALBUMVIEW_COLUMNS # Find the match quality column in album view columns match_quality_column = None for column in ALBUMVIEW_COLUMNS: - if isinstance(column, MatchQualityColumn): + if isinstance(column, DelegateColumn) and column.key == "~match_quality": match_quality_column = column break @@ -524,16 +677,16 @@ def test_match_quality_column_in_itemview_columns(self) -> None: assert match_quality_column.key == "~match_quality" assert match_quality_column.sortable is True assert match_quality_column.sort_type is not None - assert match_quality_column.sortkey is not None + assert match_quality_column.delegate_class == MatchQualityColumnDelegate - def test_match_quality_column_not_in_fileview_columns(self) -> None: - """Test that MatchQualityColumn is NOT included in FILEVIEW_COLUMNS.""" + def test_delegate_column_not_in_fileview_columns(self) -> None: + """Test that DelegateColumn is NOT included in FILEVIEW_COLUMNS.""" from picard.ui.itemviews.columns import FILEVIEW_COLUMNS - # Verify that no match quality column exists in file view columns + # Verify that no match quality delegate column exists in file view columns match_quality_column = None for column in FILEVIEW_COLUMNS: - if isinstance(column, MatchQualityColumn): + if isinstance(column, DelegateColumn) and column.key == "~match_quality": match_quality_column = column break @@ -549,41 +702,50 @@ def test_album_view_has_match_quality_column_after_album_artist(self) -> None: assert idx_match == idx_albumartist + 1 match_quality_column = ALBUMVIEW_COLUMNS[idx_match] - assert isinstance(match_quality_column, MatchQualityColumn) + assert isinstance(match_quality_column, DelegateColumn) assert match_quality_column.key == "~match_quality" album_artist_column = ALBUMVIEW_COLUMNS[idx_albumartist] assert album_artist_column.key == "albumartist" - def test_sortkey_progress_function(self) -> None: - """Test the _sortkey_progress function used by MatchQualityColumn.""" - from picard.ui.itemviews.columns import _sortkey_match_quality + def test_numeric_sorting_with_provider(self) -> None: + """Test that the provider uses numeric sorting correctly.""" + from picard.ui.itemviews.columns import ALBUMVIEW_COLUMNS + + # Find the match quality column + match_quality_column = None + for column in ALBUMVIEW_COLUMNS: + if isinstance(column, DelegateColumn) and column.key == "~match_quality": + match_quality_column = column + break + + assert match_quality_column is not None # Test with album object album = Mock() album.get_num_matched_tracks.return_value = 3 album.tracks = [Mock() for _ in range(5)] - result = _sortkey_match_quality(album) - expected = _apply_platform_multiplier(0.6) - assert result == expected + # Test that the provider correctly evaluates match percentage + result = match_quality_column.delegate_provider.evaluate(album) + assert result == "0.6" # 3/5 = 0.6 # Test with track object (should return 0.0) track = Mock() # Mock hasattr to return False for track objects with patch("builtins.hasattr", return_value=False): - result = _sortkey_match_quality(track) - assert result == 0.0 + result = match_quality_column.delegate_provider.evaluate(track) + assert result == "0.0" # Test with album with no tracks album.tracks = [] - result = _sortkey_match_quality(album) - assert result == 0.0 + result = match_quality_column.delegate_provider.evaluate(album) + assert result == "0.0" - def test_treeitem_handles_match_quality_column(self, mock_album: Mock) -> None: - """Test that TreeItem properly handles MatchQualityColumn.""" - # Create a mock column - column = Mock(spec=MatchQualityColumn) + def test_treeitem_handles_delegate_column(self, mock_album: Mock) -> None: + """Test that TreeItem properly handles DelegateColumn.""" + # Create a mock delegate column + column = Mock(spec=DelegateColumn) column.size = QtCore.QSize(16, 16) # Create a mock item @@ -593,7 +755,7 @@ def test_treeitem_handles_match_quality_column(self, mock_album: Mock) -> None: # Mock the update_colums_text method with patch.object(TreeItem, "update_colums_text"): # Simulate the condition in update_colums_text - if isinstance(column, MatchQualityColumn): + if isinstance(column, DelegateColumn): item.setSizeHint(1, column.size) # Verify setSizeHint was called @@ -670,106 +832,3 @@ def test_cluster_item_formatting(self) -> None: with patch.object(ClusterItem, "columns", mock_columns): item = ClusterItem(cluster, parent=None) assert item is not None - - -class TestMatchQualitySorting: - """Test match quality sorting behavior.""" - - @pytest.fixture - def mock_album_loading(self): - """Create a mock album that is still loading.""" - album = Mock() - album.status = AlbumStatus.LOADING - album.get_num_matched_tracks.return_value = 0 - album.tracks = [] - return album - - @pytest.fixture - def mock_album_loaded(self): - """Create a mock album that has finished loading.""" - album = Mock() - album.status = AlbumStatus.LOADED - album.get_num_matched_tracks.return_value = 3 - album.tracks = [Mock(), Mock(), Mock(), Mock(), Mock()] # 5 tracks total - return album - - @pytest.fixture - def mock_track(self): - """Create a mock track object.""" - track = Mock() - # Track objects don't have get_num_matched_tracks or tracks attributes - # We'll use patch to mock hasattr behavior - return track - - def test_sortkey_match_quality_loading_album(self, mock_album_loading): - """Test that loading albums return 0.0 to avoid premature sorting.""" - result = _sortkey_match_quality(mock_album_loading) - assert result == 0.0 - - def test_sortkey_match_quality_loaded_album(self, mock_album_loaded): - """Test that loaded albums return correct match percentage.""" - result = _sortkey_match_quality(mock_album_loaded) - # 3 matched out of 5 total = 0.6 - expected = _apply_platform_multiplier(0.6) - assert result == expected - - def test_sortkey_match_quality_track_object(self, mock_track): - """Test that track objects return 0.0.""" - # Mock hasattr to return False for track objects - with patch("builtins.hasattr", side_effect=lambda obj, attr: attr not in ("get_num_matched_tracks", "tracks")): - result = _sortkey_match_quality(mock_track) - assert result == 0.0 - - def test_sortkey_match_quality_no_tracks(self, mock_album_loaded): - """Test that albums with no tracks return 0.0.""" - mock_album_loaded.tracks = [] - result = _sortkey_match_quality(mock_album_loaded) - assert result == 0.0 - - def test_sortkey_match_quality_all_matched(self, mock_album_loaded): - """Test that albums with all tracks matched return correct value.""" - mock_album_loaded.get_num_matched_tracks.return_value = 5 - result = _sortkey_match_quality(mock_album_loaded) - expected = _apply_platform_multiplier(1.0) - assert result == expected - - def test_sortkey_match_quality_no_matches(self, mock_album_loaded): - """Test that albums with no matches return 0.0.""" - mock_album_loaded.get_num_matched_tracks.return_value = 0 - result = _sortkey_match_quality(mock_album_loaded) - assert result == 0.0 - - def test_sortkey_match_quality_partial_matches(self, mock_album_loaded): - """Test that albums with partial matches return correct percentage.""" - mock_album_loaded.get_num_matched_tracks.return_value = 2 - result = _sortkey_match_quality(mock_album_loaded) - # 2 matched out of 5 total = 0.4 - expected = _apply_platform_multiplier(0.4) - assert result == expected - - def test_sortkey_match_quality_no_status_attribute(self): - """Test that objects without status attribute are handled gracefully.""" - obj = Mock() - obj.get_num_matched_tracks.return_value = 2 - obj.tracks = [Mock(), Mock(), Mock()] # 3 tracks - # No status attribute - result = _sortkey_match_quality(obj) - # Should calculate normally: 2/3 = 0.666... - expected = _apply_platform_multiplier(0.6666666666666666) - assert result == pytest.approx(expected, rel=1e-10) - - def test_sortkey_match_quality_error_status(self, mock_album_loaded): - """Test that albums with error status are handled correctly.""" - mock_album_loaded.status = AlbumStatus.ERROR - result = _sortkey_match_quality(mock_album_loaded) - # Should still calculate the match percentage - expected = _apply_platform_multiplier(0.6) - assert result == expected - - def test_sortkey_match_quality_none_status(self, mock_album_loaded): - """Test that albums with None status are handled correctly.""" - mock_album_loaded.status = None - result = _sortkey_match_quality(mock_album_loaded) - # Should still calculate the match percentage - expected = _apply_platform_multiplier(0.6) - assert result == expected diff --git a/test/test_ui_columns.py b/test/test_ui_columns.py index 051901b584..74ca9dfbf5 100644 --- a/test/test_ui_columns.py +++ b/test/test_ui_columns.py @@ -29,7 +29,7 @@ DefaultColumn, ImageColumn, ) -from picard.ui.itemviews.columns import IconColumn +from picard.ui.itemviews.custom_columns import IconColumn class ColumnTest(PicardTestCase): @@ -68,9 +68,18 @@ def test_default_column(self): self.assertTrue(column.is_default) def test_icon_column(self): - column = IconColumn('title', 'key') + class _Provider: + def __init__(self): + self._icon = None + + def get_icon(self): + if self._icon is None: + self._icon = 'icon' + return self._icon + + provider = _Provider() + column = IconColumn('title', 'key', provider) self.assertIsInstance(column, ImageColumn) - column.header_icon_func = lambda: 'icon' self.assertEqual(column.header_icon, 'icon') column.set_header_icon_size(10, 20, 2) self.assertEqual(column.header_icon_size.width(), 10)