From 6dd34b292f3dcd202a47b7c1b6f3c4a7bd9a152c Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Fri, 7 Mar 2025 15:44:21 +0000 Subject: [PATCH 1/8] Improve ApplicationError reporting --- app/config.py | 2 +- app/log.py | 50 +++++++++++++++++++++++++------------------- app/musicmuster.py | 5 ++--- app/playlistmodel.py | 1 - 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/app/config.py b/app/config.py index 5d5ef6c..431dce0 100644 --- a/app/config.py +++ b/app/config.py @@ -123,11 +123,11 @@ class Config(object): ROWS_FROM_ZERO = True SCROLL_TOP_MARGIN = 3 SECTION_ENDINGS = ("-", "+-", "-+") + SECTION_HEADER = "[Section header]" SECTION_STARTS = ("+", "+-", "-+") SONGFACTS_ON_NEXT = False START_GAP_WARNING_THRESHOLD = 300 SUBTOTAL_ON_ROW_ZERO = "[No subtotal on first row]" - TEXT_NO_TRACK_NO_NOTE = "[Section header]" TOD_TIME_FORMAT = "%H:%M:%S" TRACK_TIME_FORMAT = "%H:%M:%S" VLC_MAIN_PLAYER_NAME = "MusicMuster Main Player" diff --git a/app/log.py b/app/log.py index cbbef18..2adec79 100644 --- a/app/log.py +++ b/app/log.py @@ -6,16 +6,18 @@ import logging.config import logging.handlers import os import sys -from traceback import print_exception +import traceback import yaml # PyQt imports +from PyQt6.QtWidgets import QMessageBox # Third party imports import stackprinter # type: ignore # App imports from config import Config +from classes import ApplicationError class FunctionFilter(logging.Filter): @@ -76,26 +78,32 @@ with open("app/logging.yaml", "r") as f: log = logging.getLogger(Config.LOG_NAME) -def log_uncaught_exceptions(type_, value, traceback): - from helpers import send_mail +def handle_exception(exc_type, exc_value, exc_traceback): + if issubclass(exc_type, ApplicationError): + error = str(exc_value) + log.error(error) + QMessageBox.critical(None, "Application Error", error) + else: + # Handle unexpected errors (log and display) + error_msg = "".join(traceback.format_exception(exc_type, exc_value, exc_traceback)) + QMessageBox.critical(None, "Application Error", error_msg) - print("\033[1;31;47m") - print_exception(type_, value, traceback) - print("\033[1;37;40m") - print( - stackprinter.format( - value, suppressed_paths=["/pypoetry/virtualenvs/"], style="darkbg" - ) - ) - if os.environ["MM_ENV"] == "PRODUCTION": - msg = stackprinter.format(value) - send_mail( - Config.ERRORS_TO, - Config.ERRORS_FROM, - "Exception (log_uncaught_exceptions) from musicmuster", - msg, - ) - log.debug(msg) + print(stackprinter.format(exc_value, suppressed_paths=['/.venv'], style='darkbg')) + + msg = stackprinter.format(exc_value) + log.error(msg) + log.error(error_msg) + print("Critical error:", error_msg) # Consider logging instead of print + + if os.environ["MM_ENV"] == "PRODUCTION": + from helpers import send_mail + + send_mail( + Config.ERRORS_TO, + Config.ERRORS_FROM, + "Exception (log_uncaught_exceptions) from musicmuster", + msg, + ) -sys.excepthook = log_uncaught_exceptions +sys.excepthook = handle_exception diff --git a/app/musicmuster.py b/app/musicmuster.py index 61fe8bd..9df8eb2 100755 --- a/app/musicmuster.py +++ b/app/musicmuster.py @@ -2811,8 +2811,8 @@ if __name__ == "__main__": with db.Session() as session: update_bitrates(session) else: + app = QApplication(sys.argv) try: - app = QApplication(sys.argv) # PyQt6 defaults to a grey for labels palette = app.palette() palette.setColor( @@ -2830,6 +2830,7 @@ if __name__ == "__main__": win.show() status = app.exec() sys.exit(status) + except Exception as exc: if os.environ["MM_ENV"] == "PRODUCTION": from helpers import send_mail @@ -2843,10 +2844,8 @@ if __name__ == "__main__": ) log.debug(msg) else: - print("\033[1;31;47mUnhandled exception starts") print( stackprinter.format( exc, suppressed_paths=["/pypoetry/virtualenvs/"], style="darkbg" ) ) - print("Unhandled exception ends\033[1;37;40m") diff --git a/app/playlistmodel.py b/app/playlistmodel.py index 5571a1a..d7f3a46 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -1,5 +1,4 @@ # Standard library imports -# Allow forward reference to PlaylistModel from __future__ import annotations from operator import attrgetter From 1f10692c15e80767a629dcb4f160bab7ade4b3f9 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 09:57:04 +0000 Subject: [PATCH 2/8] Make notes substring unique --- app/dbtables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/dbtables.py b/app/dbtables.py index 6d80d57..6c6cbe8 100644 --- a/app/dbtables.py +++ b/app/dbtables.py @@ -53,7 +53,7 @@ class NoteColoursTable(Model): __tablename__ = "notecolours" id: Mapped[int] = mapped_column(primary_key=True, autoincrement=True) - substring: Mapped[str] = mapped_column(String(256), index=True) + substring: Mapped[str] = mapped_column(String(256), index=True, unique=True) colour: Mapped[str] = mapped_column(String(21), index=False) enabled: Mapped[bool] = mapped_column(default=True, index=True) foreground: Mapped[Optional[str]] = mapped_column(String(21), index=False) From 76039aa5e696bb534249466d863e64ad95e945e4 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 11:42:59 +0000 Subject: [PATCH 3/8] Only try to show ApplicationError dialog when we have a QApplication --- app/log.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/log.py b/app/log.py index 2adec79..c729025 100644 --- a/app/log.py +++ b/app/log.py @@ -10,7 +10,7 @@ import traceback import yaml # PyQt imports -from PyQt6.QtWidgets import QMessageBox +from PyQt6.QtWidgets import QApplication, QMessageBox # Third party imports import stackprinter # type: ignore @@ -79,14 +79,14 @@ log = logging.getLogger(Config.LOG_NAME) def handle_exception(exc_type, exc_value, exc_traceback): - if issubclass(exc_type, ApplicationError): - error = str(exc_value) - log.error(error) + error = str(exc_value) + if QApplication.instance() is not None: QMessageBox.critical(None, "Application Error", error) + if issubclass(exc_type, ApplicationError): + log.error(error) else: # Handle unexpected errors (log and display) error_msg = "".join(traceback.format_exception(exc_type, exc_value, exc_traceback)) - QMessageBox.critical(None, "Application Error", error_msg) print(stackprinter.format(exc_value, suppressed_paths=['/.venv'], style='darkbg')) From 3b004567dff177df61b412766b1ba0a42c9d623c Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 11:45:38 +0000 Subject: [PATCH 4/8] Implement dogpile cache for Notecolours --- app/models.py | 38 ++++++++++++++++++++++++++++++-------- pyproject.toml | 1 + uv.lock | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/app/models.py b/app/models.py index e3a756b..66f8c73 100644 --- a/app/models.py +++ b/app/models.py @@ -10,6 +10,8 @@ import sys # PyQt imports # Third party imports +from dogpile.cache import make_region +from dogpile.cache.api import NO_VALUE from sqlalchemy import ( bindparam, delete, @@ -40,6 +42,11 @@ if "unittest" in sys.modules and "sqlite" not in DATABASE_URL: raise ValueError("Unit tests running on non-Sqlite database") db = DatabaseManager.get_instance(DATABASE_URL, engine_options=Config.ENGINE_OPTIONS).db +# Configure the cache region +cache_region = make_region().configure( + 'dogpile.cache.memory', # Use in-memory caching for now (switch to Redis if needed) + expiration_time=600 # Cache expires after 10 minutes +) def run_sql(session: Session, sql: str) -> Sequence[RowMapping]: """ @@ -54,6 +61,7 @@ def run_sql(session: Session, sql: str) -> Sequence[RowMapping]: # Database classes class NoteColours(dbtables.NoteColoursTable): + def __init__( self, session: Session, @@ -80,7 +88,22 @@ class NoteColours(dbtables.NoteColoursTable): Return all records """ - result = session.scalars(select(cls)).all() + cache_key = "note_colours_all" + cached_result = cache_region.get(cache_key) + + if cached_result is NO_VALUE: + # Query the database + result = session.scalars( + select(cls) + .where( + cls.enabled.is_(True), + ) + .order_by(cls.order) + ).all() + cache_region.set(cache_key, result) + else: + result = cached_result + return result @staticmethod @@ -97,13 +120,7 @@ class NoteColours(dbtables.NoteColoursTable): return None match = False - for rec in session.scalars( - select(NoteColours) - .where( - NoteColours.enabled.is_(True), - ) - .order_by(NoteColours.order) - ).all(): + for rec in NoteColours.get_all(session): if rec.is_regex: flags = re.UNICODE if not rec.is_casesensitive: @@ -126,6 +143,11 @@ class NoteColours(dbtables.NoteColoursTable): return rec.colour return None + def invalidate_cache(self) -> None: + """Invalidate dogpile cache""" + + cache_region.delete("note_colours_all") + class Playdates(dbtables.PlaydatesTable): def __init__( diff --git a/pyproject.toml b/pyproject.toml index ee189ae..09d7c0a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,7 @@ dependencies = [ "pyyaml (>=6.0.2,<7.0.0)", "audioop-lts>=0.2.1", "types-pyyaml>=6.0.12.20241230", + "dogpile-cache>=1.3.4", ] [dependency-groups] diff --git a/uv.lock b/uv.lock index e14e719..b8f3114 100644 --- a/uv.lock +++ b/uv.lock @@ -168,6 +168,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/4e/8c/f3147f5c4b73e7550fe5f9352eaa956ae838d5c51eb58e7a25b9f3e2643b/decorator-5.2.1-py3-none-any.whl", hash = "sha256:d316bb415a2d9e2d2b3abcc4084c6502fc09240e292cd76a76afc106a1c8e04a", size = 9190 }, ] +[[package]] +name = "dogpile-cache" +version = "1.3.4" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "decorator" }, + { name = "stevedore" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/cb/b7/2fa37f52b4f38bc8eb6d4923163dd822ca6f9e2f817378478a5de73b239e/dogpile_cache-1.3.4.tar.gz", hash = "sha256:4f0295575f5fdd3f7e13c84ba8e36656971d1869a2081b4737ec99ede378a8c0", size = 933234 } +wheels = [ + { url = "https://files.pythonhosted.org/packages/3c/82/d118accb66f9048acbc4ff91592755c24d52f54cce40d6b0b2a0ce351cf5/dogpile.cache-1.3.4-py3-none-any.whl", hash = "sha256:a393412f93d24a8942fdf9248dc80678127d54c5e60a7be404027aa193cafe12", size = 62859 }, +] + [[package]] name = "entrypoints" version = "0.4" @@ -449,6 +462,7 @@ dependencies = [ { name = "alembic" }, { name = "audioop-lts" }, { name = "colorlog" }, + { name = "dogpile-cache" }, { name = "fuzzywuzzy" }, { name = "mutagen" }, { name = "mysqlclient" }, @@ -492,6 +506,7 @@ requires-dist = [ { name = "alembic", specifier = ">=1.14.0" }, { name = "audioop-lts", specifier = ">=0.2.1" }, { name = "colorlog", specifier = ">=6.9.0" }, + { name = "dogpile-cache", specifier = ">=1.3.4" }, { name = "fuzzywuzzy", specifier = ">=0.18.0" }, { name = "mutagen", specifier = ">=1.47.0" }, { name = "mysqlclient", specifier = ">=2.2.5" }, @@ -641,6 +656,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/cc/20/ff623b09d963f88bfde16306a54e12ee5ea43e9b597108672ff3a408aad6/pathspec-0.12.1-py3-none-any.whl", hash = "sha256:a0d503e138a4c123b27490a4f7beda6a01c6f288df0e4a8b79c7eb0dc7b4cc08", size = 31191 }, ] +[[package]] +name = "pbr" +version = "6.1.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "setuptools" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/01/d2/510cc0d218e753ba62a1bc1434651db3cd797a9716a0a66cc714cb4f0935/pbr-6.1.1.tar.gz", hash = "sha256:93ea72ce6989eb2eed99d0f75721474f69ad88128afdef5ac377eb797c4bf76b", size = 125702 } +wheels = [ + { url = "https://files.pythonhosted.org/packages/47/ac/684d71315abc7b1214d59304e23a982472967f6bf4bde5a98f1503f648dc/pbr-6.1.1-py2.py3-none-any.whl", hash = "sha256:38d4daea5d9fa63b3f626131b9d34947fd0c8be9b05a29276870580050a25a76", size = 108997 }, +] + [[package]] name = "pexpect" version = "4.9.0" @@ -1028,6 +1055,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/19/71/39c7c0d87f8d4e6c020a393182060eaefeeae6c01dab6a84ec346f2567df/rich-13.9.4-py3-none-any.whl", hash = "sha256:6049d5e6ec054bf2779ab3358186963bac2ea89175919d699e378b99738c2a90", size = 242424 }, ] +[[package]] +name = "setuptools" +version = "75.8.2" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d1/53/43d99d7687e8cdef5ab5f9ec5eaf2c0423c2b35133a2b7e7bc276fc32b21/setuptools-75.8.2.tar.gz", hash = "sha256:4880473a969e5f23f2a2be3646b2dfd84af9028716d398e46192f84bc36900d2", size = 1344083 } +wheels = [ + { url = "https://files.pythonhosted.org/packages/a9/38/7d7362e031bd6dc121e5081d8cb6aa6f6fedf2b67bf889962134c6da4705/setuptools-75.8.2-py3-none-any.whl", hash = "sha256:558e47c15f1811c1fa7adbd0096669bf76c1d3f433f58324df69f3f5ecac4e8f", size = 1229385 }, +] + [[package]] name = "sqlalchemy" version = "2.0.38" @@ -1072,6 +1108,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/7c/15/485186e37a06d28b7fc9020ad57ba1e3778ee9e8930ff6c9ea350946ffe1/stackprinter-0.2.12-py3-none-any.whl", hash = "sha256:0a0623d46a5babd7a8a9787f605f4dd4a42d6ff7aee140541d5e9291a506e8d9", size = 29282 }, ] +[[package]] +name = "stevedore" +version = "5.4.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pbr" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/28/3f/13cacea96900bbd31bb05c6b74135f85d15564fc583802be56976c940470/stevedore-5.4.1.tar.gz", hash = "sha256:3135b5ae50fe12816ef291baff420acb727fcd356106e3e9cbfa9e5985cd6f4b", size = 513858 } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f7/45/8c4ebc0c460e6ec38e62ab245ad3c7fc10b210116cea7c16d61602aa9558/stevedore-5.4.1-py3-none-any.whl", hash = "sha256:d10a31c7b86cba16c1f6e8d15416955fc797052351a56af15e608ad20811fcfe", size = 49533 }, +] + [[package]] name = "text-unidecode" version = "1.3" From 2f8afeb8146086dc33fdb7fbdd8a060d33567794 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 12:02:07 +0000 Subject: [PATCH 5/8] WIP Issue 285 --- app/playlistmodel.py | 228 ++++++++++++++++++++++--------------------- 1 file changed, 118 insertions(+), 110 deletions(-) diff --git a/app/playlistmodel.py b/app/playlistmodel.py index d7f3a46..1c4c329 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -11,7 +11,6 @@ import re from PyQt6.QtCore import ( QAbstractTableModel, QModelIndex, - QObject, QRegularExpression, QSortFilterProxyModel, Qt, @@ -25,7 +24,6 @@ from PyQt6.QtGui import ( ) # Third party imports -import line_profiler from sqlalchemy.orm.session import Session import obswebsocket # type: ignore @@ -72,18 +70,14 @@ class PlaylistModel(QAbstractTableModel): database. """ - def __init__( - self, - playlist_id: int, - is_template: bool, - *args: Optional[QObject], - **kwargs: Optional[QObject], - ) -> None: + def __init__(self, playlist_id: int, is_template: bool,) -> None: + + super().__init__() + log.debug("PlaylistModel.__init__()") self.playlist_id = playlist_id self.is_template = is_template - super().__init__(*args, **kwargs) self.playlist_rows: dict[int, RowAndTrack] = {} self.signals = MusicMusterSignals() @@ -101,13 +95,17 @@ class PlaylistModel(QAbstractTableModel): def __repr__(self) -> str: return ( - f"" + f"" ) def active_section_header(self) -> int: """ - Return the row number of the first header that has either unplayed tracks - or currently being played track below it. + Return the row number of the first header that has any of the following below it: + - unplayed tracks + - the currently being played track + - the track marked as next to play """ header_row = 0 @@ -119,23 +117,20 @@ class PlaylistModel(QAbstractTableModel): if not self.is_played_row(row_number): break - # If track is played, we need to check it's not the current - # next or previous track because we don't want to scroll them - # out of view + # Here means that row_number points to a played track. The + # current track will be marked as played when we start + # playing it. It's also possible that the track marked as + # next has already been played. Check for either of those. - for ts in [ - track_sequence.next, - track_sequence.current, - ]: + for ts in [track_sequence.next, track_sequence.current]: if ( ts and ts.row_number == row_number and ts.playlist_id == self.playlist_id ): - break - else: - continue # continue iterating over playlist_rows - break # current row is in one of the track sequences + # We've found the current or next track, so return + # the last-found header row + return header_row return header_row @@ -152,31 +147,34 @@ class PlaylistModel(QAbstractTableModel): try: rat = self.playlist_rows[row_number] except KeyError: - log.error( + raise ApplicationError( f"{self}: KeyError in add_track_to_header ({row_number=}, {track_id=})" ) - return if rat.path: - log.error( + raise ApplicationError( f"{self}: Header row already has track associated ({rat=}, {track_id=})" ) - return with db.Session() as session: playlistrow = session.get(PlaylistRows, rat.playlistrow_id) - if playlistrow: - # Add track to PlaylistRows - playlistrow.track_id = track_id - # Add any further note (header will already have a note) - if note: - playlistrow.note += "\n" + note - # Update local copy - self.refresh_row(session, row_number) - # Repaint row - self.invalidate_row(row_number) - session.commit() + if not playlistrow: + raise ApplicationError( + f"{self}: Failed to retrieve playlist row ({rat.playlistrow_id=}" + ) + # Add track to PlaylistRows + playlistrow.track_id = track_id + # Add any further note (header will already have a note) + if note: + playlistrow.note += " " + note + session.commit() + + # Update local copy + self.refresh_row(session, row_number) + # Repaint row + self.invalidate_row(row_number) self.signals.resize_rows_signal.emit(self.playlist_id) + # @line_profiler.profile def background_role(self, row: int, column: int, rat: RowAndTrack) -> QBrush: """Return background setting""" @@ -257,26 +255,28 @@ class PlaylistModel(QAbstractTableModel): - update track times """ + log.debug(f"{self}: current_track_started()") + if not track_sequence.current: return row_number = track_sequence.current.row_number # Check for OBS scene change - log.debug(f"{self}: Call OBS scene change") self.obs_scene_change(row_number) # Sanity check that we have a track_id - if not track_sequence.current.track_id: - log.error( - f"{self}: current_track_started() called with {track_sequence.current.track_id=}" + track_id = track_sequence.current.track_id + if not track_id: + raise ApplicationError( + f"{self}: current_track_started() called with {track_id=}" ) - return with db.Session() as session: # Update Playdates in database - log.debug(f"{self}: update playdates") - Playdates(session, track_sequence.current.track_id) + log.debug(f"{self}: update playdates {track_id=}") + Playdates(session, track_id) + session.commit() # Mark track as played in playlist log.debug(f"{self}: Mark track as played") @@ -315,14 +315,25 @@ class PlaylistModel(QAbstractTableModel): if next_row is not None: self.set_next_row(next_row) - session.commit() - + # @line_profiler.profile def data( self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole - ) -> QVariant: + ) -> QVariant | QFont | QBrush | str: """Return data to view""" - if not index.isValid() or not (0 <= index.row() < len(self.playlist_rows)): + if ( + not index.isValid() + or not (0 <= index.row() < len(self.playlist_rows)) + or role in [ + Qt.ItemDataRole.DecorationRole, + Qt.ItemDataRole.StatusTipRole, + Qt.ItemDataRole.WhatsThisRole, + Qt.ItemDataRole.SizeHintRole, + Qt.ItemDataRole.TextAlignmentRole, + Qt.ItemDataRole.CheckStateRole, + Qt.ItemDataRole.InitialSortOrderRole, + ] + ): return QVariant() row = index.row() @@ -330,32 +341,21 @@ class PlaylistModel(QAbstractTableModel): # rat for playlist row data as it's used a lot rat = self.playlist_rows[row] - # Dispatch to role-specific functions - dispatch_table = { - int(Qt.ItemDataRole.BackgroundRole): self.background_role, - int(Qt.ItemDataRole.DisplayRole): self.display_role, - int(Qt.ItemDataRole.EditRole): self.edit_role, - int(Qt.ItemDataRole.FontRole): self.font_role, - int(Qt.ItemDataRole.ForegroundRole): self.foreground_role, - int(Qt.ItemDataRole.ToolTipRole): self.tooltip_role, - } + # These are ordered in approximately the frequency with which + # they are called + if role == Qt.ItemDataRole.BackgroundRole: + return self.background_role(row, column, rat) + elif role == Qt.ItemDataRole.DisplayRole: + return self.display_role(row, column, rat) + elif role == Qt.ItemDataRole.EditRole: + return self.edit_role(row, column, rat) + elif role == Qt.ItemDataRole.FontRole: + return self.font_role(row, column, rat) + elif role == Qt.ItemDataRole.ForegroundRole: + return self.foreground_role(row, column, rat) + elif role == Qt.ItemDataRole.ToolTipRole: + return self.tooltip_role(row, column, rat) - if role in dispatch_table: - return QVariant(dispatch_table[role](row, column, rat)) - - # Document other roles but don't use them - if role in [ - Qt.ItemDataRole.DecorationRole, - Qt.ItemDataRole.StatusTipRole, - Qt.ItemDataRole.WhatsThisRole, - Qt.ItemDataRole.SizeHintRole, - Qt.ItemDataRole.TextAlignmentRole, - Qt.ItemDataRole.CheckStateRole, - Qt.ItemDataRole.InitialSortOrderRole, - ]: - return QVariant() - - # Fall through to no-op return QVariant() def delete_rows(self, row_numbers: list[int]) -> None: @@ -385,8 +385,10 @@ class PlaylistModel(QAbstractTableModel): super().endRemoveRows() self.reset_track_sequence_row_numbers() + self.update_track_times() - def display_role(self, row: int, column: int, rat: RowAndTrack) -> QVariant: + # @line_profiler.profile + def display_role(self, row: int, column: int, rat: RowAndTrack) -> str: """ Return text for display """ @@ -406,45 +408,45 @@ class PlaylistModel(QAbstractTableModel): if column == HEADER_NOTES_COLUMN: header_text = self.header_text(rat) if not header_text: - return QVariant(Config.TEXT_NO_TRACK_NO_NOTE) + return Config.SECTION_HEADER else: formatted_header = self.header_text(rat) trimmed_header = self.remove_section_timer_markers(formatted_header) - return QVariant(trimmed_header) + return trimmed_header else: - return QVariant("") + return "" if column == Col.START_TIME.value: start_time = rat.forecast_start_time if start_time: - return QVariant(start_time.strftime(Config.TRACK_TIME_FORMAT)) - return QVariant() + return start_time.strftime(Config.TRACK_TIME_FORMAT) + return "" if column == Col.END_TIME.value: end_time = rat.forecast_end_time if end_time: - return QVariant(end_time.strftime(Config.TRACK_TIME_FORMAT)) - return QVariant() + return end_time.strftime(Config.TRACK_TIME_FORMAT) + return "" if column == Col.INTRO.value: if rat.intro: - return QVariant(f"{rat.intro / 1000:{Config.INTRO_SECONDS_FORMAT}}") + return f"{rat.intro / 1000:{Config.INTRO_SECONDS_FORMAT}}" else: - return QVariant("") + return "" - dispatch_table = { - Col.ARTIST.value: QVariant(rat.artist), - Col.BITRATE.value: QVariant(rat.bitrate), - Col.DURATION.value: QVariant(ms_to_mmss(rat.duration)), - Col.LAST_PLAYED.value: QVariant(get_relative_date(rat.lastplayed)), - Col.NOTE.value: QVariant(rat.note), - Col.START_GAP.value: QVariant(rat.start_gap), - Col.TITLE.value: QVariant(rat.title), + dispatch_table: dict[int, str] = { + Col.ARTIST.value: rat.artist, + Col.BITRATE.value: str(rat.bitrate), + Col.DURATION.value: ms_to_mmss(rat.duration), + Col.LAST_PLAYED.value: get_relative_date(rat.lastplayed), + Col.NOTE.value: rat.note, + Col.START_GAP.value: str(rat.start_gap), + Col.TITLE.value: rat.title, } if column in dispatch_table: return dispatch_table[column] - return QVariant() + return "" def end_reset_model(self, playlist_id: int) -> None: """ @@ -461,7 +463,8 @@ class PlaylistModel(QAbstractTableModel): super().endResetModel() self.reset_track_sequence_row_numbers() - def edit_role(self, row: int, column: int, rat: RowAndTrack) -> QVariant: + # @line_profiler.profile + def edit_role(self, row: int, column: int, rat: RowAndTrack) -> str: """ Return text for editing """ @@ -469,19 +472,20 @@ class PlaylistModel(QAbstractTableModel): # If this is a header row and we're being asked for the # HEADER_NOTES_COLUMN, return the note value if self.is_header_row(row) and column == HEADER_NOTES_COLUMN: - return QVariant(rat.note) + return rat.note if column == Col.INTRO.value: - return QVariant(rat.intro) + return str(rat.intro or "") if column == Col.TITLE.value: - return QVariant(rat.title) + return rat.title if column == Col.ARTIST.value: - return QVariant(rat.artist) + return rat.artist if column == Col.NOTE.value: - return QVariant(rat.note) + return rat.note - return QVariant() + return "" + # @line_profiler.profile def foreground_role(self, row: int, column: int, rat: RowAndTrack) -> QBrush: """Return header foreground colour or QBrush() if none""" @@ -518,19 +522,20 @@ class PlaylistModel(QAbstractTableModel): return default - def font_role(self, row: int, column: int, rat: RowAndTrack) -> QVariant: + # @line_profiler.profile + def font_role(self, row: int, column: int, rat: RowAndTrack) -> QFont: """ Return font """ # Notes column is never bold if column == Col.NOTE.value: - return QVariant() + return QFont() boldfont = QFont() boldfont.setBold(not self.playlist_rows[row].played) - return QVariant(boldfont) + return boldfont def get_duplicate_rows(self) -> list[int]: """ @@ -729,7 +734,8 @@ class PlaylistModel(QAbstractTableModel): self.reset_track_sequence_row_numbers() self.invalidate_rows(list(range(new_row_number, len(self.playlist_rows)))) - @line_profiler.profile + # Keep this decorator for now + # @line_profiler.profile def invalidate_row(self, modified_row: int) -> None: """ Signal to view to refresh invalidated row @@ -742,7 +748,8 @@ class PlaylistModel(QAbstractTableModel): self.index(modified_row, self.columnCount() - 1), ) - @line_profiler.profile + # Keep this decorator for now + # @line_profiler.profile def invalidate_rows(self, modified_rows: list[int]) -> None: """ Signal to view to refresh invlidated rows @@ -1558,19 +1565,20 @@ class PlaylistModel(QAbstractTableModel): def supportedDropActions(self) -> Qt.DropAction: return Qt.DropAction.MoveAction | Qt.DropAction.CopyAction - def tooltip_role(self, row: int, column: int, rat: RowAndTrack) -> QVariant: + # @line_profiler.profile + def tooltip_role(self, row: int, column: int, rat: RowAndTrack) -> str: """ Return tooltip. Currently only used for last_played column. """ if column != Col.LAST_PLAYED.value: - return QVariant() + return "" with db.Session() as session: track_id = self.playlist_rows[row].track_id if not track_id: - return QVariant() + return "" playdates = Playdates.last_playdates(session, track_id) - return QVariant( + return ( "
".join( [ a.lastplayed.strftime(Config.LAST_PLAYED_TOOLTIP_DATE_FORMAT) From 85493de179dbae556daaa0207fc01a706da078f0 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 12:03:47 +0000 Subject: [PATCH 6/8] Remove profiling decorators --- app/playlistmodel.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/playlistmodel.py b/app/playlistmodel.py index 1c4c329..4663275 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -624,7 +624,6 @@ class PlaylistModel(QAbstractTableModel): for a in self.playlist_rows.values() if not a.played and a.track_id is not None ] - # log.debug(f"{self}: get_unplayed_rows() returned: {result=}") return result def headerData( @@ -1146,7 +1145,6 @@ class PlaylistModel(QAbstractTableModel): self.signals.resize_rows_signal.emit(self.playlist_id) session.commit() - @line_profiler.profile def reset_track_sequence_row_numbers(self) -> None: """ Signal handler for when row ordering has changed. @@ -1281,7 +1279,6 @@ class PlaylistModel(QAbstractTableModel): return header_text - @line_profiler.profile def rowCount(self, index: QModelIndex = QModelIndex()) -> int: """Standard function for view""" @@ -1604,7 +1601,6 @@ class PlaylistModel(QAbstractTableModel): else: self.insert_row(proposed_row_number=row_number, track_id=track_id) - @line_profiler.profile def update_track_times(self) -> None: """ Update track start/end times in self.playlist_rows From 963da0b5d0b88fe1793f5f157cd3f14266de82e2 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 21:30:37 +0000 Subject: [PATCH 7/8] No db calls when servicing data() except for caching --- app/models.py | 8 ++++---- app/music_manager.py | 6 ++++++ app/playlistmodel.py | 41 ++++++++++++++++++++++------------------- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/app/models.py b/app/models.py index 66f8c73..cec667d 100644 --- a/app/models.py +++ b/app/models.py @@ -109,7 +109,7 @@ class NoteColours(dbtables.NoteColoursTable): @staticmethod def get_colour( session: Session, text: str, foreground: bool = False - ) -> Optional[str]: + ) -> str: """ Parse text and return background (foreground if foreground==True) colour string if matched, else None @@ -117,7 +117,7 @@ class NoteColours(dbtables.NoteColoursTable): """ if not text: - return None + return "" match = False for rec in NoteColours.get_all(session): @@ -138,10 +138,10 @@ class NoteColours(dbtables.NoteColoursTable): if match: if foreground: - return rec.foreground + return rec.foreground or "" else: return rec.colour - return None + return "" def invalidate_cache(self) -> None: """Invalidate dogpile cache""" diff --git a/app/music_manager.py b/app/music_manager.py index 565f0dc..9ae6147 100644 --- a/app/music_manager.py +++ b/app/music_manager.py @@ -439,6 +439,12 @@ class RowAndTrack: self.row_number = playlist_row.row_number self.track_id = playlist_row.track_id + # Playlist display data + self.row_fg: Optional[str] = None + self.row_bg: Optional[str] = None + self.note_fg: Optional[str] = None + self.note_bg: Optional[str] = None + # Collect track data if there's a track if playlist_row.track_id: self.artist = playlist_row.track.artist diff --git a/app/playlistmodel.py b/app/playlistmodel.py index 4663275..04205a5 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -70,8 +70,11 @@ class PlaylistModel(QAbstractTableModel): database. """ - def __init__(self, playlist_id: int, is_template: bool,) -> None: - + def __init__( + self, + playlist_id: int, + is_template: bool, + ) -> None: super().__init__() log.debug("PlaylistModel.__init__()") @@ -182,12 +185,13 @@ class PlaylistModel(QAbstractTableModel): # Header row if self.is_header_row(row): # Check for specific header colouring - with db.Session() as session: - note_background = NoteColours.get_colour(session, rat.note) - if note_background: - return QBrush(QColor(note_background)) - else: - return QBrush(QColor(Config.COLOUR_NOTES_PLAYLIST)) + if rat.row_bg is None: + with db.Session() as session: + rat.row_bg = NoteColours.get_colour(session, rat.note) + if rat.row_bg: + return QBrush(QColor(rat.row_bg)) + else: + return QBrush(QColor(Config.COLOUR_NOTES_PLAYLIST)) # Unreadable track file if file_is_unreadable(rat.path): return QBrush(QColor(Config.COLOUR_UNREADABLE)) @@ -219,10 +223,11 @@ class PlaylistModel(QAbstractTableModel): return QBrush(QColor(Config.COLOUR_BITRATE_OK)) if column == Col.NOTE.value: if rat.note: - with db.Session() as session: - note_background = NoteColours.get_colour(session, rat.note) - if note_background: - return QBrush(QColor(note_background)) + if rat.note_bg is None: + with db.Session() as session: + rat.note_bg = NoteColours.get_colour(session, rat.note) + if rat.note_bg: + return QBrush(QColor(rat.note_bg)) return QBrush() @@ -1575,13 +1580,11 @@ class PlaylistModel(QAbstractTableModel): if not track_id: return "" playdates = Playdates.last_playdates(session, track_id) - return ( - "
".join( - [ - a.lastplayed.strftime(Config.LAST_PLAYED_TOOLTIP_DATE_FORMAT) - for a in playdates - ] - ) + return "
".join( + [ + a.lastplayed.strftime(Config.LAST_PLAYED_TOOLTIP_DATE_FORMAT) + for a in playdates + ] ) def update_or_insert(self, track_id: int, row_number: int) -> None: From 74402f640f41af16771042c6a87aa5c9c5a8b499 Mon Sep 17 00:00:00 2001 From: Keith Edmunds Date: Sat, 8 Mar 2025 21:36:09 +0000 Subject: [PATCH 8/8] Only invalidate required roles --- app/musicmuster.py | 5 +- app/playlistmodel.py | 131 +++++++++++++++++++++++++++++++------------ 2 files changed, 99 insertions(+), 37 deletions(-) diff --git a/app/musicmuster.py b/app/musicmuster.py index 9df8eb2..0e22a3f 100755 --- a/app/musicmuster.py +++ b/app/musicmuster.py @@ -2308,7 +2308,10 @@ class Window(QMainWindow): session.commit() self.preview_manager.set_intro(intro) self.current.base_model.refresh_row(session, row_number) - self.current.base_model.invalidate_row(row_number) + roles = [ + Qt.ItemDataRole.DisplayRole, + ] + self.current.base_model.invalidate_row(row_number, roles) def preview_start(self) -> None: """Restart preview""" diff --git a/app/playlistmodel.py b/app/playlistmodel.py index 04205a5..4be929f 100644 --- a/app/playlistmodel.py +++ b/app/playlistmodel.py @@ -173,11 +173,17 @@ class PlaylistModel(QAbstractTableModel): # Update local copy self.refresh_row(session, row_number) # Repaint row - self.invalidate_row(row_number) + roles = [ + Qt.ItemDataRole.BackgroundRole, + Qt.ItemDataRole.DisplayRole, + Qt.ItemDataRole.FontRole, + Qt.ItemDataRole.ForegroundRole, + ] + # only invalidate required roles + self.invalidate_row(row_number, roles) self.signals.resize_rows_signal.emit(self.playlist_id) - # @line_profiler.profile def background_role(self, row: int, column: int, rat: RowAndTrack) -> QBrush: """Return background setting""" @@ -295,11 +301,16 @@ class PlaylistModel(QAbstractTableModel): ) # Update colour and times for current row - self.invalidate_row(row_number) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.DisplayRole + ] + self.invalidate_row(row_number, roles) # Update previous row in case we're hiding played rows if track_sequence.previous and track_sequence.previous.row_number: - self.invalidate_row(track_sequence.previous.row_number) + # only invalidate required roles + self.invalidate_row(track_sequence.previous.row_number, roles) # Update all other track times self.update_track_times() @@ -320,7 +331,6 @@ class PlaylistModel(QAbstractTableModel): if next_row is not None: self.set_next_row(next_row) - # @line_profiler.profile def data( self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole ) -> QVariant | QFont | QBrush | str: @@ -329,7 +339,8 @@ class PlaylistModel(QAbstractTableModel): if ( not index.isValid() or not (0 <= index.row() < len(self.playlist_rows)) - or role in [ + or role + in [ Qt.ItemDataRole.DecorationRole, Qt.ItemDataRole.StatusTipRole, Qt.ItemDataRole.WhatsThisRole, @@ -392,7 +403,6 @@ class PlaylistModel(QAbstractTableModel): self.reset_track_sequence_row_numbers() self.update_track_times() - # @line_profiler.profile def display_role(self, row: int, column: int, rat: RowAndTrack) -> str: """ Return text for display @@ -468,7 +478,6 @@ class PlaylistModel(QAbstractTableModel): super().endResetModel() self.reset_track_sequence_row_numbers() - # @line_profiler.profile def edit_role(self, row: int, column: int, rat: RowAndTrack) -> str: """ Return text for editing @@ -490,17 +499,17 @@ class PlaylistModel(QAbstractTableModel): return "" - # @line_profiler.profile def foreground_role(self, row: int, column: int, rat: RowAndTrack) -> QBrush: """Return header foreground colour or QBrush() if none""" if self.is_header_row(row): - with db.Session() as session: - note_foreground = NoteColours.get_colour( - session, rat.note, foreground=True - ) - if note_foreground: - return QBrush(QColor(note_foreground)) + if rat.row_fg is None: + with db.Session() as session: + rat.row_fg = NoteColours.get_colour( + session, rat.note, foreground=True + ) + if rat.row_fg: + return QBrush(QColor(rat.row_fg)) return QBrush() @@ -527,7 +536,6 @@ class PlaylistModel(QAbstractTableModel): return default - # @line_profiler.profile def font_role(self, row: int, column: int, rat: RowAndTrack) -> QFont: """ Return font @@ -704,7 +712,11 @@ class PlaylistModel(QAbstractTableModel): self.played_tracks_hidden = hide for row_number in range(len(self.playlist_rows)): if self.is_played_row(row_number): - self.invalidate_row(row_number) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.DisplayRole, + ] + self.invalidate_row(row_number, roles) def insert_row( self, @@ -736,11 +748,16 @@ class PlaylistModel(QAbstractTableModel): self.signals.resize_rows_signal.emit(self.playlist_id) self.reset_track_sequence_row_numbers() - self.invalidate_rows(list(range(new_row_number, len(self.playlist_rows)))) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.BackgroundRole, + Qt.ItemDataRole.DisplayRole, + Qt.ItemDataRole.FontRole, + Qt.ItemDataRole.ForegroundRole, + ] + self.invalidate_rows(list(range(new_row_number, len(self.playlist_rows))), roles) - # Keep this decorator for now - # @line_profiler.profile - def invalidate_row(self, modified_row: int) -> None: + def invalidate_row(self, modified_row: int, roles: list[Qt.ItemDataRole]) -> None: """ Signal to view to refresh invalidated row """ @@ -750,11 +767,10 @@ class PlaylistModel(QAbstractTableModel): self.dataChanged.emit( self.index(modified_row, 0), self.index(modified_row, self.columnCount() - 1), + roles ) - # Keep this decorator for now - # @line_profiler.profile - def invalidate_rows(self, modified_rows: list[int]) -> None: + def invalidate_rows(self, modified_rows: list[int], roles: list[Qt.ItemDataRole]) -> None: """ Signal to view to refresh invlidated rows """ @@ -762,7 +778,8 @@ class PlaylistModel(QAbstractTableModel): log.debug(f"issue285: {self}: invalidate_rows({modified_rows=})") for modified_row in modified_rows: - self.invalidate_row(modified_row) + # only invalidate required roles + self.invalidate_row(modified_row, roles) def is_header_row(self, row_number: int) -> bool: """ @@ -837,7 +854,11 @@ class PlaylistModel(QAbstractTableModel): self.refresh_row(session, row_number) self.update_track_times() - self.invalidate_rows(row_numbers) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.FontRole, + ] + self.invalidate_rows(row_numbers, roles) def move_rows(self, from_rows: list[int], to_row_number: int) -> None: """ @@ -902,7 +923,11 @@ class PlaylistModel(QAbstractTableModel): # Update display self.reset_track_sequence_row_numbers() self.update_track_times() - self.invalidate_rows(list(row_map.keys())) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.DisplayRole, + ] + self.invalidate_rows(list(row_map.keys()), roles) def move_rows_between_playlists( self, @@ -1079,7 +1104,11 @@ class PlaylistModel(QAbstractTableModel): return # Update display - self.invalidate_row(track_sequence.previous.row_number) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.BackgroundRole, + ] + self.invalidate_row(track_sequence.previous.row_number, roles) def refresh_data(self, session: Session) -> None: """ @@ -1132,7 +1161,11 @@ class PlaylistModel(QAbstractTableModel): playlist_row.track_id = None session.commit() self.refresh_row(session, row_number) - self.invalidate_row(row_number) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.DisplayRole, + ] + self.invalidate_row(row_number, roles) def rescan_track(self, row_number: int) -> None: """ @@ -1146,7 +1179,12 @@ class PlaylistModel(QAbstractTableModel): set_track_metadata(track) self.refresh_row(session, row_number) self.update_track_times() - self.invalidate_row(row_number) + roles = [ + Qt.ItemDataRole.BackgroundRole, + Qt.ItemDataRole.DisplayRole, + ] + # only invalidate required roles + self.invalidate_row(row_number, roles) self.signals.resize_rows_signal.emit(self.playlist_id) session.commit() @@ -1206,7 +1244,13 @@ class PlaylistModel(QAbstractTableModel): # self.playlist_rows directly. self.playlist_rows[row_number].note = "" session.commit() - self.invalidate_rows(row_numbers) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.BackgroundRole, + Qt.ItemDataRole.DisplayRole, + Qt.ItemDataRole.ForegroundRole, + ] + self.invalidate_rows(row_numbers, roles) def _reversed_contiguous_row_groups( self, row_numbers: list[int] @@ -1415,9 +1459,14 @@ class PlaylistModel(QAbstractTableModel): self.signals.search_songfacts_signal.emit( self.playlist_rows[row_number].title ) + roles = [ + Qt.ItemDataRole.BackgroundRole, + ] if old_next_row is not None: - self.invalidate_row(old_next_row) - self.invalidate_row(row_number) + # only invalidate required roles + self.invalidate_row(old_next_row, roles) + # only invalidate required roles + self.invalidate_row(row_number, roles) self.signals.next_track_changed_signal.emit() self.update_track_times() @@ -1567,7 +1616,6 @@ class PlaylistModel(QAbstractTableModel): def supportedDropActions(self) -> Qt.DropAction: return Qt.DropAction.MoveAction | Qt.DropAction.CopyAction - # @line_profiler.profile def tooltip_role(self, row: int, column: int, rat: RowAndTrack) -> str: """ Return tooltip. Currently only used for last_played column. @@ -1600,7 +1648,14 @@ class PlaylistModel(QAbstractTableModel): with db.Session() as session: for row in track_rows: self.refresh_row(session, row) - self.invalidate_rows(track_rows) + # only invalidate required roles + roles = [ + Qt.ItemDataRole.BackgroundRole, + Qt.ItemDataRole.DisplayRole, + Qt.ItemDataRole.FontRole, + Qt.ItemDataRole.ForegroundRole, + ] + self.invalidate_rows(track_rows, roles) else: self.insert_row(proposed_row_number=row_number, track_id=track_id) @@ -1749,9 +1804,13 @@ class PlaylistProxyModel(QSortFilterProxyModel): # milliseconds so that it hides then. We add # 100mS on so that the if clause above is # true next time through. + # only invalidate required roles + roles = [ + Qt.ItemDataRole.DisplayRole, + ] QTimer.singleShot( Config.HIDE_AFTER_PLAYING_OFFSET + 100, - lambda: self.sourceModel().invalidate_row(source_row), + lambda: self.sourceModel().invalidate_row(source_row, roles), ) return True # Next track not playing yet so don't hide previous