From 8c15db6c4a6a93b0f63759953815db228e539f10 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 16:21:33 +0100 Subject: [PATCH 01/35] patch closeEvent --- mne/gui/_coreg.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 76d24035428..a5262f8cf8b 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -174,6 +174,7 @@ def _get_default(var, val): self._to_cf_t = None self._omit_hsp_distance = 0.0 self._fiducials_file = None + self._is_trans_saved = False self._fid_colors = tuple( DEFAULTS['coreg'][f'{key}_color'] for key in ('lpa', 'nasion', 'rpa')) @@ -230,6 +231,32 @@ def _get_default(var, val): self._renderer.set_interaction(interaction) self._renderer._status_bar_initialize() + # BEGIN: patch the window + win = self._renderer._window + + def closeEventCallback(event): + if not self._is_trans_saved: + from PyQt5.QtWidgets import QMessageBox + ret = QMessageBox.warning( + win, + "CoregistrationUI", + "The Head<>MRI transform has not been saved. " + "Do you want to save it?", + QMessageBox.Save | QMessageBox.Cancel, + QMessageBox.Save + ) + if ret == QMessageBox.Save: + self._forward_widget_command( + "save_trans", "set_value", None) + else: + assert ret == QMessageBox.Cancel + # close the window + win.signal_close.emit() + event.accept() + + win.closeEvent = closeEventCallback + # END: patch the window + # coregistration model setup self._immediate_redraw = (self._renderer._kind != 'qt') self._info = info @@ -1237,6 +1264,7 @@ def _save_trans(self, fname): write_trans(fname, self.coreg.trans, overwrite=True) self._display_message( f"{fname} transform file is saved.") + self._is_trans_saved = True def _load_trans(self, fname): mri_head_t = _ensure_trans(read_trans(fname, return_all=True), From 435acae834280399404d557fe2f7c19c457e8985 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 16:42:53 +0100 Subject: [PATCH 02/35] save_mri_fids --- mne/gui/_coreg.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index a5262f8cf8b..e0d5e3fc6d5 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -248,6 +248,8 @@ def closeEventCallback(event): if ret == QMessageBox.Save: self._forward_widget_command( "save_trans", "set_value", None) + self._forward_widget_command( + "save_mri_fids", "set_value", None) else: assert ret == QMessageBox.Cancel # close the window From 6818814a82118cc9ac02f00e283d00c01afe5062 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 16:50:08 +0100 Subject: [PATCH 03/35] refactor --- mne/gui/_coreg.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index e0d5e3fc6d5..d73d99e0682 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -174,7 +174,8 @@ def _get_default(var, val): self._to_cf_t = None self._omit_hsp_distance = 0.0 self._fiducials_file = None - self._is_trans_saved = False + self._trans_saved = False + self._fids_saved = False self._fid_colors = tuple( DEFAULTS['coreg'][f'{key}_color'] for key in ('lpa', 'nasion', 'rpa')) @@ -235,7 +236,7 @@ def _get_default(var, val): win = self._renderer._window def closeEventCallback(event): - if not self._is_trans_saved: + if not self._trans_saved: from PyQt5.QtWidgets import QMessageBox ret = QMessageBox.warning( win, @@ -248,10 +249,11 @@ def closeEventCallback(event): if ret == QMessageBox.Save: self._forward_widget_command( "save_trans", "set_value", None) - self._forward_widget_command( - "save_mri_fids", "set_value", None) else: assert ret == QMessageBox.Cancel + if not self._fids_saved: + self._forward_widget_command( + "save_mri_fids", "set_value", None) # close the window win.signal_close.emit() event.accept() @@ -970,6 +972,8 @@ def _reset(self, keep_trans=False): self._update_plot() self._update_parameters() self._update_distance_estimation() + self._trans_saved = False + self._fids_saved = False def _forward_widget_command(self, names, command, value, input_value=True, output_value=False): @@ -1261,12 +1265,13 @@ def _save_mri_fiducials(self, fname): ) self._set_fiducials_file(fname) self._display_message(f"Saving {fname}... Done!") + self._fids_saved = True def _save_trans(self, fname): write_trans(fname, self.coreg.trans, overwrite=True) self._display_message( f"{fname} transform file is saved.") - self._is_trans_saved = True + self._trans_saved = True def _load_trans(self, fname): mri_head_t = _ensure_trans(read_trans(fname, return_all=True), From e1ae907a3112285ac04df307435583ae079c5fc6 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 17:02:50 +0100 Subject: [PATCH 04/35] draft --- mne/gui/_coreg.py | 16 +++++----------- mne/viz/backends/_notebook.py | 2 +- mne/viz/backends/_qt.py | 10 +++++++++- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index d73d99e0682..b93efcf15c2 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -228,18 +228,17 @@ def _get_default(var, val): self._renderer = _get_renderer( size=self._defaults["size"], bgcolor=self._defaults["bgcolor"]) self._renderer.enable_depth_peeling() - self._renderer._window_close_connect(self._clean) self._renderer.set_interaction(interaction) self._renderer._status_bar_initialize() - # BEGIN: patch the window - win = self._renderer._window + # connect callbacks to close event + self._renderer._window_close_connect(self._clean) - def closeEventCallback(event): + def closeEventCallback(): if not self._trans_saved: from PyQt5.QtWidgets import QMessageBox ret = QMessageBox.warning( - win, + self._renderer._window, "CoregistrationUI", "The Head<>MRI transform has not been saved. " "Do you want to save it?", @@ -254,12 +253,7 @@ def closeEventCallback(event): if not self._fids_saved: self._forward_widget_command( "save_mri_fids", "set_value", None) - # close the window - win.signal_close.emit() - event.accept() - - win.closeEvent = closeEventCallback - # END: patch the window + self._renderer._window_close_connect(closeEventCallback) # coregistration model setup self._immediate_redraw = (self._renderer._kind != 'qt') diff --git a/mne/viz/backends/_notebook.py b/mne/viz/backends/_notebook.py index 23c7d663a91..d9ebc9a1e0f 100644 --- a/mne/viz/backends/_notebook.py +++ b/mne/viz/backends/_notebook.py @@ -369,7 +369,7 @@ def __init__(self, brain, width, height, dpi): class _IpyWindow(_AbstractWindow): def _window_close_connect(self, func): - pass + self._window = None def _window_get_dpi(self): return 96 diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index ace163d3414..2367d402b11 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -506,13 +506,21 @@ def _window_initialize(self): self._window = self.figure.plotter.app_window self._window.setLocale(QLocale(QLocale.Language.English)) self._window.signal_close.connect(self._window_clean) + self._window_close_callbacks = list() + + # patch closeEvent + def closeEvent(event): + for callback in self._window_close_callbacks: + callback() + self._window.signal_close.emit() + event.accept() def _window_clean(self): self.figure.plotter = None self._interactor = None def _window_close_connect(self, func): - self._window.signal_close.connect(func) + self._window_close_callbacks.append(func) def _window_get_dpi(self): return self._window.windowHandle().screen().logicalDotsPerInch() From 91be3a3e9ccd6ed58e0fe0f094517d7c7f4897ff Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 17:36:01 +0100 Subject: [PATCH 05/35] fix --- mne/gui/_coreg.py | 3 +-- mne/viz/backends/_qt.py | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index b93efcf15c2..eda709cce63 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -232,8 +232,6 @@ def _get_default(var, val): self._renderer._status_bar_initialize() # connect callbacks to close event - self._renderer._window_close_connect(self._clean) - def closeEventCallback(): if not self._trans_saved: from PyQt5.QtWidgets import QMessageBox @@ -253,6 +251,7 @@ def closeEventCallback(): if not self._fids_saved: self._forward_widget_command( "save_mri_fids", "set_value", None) + self._clean() self._renderer._window_close_connect(closeEventCallback) # coregistration model setup diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index 2367d402b11..e20185e3da6 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -514,6 +514,7 @@ def closeEvent(event): callback() self._window.signal_close.emit() event.accept() + self._window.closeEvent = closeEvent def _window_clean(self): self.figure.plotter = None From 659745e6e62a4bdda78385b8d2d549121e100ff1 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 17:38:20 +0100 Subject: [PATCH 06/35] revert --- mne/viz/backends/_notebook.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/backends/_notebook.py b/mne/viz/backends/_notebook.py index d9ebc9a1e0f..23c7d663a91 100644 --- a/mne/viz/backends/_notebook.py +++ b/mne/viz/backends/_notebook.py @@ -369,7 +369,7 @@ def __init__(self, brain, width, height, dpi): class _IpyWindow(_AbstractWindow): def _window_close_connect(self, func): - self._window = None + pass def _window_get_dpi(self): return 96 From 9f698a40813fe4ef2b15a01495e4c2948797e88d Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 7 Feb 2022 17:42:38 +0100 Subject: [PATCH 07/35] update testing --- mne/gui/_coreg.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index eda709cce63..0584a3dca63 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -231,18 +231,22 @@ def _get_default(var, val): self._renderer.set_interaction(interaction) self._renderer._status_bar_initialize() - # connect callbacks to close event + # connect callback to close event def closeEventCallback(): + from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING if not self._trans_saved: from PyQt5.QtWidgets import QMessageBox - ret = QMessageBox.warning( - self._renderer._window, - "CoregistrationUI", - "The Head<>MRI transform has not been saved. " - "Do you want to save it?", - QMessageBox.Save | QMessageBox.Cancel, - QMessageBox.Save - ) + if MNE_3D_BACKEND_TESTING: + ret = QMessageBox.Cancel + else: + ret = QMessageBox.warning( + self._renderer._window, + "CoregistrationUI", + "The Head<>MRI transform has not been saved. " + "Do you want to save it?", + QMessageBox.Save | QMessageBox.Cancel, + QMessageBox.Save + ) if ret == QMessageBox.Save: self._forward_widget_command( "save_trans", "set_value", None) From 8652e15faa8e04a286861b09189bf5ae130c7caf Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Wed, 9 Feb 2022 16:34:25 +0100 Subject: [PATCH 08/35] refactor --- mne/gui/_coreg.py | 53 +++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 0584a3dca63..a3928b8fe8b 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -228,36 +228,10 @@ def _get_default(var, val): self._renderer = _get_renderer( size=self._defaults["size"], bgcolor=self._defaults["bgcolor"]) self._renderer.enable_depth_peeling() + self._renderer._window_close_connect(self._close_callback) self._renderer.set_interaction(interaction) self._renderer._status_bar_initialize() - # connect callback to close event - def closeEventCallback(): - from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING - if not self._trans_saved: - from PyQt5.QtWidgets import QMessageBox - if MNE_3D_BACKEND_TESTING: - ret = QMessageBox.Cancel - else: - ret = QMessageBox.warning( - self._renderer._window, - "CoregistrationUI", - "The Head<>MRI transform has not been saved. " - "Do you want to save it?", - QMessageBox.Save | QMessageBox.Cancel, - QMessageBox.Save - ) - if ret == QMessageBox.Save: - self._forward_widget_command( - "save_trans", "set_value", None) - else: - assert ret == QMessageBox.Cancel - if not self._fids_saved: - self._forward_widget_command( - "save_mri_fids", "set_value", None) - self._clean() - self._renderer._window_close_connect(closeEventCallback) - # coregistration model setup self._immediate_redraw = (self._renderer._kind != 'qt') self._info = info @@ -1722,3 +1696,28 @@ def _clean(self): def close(self): """Close interface and cleanup data structure.""" self._renderer.close() + + def _close_callback(self): + from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING + if not self._trans_saved: + from PyQt5.QtWidgets import QMessageBox + if MNE_3D_BACKEND_TESTING: + ret = QMessageBox.Cancel + else: + ret = QMessageBox.warning( + self._renderer._window, + "CoregistrationUI", + "The Head<>MRI transform has not been saved. " + "Do you want to save it?", + QMessageBox.Save | QMessageBox.Cancel, + QMessageBox.Save + ) + if ret == QMessageBox.Save: + self._forward_widget_command( + "save_trans", "set_value", None) + else: + assert ret == QMessageBox.Cancel + if not self._fids_saved: + self._forward_widget_command( + "save_mri_fids", "set_value", None) + self._clean() From 2a2b64134351b5ec5f70c6f35b929457403e77cd Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Wed, 9 Feb 2022 17:14:21 +0100 Subject: [PATCH 09/35] try again --- mne/viz/_brain/_brain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/_brain/_brain.py b/mne/viz/_brain/_brain.py index bf87ff29ab3..114012204b4 100644 --- a/mne/viz/_brain/_brain.py +++ b/mne/viz/_brain/_brain.py @@ -743,7 +743,7 @@ def _clean(self): for renderer in self._renderer._all_renderers: renderer.RemoveAllLights() # app_window cannot be set to None because it is used in __del__ - for key in ('lighting', 'interactor', '_RenderWindow'): + for key in ('lighting', 'interactor'): setattr(self.plotter, key, None) # Qt LeaveEvent requires _Iren so we use _FakeIren instead of None # to resolve the ref to vtkGenericRenderWindowInteractor From f68ddfc4f07ac7bf0d53bbcaa159a5808f62dcb6 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 15:47:09 +0100 Subject: [PATCH 10/35] add _dialog_warning --- mne/gui/_coreg.py | 38 ++++++++++++++++++----------------- mne/viz/backends/_abstract.py | 7 +++++++ mne/viz/backends/_notebook.py | 10 +++++++-- mne/viz/backends/_qt.py | 36 ++++++++++++++++++++++++++++++--- 4 files changed, 68 insertions(+), 23 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 580bd06c480..c57376a3e63 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1718,25 +1718,27 @@ def close(self): self._renderer.close() def _close_callback(self): - from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING if not self._trans_saved: - from PyQt5.QtWidgets import QMessageBox - if MNE_3D_BACKEND_TESTING: - ret = QMessageBox.Cancel - else: - ret = QMessageBox.warning( - self._renderer._window, - "CoregistrationUI", - "The Head<>MRI transform has not been saved. " - "Do you want to save it?", - QMessageBox.Save | QMessageBox.Cancel, - QMessageBox.Save - ) - if ret == QMessageBox.Save: - self._forward_widget_command( - "save_trans", "set_value", None) - else: - assert ret == QMessageBox.Cancel + from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING + + def callback(button_name): + if button_name == "Save": + self._forward_widget_command( + "save_trans", "set_value", None) + else: + assert button_name == "Cancel" + + self._widgets["close_dialog"] = self._renderer._dialog_warning( + title="CoregistrationUI", + text="The Head<>MRI transform has not been saved.", + info_text="Do you want to save it?", + callback=callback, + # modal=True means that the dialog blocks the application + # until one of the buttons is clicked + modal=not MNE_3D_BACKEND_TESTING, + ) + self._widgets["close_dialog"].show() + if not self._fids_saved: self._forward_widget_command( "save_mri_fids", "set_value", None) diff --git a/mne/viz/backends/_abstract.py b/mne/viz/backends/_abstract.py index e55b7be060a..3a579664108 100644 --- a/mne/viz/backends/_abstract.py +++ b/mne/viz/backends/_abstract.py @@ -628,6 +628,13 @@ def _playback_initialize(self, func, timeout, value, rng, pass +class _AbstractDialog(ABC): + @abstractmethod + def _dialog_warning(self, title, text, info_text, callback, *, + window=None): + pass + + class _AbstractLayout(ABC): @abstractmethod def _layout_initialize(self, max_width): diff --git a/mne/viz/backends/_notebook.py b/mne/viz/backends/_notebook.py index cb1d39f457c..ee26cd61594 100644 --- a/mne/viz/backends/_notebook.py +++ b/mne/viz/backends/_notebook.py @@ -15,10 +15,16 @@ _AbstractStatusBar, _AbstractLayout, _AbstractWidget, _AbstractWindow, _AbstractMplCanvas, _AbstractPlayback, _AbstractBrainMplCanvas, _AbstractMplInterface, - _AbstractWidgetList, _AbstractAction) + _AbstractWidgetList, _AbstractAction, _AbstractDialog) from ._pyvista import _PyVistaRenderer, _close_all, _set_3d_view, _set_3d_title # noqa: F401,E501, analysis:ignore +class _IpyDialog(_AbstractDialog): + def _dialog_warning(self, title, text, info_text, callback, *, + window=None): + pass + + class _IpyLayout(_AbstractLayout): def _layout_initialize(self, max_width): self._layout_max_width = max_width @@ -501,7 +507,7 @@ def trigger(self): class _Renderer(_PyVistaRenderer, _IpyDock, _IpyToolBar, _IpyMenuBar, - _IpyStatusBar, _IpyWindow, _IpyPlayback): + _IpyStatusBar, _IpyWindow, _IpyPlayback, _IpyDialog): _kind = 'notebook' def __init__(self, *args, **kwargs): diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index b91327f8559..40a3b4f783b 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -18,7 +18,7 @@ QSizePolicy, QScrollArea, QStyle, QProgressBar, QStyleOptionSlider, QLayout, QCheckBox, QButtonGroup, QRadioButton, QLineEdit, - QFileDialog, QPushButton) + QFileDialog, QPushButton, QMessageBox) from ._pyvista import _PyVistaRenderer from ._pyvista import (_close_all, _close_3d_figure, _check_3d_figure, # noqa: F401,E501 analysis:ignore @@ -27,11 +27,29 @@ _AbstractStatusBar, _AbstractLayout, _AbstractWidget, _AbstractWindow, _AbstractMplCanvas, _AbstractPlayback, _AbstractBrainMplCanvas, _AbstractMplInterface, - _AbstractWidgetList, _AbstractAction) + _AbstractWidgetList, _AbstractAction, _AbstractDialog) from ._utils import _init_qt_resources, _qt_disable_paint from ..utils import logger, _check_option +class _QtDialog(_AbstractDialog): + def _dialog_warning(self, title, text, info_text, callback, *, + modal=True, window=None): + window = self._window if window is None else window + widget = QMessageBox(window) + widget.setWindowTitle(title) + widget.setText(text) + widget.setInformativeText(info_text) + widget.setStandardButtons(QMessageBox.Save | QMessageBox.Cancel) + widget.setDefaultButton(QMessageBox.Save) + + def func(button): + callback(button.text()) + + widget.buttonClicked.connect(func) + return _QtDialogWidget(widget, modal) + + class _QtLayout(_AbstractLayout): def _layout_initialize(self, max_width): pass @@ -709,13 +727,25 @@ def set_tooltip(self, tooltip): self._widget.setToolTip(tooltip) +class _QtDialogWidget(_QtWidget): + def __init__(self, widget, modal): + super().__init__(widget) + self._modal = modal + + def show(self): + if self._modal: + self._widget.exec() + else: + self._widget.show() + + class _QtAction(_AbstractAction): def trigger(self): self._action.trigger() class _Renderer(_PyVistaRenderer, _QtDock, _QtToolBar, _QtMenuBar, - _QtStatusBar, _QtWindow, _QtPlayback): + _QtStatusBar, _QtWindow, _QtPlayback, _QtDialog): _kind = 'qt' def __init__(self, *args, **kwargs): From c4090d97b7392eb04c7c201fba133f49704d8034 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 15:51:34 +0100 Subject: [PATCH 11/35] set icon --- mne/viz/backends/_qt.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index 40a3b4f783b..b171b9f8e6d 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -39,6 +39,7 @@ def _dialog_warning(self, title, text, info_text, callback, *, widget = QMessageBox(window) widget.setWindowTitle(title) widget.setText(text) + widget.setIcon(QMessageBox.Warning) widget.setInformativeText(info_text) widget.setStandardButtons(QMessageBox.Save | QMessageBox.Cancel) widget.setDefaultButton(QMessageBox.Save) From 5fe7a9fe0ca995bf80c7140a60ad88e39447e4ad Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 15:58:24 +0100 Subject: [PATCH 12/35] nitpick --- mne/gui/_coreg.py | 2 +- mne/viz/backends/_abstract.py | 2 +- mne/viz/backends/_notebook.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 7001fc62015..810e1906241 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1733,7 +1733,7 @@ def callback(button_name): info_text="Do you want to save it?", callback=callback, # modal=True means that the dialog blocks the application - # until one of the buttons is clicked + # when show() is called, until one of the buttons is clicked modal=not MNE_3D_BACKEND_TESTING, ) self._widgets["close_dialog"].show() diff --git a/mne/viz/backends/_abstract.py b/mne/viz/backends/_abstract.py index fe008deefc8..9b0c0895554 100644 --- a/mne/viz/backends/_abstract.py +++ b/mne/viz/backends/_abstract.py @@ -626,7 +626,7 @@ def _playback_initialize(self, func, timeout, value, rng, class _AbstractDialog(ABC): @abstractmethod def _dialog_warning(self, title, text, info_text, callback, *, - window=None): + modal=True, window=None): pass diff --git a/mne/viz/backends/_notebook.py b/mne/viz/backends/_notebook.py index ee26cd61594..eea18d7ca16 100644 --- a/mne/viz/backends/_notebook.py +++ b/mne/viz/backends/_notebook.py @@ -21,7 +21,7 @@ class _IpyDialog(_AbstractDialog): def _dialog_warning(self, title, text, info_text, callback, *, - window=None): + modal=True, window=None): pass From 0ef604a0f31621de25fafec40cdc7798b666d989 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 17:06:07 +0100 Subject: [PATCH 13/35] one dialog for all --- mne/gui/_coreg.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 810e1906241..d66f5ba4474 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1717,20 +1717,32 @@ def close(self): self._renderer.close() def _close_callback(self): + # prepare the dialog text + text = "The " if not self._trans_saved: + text += "Head<>MRI transform" + if not self._fids_saved: + if text: + text += " and " + text += "fiducials" + + if not self._trans_saved or not self._fids_saved: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING + text += " has/have not been saved." def callback(button_name): if button_name == "Save": self._forward_widget_command( "save_trans", "set_value", None) + self._forward_widget_command( + "save_mri_fids", "set_value", None) else: assert button_name == "Cancel" self._widgets["close_dialog"] = self._renderer._dialog_warning( title="CoregistrationUI", - text="The Head<>MRI transform has not been saved.", - info_text="Do you want to save it?", + text=text, + info_text="Do you want to save?", callback=callback, # modal=True means that the dialog blocks the application # when show() is called, until one of the buttons is clicked @@ -1738,7 +1750,4 @@ def callback(button_name): ) self._widgets["close_dialog"].show() - if not self._fids_saved: - self._forward_widget_command( - "save_mri_fids", "set_value", None) self._clean() From 07f7e2f14fb879b6accdd842a49e85fe04665801 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 17:08:13 +0100 Subject: [PATCH 14/35] fix --- mne/gui/_coreg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index d66f5ba4474..f341f3081f7 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1718,7 +1718,7 @@ def close(self): def _close_callback(self): # prepare the dialog text - text = "The " + text = "" if not self._trans_saved: text += "Head<>MRI transform" if not self._fids_saved: From a2df6dc035585adefa58b7516312d1dec9b648c1 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 17:09:32 +0100 Subject: [PATCH 15/35] fix --- mne/gui/_coreg.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index f341f3081f7..acf2b107378 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1732,10 +1732,12 @@ def _close_callback(self): def callback(button_name): if button_name == "Save": - self._forward_widget_command( - "save_trans", "set_value", None) - self._forward_widget_command( - "save_mri_fids", "set_value", None) + if not self._trans_saved: + self._forward_widget_command( + "save_trans", "set_value", None) + if not self._fids_saved: + self._forward_widget_command( + "save_mri_fids", "set_value", None) else: assert button_name == "Cancel" From a221c2bca93f250b506e1caf7df6780844721a47 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 14 Feb 2022 17:11:10 +0100 Subject: [PATCH 16/35] refactor --- mne/gui/_coreg.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index acf2b107378..8ab626d547b 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1717,17 +1717,16 @@ def close(self): self._renderer.close() def _close_callback(self): - # prepare the dialog text - text = "" - if not self._trans_saved: - text += "Head<>MRI transform" - if not self._fids_saved: - if text: - text += " and " - text += "fiducials" - if not self._trans_saved or not self._fids_saved: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING + # prepare the dialog's text + text = "" + if not self._trans_saved: + text += "Head<>MRI transform" + if not self._fids_saved: + if text: + text += " and " + text += "fiducials" text += " has/have not been saved." def callback(button_name): From bd510b61e82cd55f2701f718067ed662acff1245 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Tue, 15 Feb 2022 16:16:03 +0100 Subject: [PATCH 17/35] remove _dialog_warning --- mne/viz/backends/_abstract.py | 7 ------- mne/viz/backends/_notebook.py | 10 ++-------- mne/viz/backends/_qt.py | 37 +++-------------------------------- 3 files changed, 5 insertions(+), 49 deletions(-) diff --git a/mne/viz/backends/_abstract.py b/mne/viz/backends/_abstract.py index 9b0c0895554..fdc65b1183b 100644 --- a/mne/viz/backends/_abstract.py +++ b/mne/viz/backends/_abstract.py @@ -623,13 +623,6 @@ def _playback_initialize(self, func, timeout, value, rng, pass -class _AbstractDialog(ABC): - @abstractmethod - def _dialog_warning(self, title, text, info_text, callback, *, - modal=True, window=None): - pass - - class _AbstractLayout(ABC): @abstractmethod def _layout_initialize(self, max_width): diff --git a/mne/viz/backends/_notebook.py b/mne/viz/backends/_notebook.py index eea18d7ca16..cb1d39f457c 100644 --- a/mne/viz/backends/_notebook.py +++ b/mne/viz/backends/_notebook.py @@ -15,16 +15,10 @@ _AbstractStatusBar, _AbstractLayout, _AbstractWidget, _AbstractWindow, _AbstractMplCanvas, _AbstractPlayback, _AbstractBrainMplCanvas, _AbstractMplInterface, - _AbstractWidgetList, _AbstractAction, _AbstractDialog) + _AbstractWidgetList, _AbstractAction) from ._pyvista import _PyVistaRenderer, _close_all, _set_3d_view, _set_3d_title # noqa: F401,E501, analysis:ignore -class _IpyDialog(_AbstractDialog): - def _dialog_warning(self, title, text, info_text, callback, *, - modal=True, window=None): - pass - - class _IpyLayout(_AbstractLayout): def _layout_initialize(self, max_width): self._layout_max_width = max_width @@ -507,7 +501,7 @@ def trigger(self): class _Renderer(_PyVistaRenderer, _IpyDock, _IpyToolBar, _IpyMenuBar, - _IpyStatusBar, _IpyWindow, _IpyPlayback, _IpyDialog): + _IpyStatusBar, _IpyWindow, _IpyPlayback): _kind = 'notebook' def __init__(self, *args, **kwargs): diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index b171b9f8e6d..b91327f8559 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -18,7 +18,7 @@ QSizePolicy, QScrollArea, QStyle, QProgressBar, QStyleOptionSlider, QLayout, QCheckBox, QButtonGroup, QRadioButton, QLineEdit, - QFileDialog, QPushButton, QMessageBox) + QFileDialog, QPushButton) from ._pyvista import _PyVistaRenderer from ._pyvista import (_close_all, _close_3d_figure, _check_3d_figure, # noqa: F401,E501 analysis:ignore @@ -27,30 +27,11 @@ _AbstractStatusBar, _AbstractLayout, _AbstractWidget, _AbstractWindow, _AbstractMplCanvas, _AbstractPlayback, _AbstractBrainMplCanvas, _AbstractMplInterface, - _AbstractWidgetList, _AbstractAction, _AbstractDialog) + _AbstractWidgetList, _AbstractAction) from ._utils import _init_qt_resources, _qt_disable_paint from ..utils import logger, _check_option -class _QtDialog(_AbstractDialog): - def _dialog_warning(self, title, text, info_text, callback, *, - modal=True, window=None): - window = self._window if window is None else window - widget = QMessageBox(window) - widget.setWindowTitle(title) - widget.setText(text) - widget.setIcon(QMessageBox.Warning) - widget.setInformativeText(info_text) - widget.setStandardButtons(QMessageBox.Save | QMessageBox.Cancel) - widget.setDefaultButton(QMessageBox.Save) - - def func(button): - callback(button.text()) - - widget.buttonClicked.connect(func) - return _QtDialogWidget(widget, modal) - - class _QtLayout(_AbstractLayout): def _layout_initialize(self, max_width): pass @@ -728,25 +709,13 @@ def set_tooltip(self, tooltip): self._widget.setToolTip(tooltip) -class _QtDialogWidget(_QtWidget): - def __init__(self, widget, modal): - super().__init__(widget) - self._modal = modal - - def show(self): - if self._modal: - self._widget.exec() - else: - self._widget.show() - - class _QtAction(_AbstractAction): def trigger(self): self._action.trigger() class _Renderer(_PyVistaRenderer, _QtDock, _QtToolBar, _QtMenuBar, - _QtStatusBar, _QtWindow, _QtPlayback, _QtDialog): + _QtStatusBar, _QtWindow, _QtPlayback): _kind = 'qt' def __init__(self, *args, **kwargs): From 102f488784c75fe1760c078924f6a9ff81df5d8c Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Tue, 15 Feb 2022 16:26:33 +0100 Subject: [PATCH 18/35] do not check [ci skip] From 77a3e46004b06d0b1224a841f02eb3e2fbe18c3b Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Tue, 15 Feb 2022 16:53:28 +0100 Subject: [PATCH 19/35] update --- mne/gui/_coreg.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 8ab626d547b..739730766d2 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1745,6 +1745,7 @@ def callback(button_name): text=text, info_text="Do you want to save?", callback=callback, + buttons=["Save", "Cancel"], # modal=True means that the dialog blocks the application # when show() is called, until one of the buttons is clicked modal=not MNE_3D_BACKEND_TESTING, From 4004f969d082c796e95bb1999dfda3a81b80bd9f Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 11:27:19 +0100 Subject: [PATCH 20/35] fix --- mne/viz/backends/_qt.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index 0bfaab3837c..d325efcfac6 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -59,7 +59,9 @@ def _dialog_warning(self, title, text, info_text, callback, *, widget.setDefaultButton(default_button) def func(button): - callback(button.text()) + # On MacOS, the text of the button may by prefixed by '&' + button_name = button.text().replace('&', '') + callback(button_name) widget.buttonClicked.connect(func) return _QtDialogWidget(widget, modal) From 576cc86e9320e07dedf2d2bb9198b4055cf02f28 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 12:03:02 +0100 Subject: [PATCH 21/35] improve testing --- mne/gui/_coreg.py | 8 +++++++- mne/gui/tests/test_coreg_gui.py | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 739730766d2..41770ea5885 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -176,6 +176,7 @@ def _get_default(var, val): self._fiducials_file = None self._trans_saved = False self._fids_saved = False + self._auto_cleanup = True self._fid_colors = tuple( DEFAULTS['coreg'][f'{key}_color'] for key in ('lpa', 'nasion', 'rpa')) @@ -1703,6 +1704,10 @@ def _configure_status_bar(self): 'status_message', 'hide', value=None, input_value=False ) + def _set_automatic_cleanup(self, state): + """Enable/Disable automatic cleanup (for testing purposes only).""" + self._auto_cleanup = state + def _clean(self): self._renderer = None self._widgets.clear() @@ -1752,4 +1757,5 @@ def callback(button_name): ) self._widgets["close_dialog"].show() - self._clean() + if self._auto_cleanup: + self._clean() diff --git a/mne/gui/tests/test_coreg_gui.py b/mne/gui/tests/test_coreg_gui.py index 2f3a5142753..fa74bb72e1d 100644 --- a/mne/gui/tests/test_coreg_gui.py +++ b/mne/gui/tests/test_coreg_gui.py @@ -217,7 +217,12 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): coreg._save_trans(tmp_trans) assert op.isfile(tmp_trans) + # test _close_callback() + coreg._set_automatic_cleanup(False) coreg.close() + coreg._widgets['close_dialog'].trigger('Cancel') # do not save + coreg._clean() # finally, cleanup internal structures + # Coregistration instance should survive assert isinstance(coreg.coreg, Coregistration) From 1f9403b4b5834eca82b168c8ba1b507874581396 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 12:05:15 +0100 Subject: [PATCH 22/35] fix --- mne/viz/backends/_qt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index 1013e2431dd..b3fe3c3f63b 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -59,7 +59,7 @@ def _dialog_warning(self, title, text, info_text, callback, *, widget.setDefaultButton(default_button) def func(button): - # On MacOS, the text of the button may by prefixed by '&' + # On MacOS, the text of the button may be prefixed by '&' button_name = button.text().replace('&', '') callback(button_name) From 931ebd53107aaaf3274a33ee16b8d0f8b2a6a0f3 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 14:36:38 +0100 Subject: [PATCH 23/35] refactor --- mne/gui/_coreg.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 41770ea5885..e74c3104e06 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -174,8 +174,8 @@ def _get_default(var, val): self._to_cf_t = None self._omit_hsp_distance = 0.0 self._fiducials_file = None - self._trans_saved = False - self._fids_saved = False + self._trans_modified = False + self._fids_modified = False self._auto_cleanup = True self._fid_colors = tuple( DEFAULTS['coreg'][f'{key}_color'] for key in @@ -293,6 +293,8 @@ def _get_default(var, val): self._set_lock_fids(True) # hack to make the dig disappear self._update_fiducials_label() self._update_fiducials() + # initialization does not count as modification by the user + self._fids_modified = False self._set_lock_fids(fid_accurate) @@ -435,6 +437,7 @@ def _set_scale_mode(self, mode): self._scale_mode = mode def _set_fiducial(self, value, coord): + self._fids_modified = True fid = self._current_fiducial fid_idx = _map_fid_name_to_idx(name=fid) @@ -445,6 +448,7 @@ def _set_fiducial(self, value, coord): self._update_plot("mri_fids") def _set_parameter(self, value, mode_name, coord): + self._trans_modified = True if self._params_locked: return if mode_name == "scale" and self._scale_mode == "uniform": @@ -943,8 +947,6 @@ def _reset(self, keep_trans=False): self._update_plot() self._update_parameters() self._update_distance_estimation() - self._trans_saved = False - self._fids_saved = False def _forward_widget_command(self, names, command, value, input_value=True, output_value=False): @@ -1236,13 +1238,13 @@ def _save_mri_fiducials(self, fname): ) self._set_fiducials_file(fname) self._display_message(f"Saving {fname}... Done!") - self._fids_saved = True + self._fids_modified = False def _save_trans(self, fname): write_trans(fname, self.coreg.trans, overwrite=True) self._display_message( f"{fname} transform file is saved.") - self._trans_saved = True + self._trans_modified = False def _load_trans(self, fname): mri_head_t = _ensure_trans(read_trans(fname, return_all=True), @@ -1722,13 +1724,13 @@ def close(self): self._renderer.close() def _close_callback(self): - if not self._trans_saved or not self._fids_saved: + if self._trans_modified or self._fids_modified: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING # prepare the dialog's text text = "" - if not self._trans_saved: + if self._trans_modified: text += "Head<>MRI transform" - if not self._fids_saved: + if self._fids_modified: if text: text += " and " text += "fiducials" @@ -1736,10 +1738,10 @@ def _close_callback(self): def callback(button_name): if button_name == "Save": - if not self._trans_saved: + if self._trans_modified: self._forward_widget_command( "save_trans", "set_value", None) - if not self._fids_saved: + if self._fids_modified: self._forward_widget_command( "save_mri_fids", "set_value", None) else: From cc6d4342b77d52796cc9b4d13e042d8bc59ac17c Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 14:37:40 +0100 Subject: [PATCH 24/35] fix --- mne/viz/backends/_qt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index b3fe3c3f63b..cf0939fe90c 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -59,7 +59,7 @@ def _dialog_warning(self, title, text, info_text, callback, *, widget.setDefaultButton(default_button) def func(button): - # On MacOS, the text of the button may be prefixed by '&' + # the text of the button may be prefixed by '&' button_name = button.text().replace('&', '') callback(button_name) From 4394ec436f350c44286ba71b2d509f2c54c6aced Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 14:46:21 +0100 Subject: [PATCH 25/35] assert --- mne/gui/tests/test_coreg_gui.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mne/gui/tests/test_coreg_gui.py b/mne/gui/tests/test_coreg_gui.py index fa74bb72e1d..d2279567047 100644 --- a/mne/gui/tests/test_coreg_gui.py +++ b/mne/gui/tests/test_coreg_gui.py @@ -213,12 +213,15 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): assert coreg._head_resolution == \ (config.get('MNE_COREG_HEAD_HIGH_RES', 'true') == 'true') + assert coreg._trans_modified tmp_trans = tmp_path / 'tmp-trans.fif' coreg._save_trans(tmp_trans) + assert not coreg._trans_modified assert op.isfile(tmp_trans) # test _close_callback() coreg._set_automatic_cleanup(False) + assert coreg._fids_modified coreg.close() coreg._widgets['close_dialog'].trigger('Cancel') # do not save coreg._clean() # finally, cleanup internal structures From 5438c026e01d3deee5eab970a7e8b1b33681452a Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Thu, 17 Feb 2022 14:49:49 +0100 Subject: [PATCH 26/35] assert --- mne/gui/tests/test_coreg_gui.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mne/gui/tests/test_coreg_gui.py b/mne/gui/tests/test_coreg_gui.py index d2279567047..7be2db01c89 100644 --- a/mne/gui/tests/test_coreg_gui.py +++ b/mne/gui/tests/test_coreg_gui.py @@ -168,12 +168,14 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): assert not coreg._lock_fids # picking + assert not coreg._fids_modified vtk_picker = TstVTKPicker(coreg._surfaces['head'], 0, (0, 0)) coreg._on_mouse_move(vtk_picker, None) coreg._on_button_press(vtk_picker, None) coreg._on_pick(vtk_picker, None) coreg._on_button_release(vtk_picker, None) coreg._on_pick(vtk_picker, None) # also pick when locked + assert coreg._fids_modified # lock fiducials coreg._set_lock_fids(True) @@ -221,7 +223,6 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): # test _close_callback() coreg._set_automatic_cleanup(False) - assert coreg._fids_modified coreg.close() coreg._widgets['close_dialog'].trigger('Cancel') # do not save coreg._clean() # finally, cleanup internal structures From 5318aaed6a929225c23b3875bef354322f89a94b Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Fri, 18 Feb 2022 15:20:42 +0100 Subject: [PATCH 27/35] save / discard / cancel --- mne/gui/_coreg.py | 11 ++++++++--- mne/gui/tests/test_coreg_gui.py | 2 +- mne/viz/backends/_qt.py | 13 ++++++++++--- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index e74c3104e06..c69fa25467d 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -176,6 +176,7 @@ def _get_default(var, val): self._fiducials_file = None self._trans_modified = False self._fids_modified = False + self._accept_close_event = True self._auto_cleanup = True self._fid_colors = tuple( DEFAULTS['coreg'][f'{key}_color'] for key in @@ -1737,6 +1738,7 @@ def _close_callback(self): text += " has/have not been saved." def callback(button_name): + self._accept_close_event = True if button_name == "Save": if self._trans_modified: self._forward_widget_command( @@ -1744,20 +1746,23 @@ def callback(button_name): if self._fids_modified: self._forward_widget_command( "save_mri_fids", "set_value", None) + elif button_name == "Cancel": + self._accept_close_event = False else: - assert button_name == "Cancel" + assert button_name == "Discard" self._widgets["close_dialog"] = self._renderer._dialog_warning( title="CoregistrationUI", text=text, info_text="Do you want to save?", callback=callback, - buttons=["Save", "Cancel"], + buttons=["Save", "Discard", "Cancel"], # modal=True means that the dialog blocks the application # when show() is called, until one of the buttons is clicked modal=not MNE_3D_BACKEND_TESTING, ) self._widgets["close_dialog"].show() - if self._auto_cleanup: + if self._accept_close_event and self._auto_cleanup: self._clean() + return self._accept_close_event diff --git a/mne/gui/tests/test_coreg_gui.py b/mne/gui/tests/test_coreg_gui.py index 7be2db01c89..9872ef31f48 100644 --- a/mne/gui/tests/test_coreg_gui.py +++ b/mne/gui/tests/test_coreg_gui.py @@ -224,7 +224,7 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): # test _close_callback() coreg._set_automatic_cleanup(False) coreg.close() - coreg._widgets['close_dialog'].trigger('Cancel') # do not save + coreg._widgets['close_dialog'].trigger('Discard') # do not save coreg._clean() # finally, cleanup internal structures # Coregistration instance should survive diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index cf0939fe90c..0cdb3611ade 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -545,10 +545,17 @@ def _window_initialize(self): # patch closeEvent def closeEvent(event): + accept_close_event = True for callback in self._window_close_callbacks: - callback() - self._window.signal_close.emit() - event.accept() + ret = callback() + # check if one of the callbacks ignores the close event + if isinstance(ret, bool) and not ret: + accept_close_event = False + if accept_close_event: + self._window.signal_close.emit() + event.accept() + else: + event.ignore() self._window.closeEvent = closeEvent def _window_clean(self): From 7a95b3a0a7e48a8f246d6d3765b3899131cfc097 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Fri, 18 Feb 2022 15:52:47 +0100 Subject: [PATCH 28/35] fix init --- mne/gui/_coreg.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index c69fa25467d..fb45febc62f 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -294,8 +294,6 @@ def _get_default(var, val): self._set_lock_fids(True) # hack to make the dig disappear self._update_fiducials_label() self._update_fiducials() - # initialization does not count as modification by the user - self._fids_modified = False self._set_lock_fids(fid_accurate) @@ -315,6 +313,9 @@ def _get_default(var, val): self._renderer.plotter.add_callback( self._redraw, self._refresh_rate_ms) self._renderer.plotter.show_axes() + # initialization does not count as modification by the user + self._trans_modified = False + self._fids_modified = False if block and self._renderer._kind != 'notebook': _qt_app_exec(self._renderer.figure.store["app"]) @@ -1724,6 +1725,20 @@ def close(self): """Close interface and cleanup data structure.""" self._renderer.close() + def _close_dialog_callback(self, button_name): + self._accept_close_event = True + if button_name == "Save": + if self._trans_modified: + self._forward_widget_command( + "save_trans", "set_value", None) + if self._fids_modified: + self._forward_widget_command( + "save_mri_fids", "set_value", None) + elif button_name == "Cancel": + self._accept_close_event = False + else: + assert button_name == "Discard" + def _close_callback(self): if self._trans_modified or self._fids_modified: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING @@ -1736,26 +1751,11 @@ def _close_callback(self): text += " and " text += "fiducials" text += " has/have not been saved." - - def callback(button_name): - self._accept_close_event = True - if button_name == "Save": - if self._trans_modified: - self._forward_widget_command( - "save_trans", "set_value", None) - if self._fids_modified: - self._forward_widget_command( - "save_mri_fids", "set_value", None) - elif button_name == "Cancel": - self._accept_close_event = False - else: - assert button_name == "Discard" - self._widgets["close_dialog"] = self._renderer._dialog_warning( title="CoregistrationUI", text=text, info_text="Do you want to save?", - callback=callback, + callback=self._close_dialog_callback, buttons=["Save", "Discard", "Cancel"], # modal=True means that the dialog blocks the application # when show() is called, until one of the buttons is clicked From 1cc1df0fe2e00b186ef7c0bc8f64823e99265eb1 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Sat, 19 Feb 2022 12:05:52 +0100 Subject: [PATCH 29/35] macos discard --- mne/viz/backends/_qt.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index 0cdb3611ade..59dfefe6930 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -61,6 +61,9 @@ def _dialog_warning(self, title, text, info_text, callback, *, def func(button): # the text of the button may be prefixed by '&' button_name = button.text().replace('&', '') + # handle MacOS Discard button + button_name = "Discard" \ + if button_name == "Don't save" else button_name callback(button_name) widget.buttonClicked.connect(func) From d81684674ba4d39f80964f6a3aa7e4ec6bad720a Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Sat, 19 Feb 2022 12:13:28 +0100 Subject: [PATCH 30/35] handle cancelling save_trans --- mne/gui/_coreg.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index fb45febc62f..8c3f4669075 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1731,6 +1731,9 @@ def _close_dialog_callback(self, button_name): if self._trans_modified: self._forward_widget_command( "save_trans", "set_value", None) + # cancel means _save_trans is not called + if self._trans_modified: + self._accept_close_event = False if self._fids_modified: self._forward_widget_command( "save_mri_fids", "set_value", None) From a9f2d608616aa056872ea08da0dc39a0a2f800e7 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Sat, 19 Feb 2022 12:25:42 +0100 Subject: [PATCH 31/35] refactor --- mne/gui/_coreg.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 8c3f4669075..0864ea092af 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1746,14 +1746,13 @@ def _close_callback(self): if self._trans_modified or self._fids_modified: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING # prepare the dialog's text - text = "" + text = "The following is/are not saved:" + text += "
    " if self._trans_modified: - text += "Head<>MRI transform" + text += "
  • Head<>MRI transform
    • " if self._fids_modified: - if text: - text += " and " - text += "fiducials" - text += " has/have not been saved." + text += "
  • fiducials
    • " + text += "
" self._widgets["close_dialog"] = self._renderer._dialog_warning( title="CoregistrationUI", text=text, From ab8fd7d5db7d25334a86b28361c582fe27f55d4a Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Sat, 19 Feb 2022 12:30:14 +0100 Subject: [PATCH 32/35] rename --- mne/gui/_coreg.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 0864ea092af..113353ce37b 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -175,7 +175,7 @@ def _get_default(var, val): self._omit_hsp_distance = 0.0 self._fiducials_file = None self._trans_modified = False - self._fids_modified = False + self._mri_fids_modified = False self._accept_close_event = True self._auto_cleanup = True self._fid_colors = tuple( @@ -315,7 +315,7 @@ def _get_default(var, val): self._renderer.plotter.show_axes() # initialization does not count as modification by the user self._trans_modified = False - self._fids_modified = False + self._mri_fids_modified = False if block and self._renderer._kind != 'notebook': _qt_app_exec(self._renderer.figure.store["app"]) @@ -439,7 +439,7 @@ def _set_scale_mode(self, mode): self._scale_mode = mode def _set_fiducial(self, value, coord): - self._fids_modified = True + self._mri_fids_modified = True fid = self._current_fiducial fid_idx = _map_fid_name_to_idx(name=fid) @@ -1240,7 +1240,7 @@ def _save_mri_fiducials(self, fname): ) self._set_fiducials_file(fname) self._display_message(f"Saving {fname}... Done!") - self._fids_modified = False + self._mri_fids_modified = False def _save_trans(self, fname): write_trans(fname, self.coreg.trans, overwrite=True) @@ -1734,7 +1734,7 @@ def _close_dialog_callback(self, button_name): # cancel means _save_trans is not called if self._trans_modified: self._accept_close_event = False - if self._fids_modified: + if self._mri_fids_modified: self._forward_widget_command( "save_mri_fids", "set_value", None) elif button_name == "Cancel": @@ -1743,14 +1743,14 @@ def _close_dialog_callback(self, button_name): assert button_name == "Discard" def _close_callback(self): - if self._trans_modified or self._fids_modified: + if self._trans_modified or self._mri_fids_modified: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING # prepare the dialog's text text = "The following is/are not saved:" text += "
    " if self._trans_modified: text += "
  • Head<>MRI transform
    • " - if self._fids_modified: + if self._mri_fids_modified: text += "
  • fiducials
    • " text += "
" self._widgets["close_dialog"] = self._renderer._dialog_warning( From 4d669c638ae5b34ccb7863878aed3b3b7139ee58 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Sat, 19 Feb 2022 13:11:57 +0100 Subject: [PATCH 33/35] mri_scale_modified --- mne/gui/_coreg.py | 33 +++++++++++++++++++++++++++++---- mne/gui/tests/test_coreg_gui.py | 6 ++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 113353ce37b..5738f24d3bc 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -176,6 +176,7 @@ def _get_default(var, val): self._fiducials_file = None self._trans_modified = False self._mri_fids_modified = False + self._mri_scale_modified = False self._accept_close_event = True self._auto_cleanup = True self._fid_colors = tuple( @@ -316,6 +317,7 @@ def _get_default(var, val): # initialization does not count as modification by the user self._trans_modified = False self._mri_fids_modified = False + self._mri_scale_modified = False if block and self._renderer._kind != 'notebook': _qt_app_exec(self._renderer.figure.store["app"]) @@ -450,7 +452,10 @@ def _set_fiducial(self, value, coord): self._update_plot("mri_fids") def _set_parameter(self, value, mode_name, coord): - self._trans_modified = True + if mode_name == "scale": + self._mri_scale_modified = True + else: + self._trans_modified = True if self._params_locked: return if mode_name == "scale" and self._scale_mode == "uniform": @@ -1231,6 +1236,7 @@ def _save_subject(self): self._display_message(f"Computing {bem_name} solution..." " Done!") self._display_message(f"Saving {self._subject_to}... Done!") + self._mri_scale_modified = False def _save_mri_fiducials(self, fname): self._display_message(f"Saving {fname}...") @@ -1726,6 +1732,7 @@ def close(self): self._renderer.close() def _close_dialog_callback(self, button_name): + from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING self._accept_close_event = True if button_name == "Save": if self._trans_modified: @@ -1737,21 +1744,39 @@ def _close_dialog_callback(self, button_name): if self._mri_fids_modified: self._forward_widget_command( "save_mri_fids", "set_value", None) + if self._mri_scale_modified: + if self._subject_to: + self._save_subject() + else: + dialog = self._renderer._dialog_warning( + title="CoregistrationUI", + text="The name of the output subject used to " + "save the scaled anatomy is not set.", + info_text="Please set a subject name", + callback=lambda x: None, + buttons=["Ok"], + modal=not MNE_3D_BACKEND_TESTING, + ) + dialog.show() + self._accept_close_event = False elif button_name == "Cancel": self._accept_close_event = False else: assert button_name == "Discard" def _close_callback(self): - if self._trans_modified or self._mri_fids_modified: + if self._trans_modified or self._mri_fids_modified or \ + self._mri_scale_modified: from ..viz.backends.renderer import MNE_3D_BACKEND_TESTING # prepare the dialog's text text = "The following is/are not saved:" text += "
    " if self._trans_modified: - text += "
  • Head<>MRI transform
    • " + text += "
  • Head<>MRI transform
    • " if self._mri_fids_modified: - text += "
  • fiducials
    • " + text += "
  • MRI fiducials
    • " + if self._mri_scale_modified: + text += "
  • scaled subject MRI
    • " text += "
" self._widgets["close_dialog"] = self._renderer._dialog_warning( title="CoregistrationUI", diff --git a/mne/gui/tests/test_coreg_gui.py b/mne/gui/tests/test_coreg_gui.py index 9872ef31f48..f25e4980e94 100644 --- a/mne/gui/tests/test_coreg_gui.py +++ b/mne/gui/tests/test_coreg_gui.py @@ -146,6 +146,7 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): assert coreg._fiducials_file == fid_fname # fitting (with scaling) + assert not coreg._mri_scale_modified coreg._reset() coreg._reset_fitting_parameters() coreg._set_scale_mode("uniform") @@ -161,6 +162,7 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): atol=1e-3) coreg._set_scale_mode("None") coreg._set_icp_fid_match("matched") + assert coreg._mri_scale_modified # unlock fiducials assert coreg._lock_fids @@ -168,14 +170,14 @@ def test_coreg_gui_pyvista(tmp_path, renderer_interactive_pyvistaqt): assert not coreg._lock_fids # picking - assert not coreg._fids_modified + assert not coreg._mri_fids_modified vtk_picker = TstVTKPicker(coreg._surfaces['head'], 0, (0, 0)) coreg._on_mouse_move(vtk_picker, None) coreg._on_button_press(vtk_picker, None) coreg._on_pick(vtk_picker, None) coreg._on_button_release(vtk_picker, None) coreg._on_pick(vtk_picker, None) # also pick when locked - assert coreg._fids_modified + assert coreg._mri_fids_modified # lock fiducials coreg._set_lock_fids(True) From 9ae9368e632de80e74d05d4050f4f9e9dcfa305a Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Sat, 19 Feb 2022 09:16:09 -0800 Subject: [PATCH 34/35] FIX: Do not check --- mne/gui/_coreg.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index 5738f24d3bc..e0a2d2cbf32 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1761,8 +1761,10 @@ def _close_dialog_callback(self, button_name): self._accept_close_event = False elif button_name == "Cancel": self._accept_close_event = False - else: - assert button_name == "Discard" + # This should pass, but in case Windows has a different name for this + # than Linux and macOS, let's just skip it... + # else: + # assert button_name in ("Discard", "Don't Save"), button_name def _close_callback(self): if self._trans_modified or self._mri_fids_modified or \ From a0086b740f58f2900100c5267db191f670375471 Mon Sep 17 00:00:00 2001 From: Guillaume Favelier Date: Mon, 21 Feb 2022 09:59:39 +0100 Subject: [PATCH 35/35] try again to standardize macos --- mne/gui/_coreg.py | 6 ++---- mne/viz/backends/_qt.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/mne/gui/_coreg.py b/mne/gui/_coreg.py index e0a2d2cbf32..5738f24d3bc 100644 --- a/mne/gui/_coreg.py +++ b/mne/gui/_coreg.py @@ -1761,10 +1761,8 @@ def _close_dialog_callback(self, button_name): self._accept_close_event = False elif button_name == "Cancel": self._accept_close_event = False - # This should pass, but in case Windows has a different name for this - # than Linux and macOS, let's just skip it... - # else: - # assert button_name in ("Discard", "Don't Save"), button_name + else: + assert button_name == "Discard" def _close_callback(self): if self._trans_modified or self._mri_fids_modified or \ diff --git a/mne/viz/backends/_qt.py b/mne/viz/backends/_qt.py index 59dfefe6930..28ebdbdd953 100644 --- a/mne/viz/backends/_qt.py +++ b/mne/viz/backends/_qt.py @@ -63,7 +63,7 @@ def func(button): button_name = button.text().replace('&', '') # handle MacOS Discard button button_name = "Discard" \ - if button_name == "Don't save" else button_name + if button_name == "Don't Save" else button_name callback(button_name) widget.buttonClicked.connect(func)