Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Live viewer: Quick handle notification and defer slow work #2221

Merged
merged 3 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release_notes/next/fix-2204-live-viewer-fast-notifiy
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#2204: Live viewer, handle notification quickly and defer slow work
26 changes: 19 additions & 7 deletions mantidimaging/gui/windows/live_viewer/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from typing import TYPE_CHECKING
from pathlib import Path
from logging import getLogger
from PyQt5.QtCore import QFileSystemWatcher, QObject, pyqtSignal
from PyQt5.QtCore import QFileSystemWatcher, QObject, pyqtSignal, QTimer

if TYPE_CHECKING:
from os import stat_result
Expand Down Expand Up @@ -109,7 +109,7 @@ def path(self, path: Path) -> None:
self.image_watcher = ImageWatcher(path)
self.image_watcher.image_changed.connect(self._handle_image_changed_in_list)
self.image_watcher.recent_image_changed.connect(self.handle_image_modified)
self.image_watcher._handle_directory_change(str(path))
self.image_watcher._handle_notified_of_directry_change(str(path))

def _handle_image_changed_in_list(self, image_files: list[Image_Data]) -> None:
"""
Expand Down Expand Up @@ -171,7 +171,11 @@ def __init__(self, directory: Path):
super().__init__()
self.directory = directory
self.watcher = QFileSystemWatcher()
self.watcher.directoryChanged.connect(self._handle_directory_change)
self.watcher.directoryChanged.connect(self._handle_notified_of_directry_change)
self.handle_change_timer = QTimer(self)
self.handle_change_timer.setSingleShot(True)
self.handle_change_timer.timeout.connect(self._handle_directory_change)
self.changed_directory = directory

self.recent_file_watcher = QFileSystemWatcher()
self.recent_file_watcher.fileChanged.connect(self.handle_image_modified)
Expand Down Expand Up @@ -217,15 +221,23 @@ def sort_images_by_modified_time(images: list[Image_Data]) -> list[Image_Data]:
"""
return sorted(images, key=lambda x: x.image_modified_time)

def _handle_directory_change(self, directory: str) -> None:
def _handle_notified_of_directry_change(self, directory: str) -> None:
"""
Quickly handle the notification event for a change, and start the timer if needed. If files are arriving faster
than _handle_directory_change() can process them, we don't end up with a queue of work to do. The timeout is set
small so that we can pull multiple notification off the queue and then run _handle_directory_change() once.
"""
self.changed_directory = Path(directory)
if not self.handle_change_timer.isActive():
self.handle_change_timer.start(10)

def _handle_directory_change(self) -> None:
"""
Handle a directory change event. Update the list of images
to reflect directory changes and emit the image_changed signal
with the sorted image list.

:param directory: directory that has changed
"""
directory_path = Path(directory)
directory_path = self.changed_directory

# Force the modification time of signal directory, because file changes may not update
# parent dir mtime
Expand Down
12 changes: 8 additions & 4 deletions mantidimaging/gui/windows/live_viewer/test/model_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ def test_WHEN_directory_change_simple_THEN_images_emitted(self):
file_list = self._make_simple_dir(self.top_path)
self.mock_signal_image.emit.assert_not_called()

self.watcher._handle_directory_change(self.top_path)
self.watcher.changed_directory = self.top_path
self.watcher._handle_directory_change()

emitted_images = self._get_recent_emitted_files()
self._file_list_count_equal(emitted_images, file_list)
Expand All @@ -110,7 +111,8 @@ def test_WHEN_directory_change_empty_subdir_THEN_images_emitted(self, _mock_time
os.utime(self.top_path / "empty", [10, 2000])
self.assertLess(self.top_path.stat().st_mtime, (self.top_path / 'empty').stat().st_mtime)

self.watcher._handle_directory_change(self.top_path / "empty")
self.watcher.changed_directory = self.top_path / "empty"
self.watcher._handle_directory_change()

emitted_images = self._get_recent_emitted_files()
self._file_list_count_equal(emitted_images, file_list)
Expand All @@ -123,7 +125,8 @@ def test_WHEN_directory_change_with_subdir_THEN_images_emitted(self, _mock_time)
self.assertLess(self.top_path.stat().st_mtime, (self.top_path / 'more').stat().st_mtime)
self.assertLess((self.top_path / 'more').stat().st_mtime, time.time())

self.watcher._handle_directory_change(self.top_path)
self.watcher.changed_directory = self.top_path
self.watcher._handle_directory_change()

emitted_images = self._get_recent_emitted_files()
self._file_list_count_equal(emitted_images, file_list)
Expand All @@ -135,7 +138,8 @@ def test_WHEN_sub_directory_change_THEN_images_emitted(self, _mock_time):
file_list2 = self._make_simple_dir(self.top_path / "more", t0=2000)
self.assertLess(self.top_path.stat().st_mtime, (self.top_path / 'more').stat().st_mtime)

self.watcher._handle_directory_change(self.top_path / "more")
self.watcher.changed_directory = self.top_path / "more"
self.watcher._handle_directory_change()

emitted_images = self._get_recent_emitted_files()
self._file_list_count_equal(emitted_images, file_list2)