Merge pull request #726 from mantidproject/705_option_to_lock_histogr…

…ams_in_compare

Adds Lock Histogram button and functionality

mantidimaging/gui/mvp_base/presenter.py

@@ -21,4 +21,4 @@ def show_error(self, error, traceback):
         if hasattr(self.view, 'show_error_dialog'):
             # If the view knows how to handle an error message
             self.view.show_error_dialog(str(error))
-        getLogger(__name__).error(f'Presenter error: {error}\n{traceback}')
+        getLogger(__name__).exception(f'Presenter error: {error}\n{traceback}')

mantidimaging/gui/ui/stack_choice_window.ui

@@ -87,6 +87,19 @@ margin-top:3px;
         </property>
        </spacer>
       </item>
+      <item>
+       <widget class="QCheckBox" name="lockHistograms">
+        <property name="toolTip">
+         <string>If activated - the histograms will share the value ranges for the contrast. If inactive they will be separate.</string>
+        </property>
+        <property name="text">
+         <string>Lock Histograms</string>
+        </property>
+        <property name="checkable">
+         <bool>true</bool>
+        </property>
+       </widget>
+      </item>
       <item>
        <widget class="QPushButton" name="roiButton">
         <property name="sizePolicy">
@@ -230,7 +243,7 @@ margin-top:3px;
      <x>0</x>
      <y>0</y>
      <width>1338</width>
-     <height>22</height>
+     <height>25</height>
     </rect>
    </property>
   </widget>

mantidimaging/gui/windows/operations/filter_previews.py

@@ -186,13 +186,14 @@ def mouse_over(self, ev):
                 self.display_formatted_detail[img](pixel_value)
 
     def link_all_views(self):
-        for view1, view2 in zip([self.image_before_vb, self.image_after_vb],
-                                [self.image_after_vb, self.image_difference_vb]):
+        for view1, view2 in [[self.image_before_vb, self.image_after_vb],
+                             [self.image_after_vb, self.image_difference_vb],
+                             [self.image_after_hist.vb, self.image_before_hist.vb]]:
             view1.linkView(ViewBox.XAxis, view2)
             view1.linkView(ViewBox.YAxis, view2)
 
     def unlink_all_views(self):
-        for view in self.image_before_vb, self.image_after_vb, self.image_difference_vb:
+        for view in self.image_before_vb, self.image_after_vb, self.image_after_hist.vb:
             view.linkView(ViewBox.XAxis, None)
             view.linkView(ViewBox.YAxis, None)
 

mantidimaging/gui/windows/stack_choice/compare_presenter.py

@@ -1,11 +1,14 @@
 # Copyright (C) 2020 ISIS Rutherford Appleton Laboratory UKRI
 # SPDX - License - Identifier: GPL-3.0-or-later
 
+import traceback
+
 from mantidimaging.core.data.images import Images
+from mantidimaging.gui.windows.stack_choice.presenter_base import StackChoicePresenterMixin
 from mantidimaging.gui.windows.stack_choice.view import StackChoiceView
 
 
-class StackComparePresenter:
+class StackComparePresenter(StackChoicePresenterMixin):
     def __init__(self, stack_one: Images, stack_two: Images, parent):
         self.view = StackChoiceView(stack_one, stack_two, self, parent)
         self.view.originalDataButton.hide()
@@ -23,6 +26,8 @@ def __init__(self, stack_one: Images, stack_two: Images, parent):
     def show(self):
         self.view.show()
 
-    def notify(self, notification):
-        # this presenter doesn't handle any notifications
-        pass
+    def notify(self, signal):
+        try:
+            super().notify(signal)
+        except Exception as e:
+            self.show_error(e, traceback.format_exc())

mantidimaging/gui/windows/stack_choice/presenter.py

@@ -1,13 +1,13 @@
 # Copyright (C) 2020 ISIS Rutherford Appleton Laboratory UKRI
 # SPDX - License - Identifier: GPL-3.0-or-later
 
-from mantidimaging.core.data.images import Images
 import traceback
-from logging import getLogger
-from typing import List, Optional, TYPE_CHECKING, Tuple, Union
+from typing import TYPE_CHECKING, List, Optional, Tuple, Union
 from uuid import UUID
 
-from mantidimaging.gui.windows.stack_choice.view import StackChoiceView, Notification
+from mantidimaging.core.data.images import Images
+from mantidimaging.gui.windows.stack_choice.presenter_base import StackChoicePresenterMixin
+from mantidimaging.gui.windows.stack_choice.view import Notification, StackChoiceView
 
 if TYPE_CHECKING:
     from mantidimaging.gui.windows.operations.presenter import FiltersWindowPresenter
@@ -20,7 +20,7 @@ def _get_stack_from_uuid(original_stack, stack_uuid):
     raise RuntimeError("No useful stacks passed to Stack Choice Presenter")
 
 
-class StackChoicePresenter:
+class StackChoicePresenter(StackChoicePresenterMixin):
     def __init__(self,
                  original_stack: Union[List[Tuple[Images, UUID]], Images],
                  new_stack: Images,
@@ -52,10 +52,10 @@ def notify(self, signal):
             elif signal == Notification.CHOOSE_NEW_DATA:
                 self.do_clean_up_original_data()
                 self.use_new_data = True
-
+            else:
+                super().notify(signal)
         except Exception as e:
-            self.operations_presenter.show_error(e, traceback.format_exc())
-            getLogger(__name__).exception("Notification handler failed")
+            self.show_error(e, traceback.format_exc())
 
     def _clean_up_original_images_stack(self):
         if isinstance(self.operations_presenter.original_images_stack, list) \

mantidimaging/gui/windows/stack_choice/presenter_base.py

@@ -0,0 +1,26 @@
+# Copyright (C) 2020 ISIS Rutherford Appleton Laboratory UKRI
+# SPDX - License - Identifier: GPL-3.0-or-later
+
+from mantidimaging.gui.mvp_base.presenter import BasePresenter
+from mantidimaging.gui.windows.stack_choice.view import Notification
+
+
+class StackChoicePresenterMixin(BasePresenter):
+    """
+    Implements common functions for StackChoice and StackCompare, but does
+    not do enough on it's own for a successful view initialisation - it needs
+    to be mixed into another presenter that extends it
+    """
+    def notify(self, signal: Notification):
+        if signal == Notification.TOGGLE_LOCK_HISTOGRAMS:
+            self.do_toggle_lock_histograms()
+
+    def do_toggle_lock_histograms(self):
+        # The state of the button changes before this signal is triggered
+        # so on first click you get isChecked = True
+        histograms_should_lock = self.view.lockHistograms.isChecked()
+
+        if histograms_should_lock:
+            self.view.connect_histogram_changes()
+        else:
+            self.view.disconnect_histogram_changes()

mantidimaging/gui/windows/stack_choice/tests/test_compare_presenter.py

@@ -7,6 +7,7 @@
 
 import mantidimaging.test_helpers.unit_test_helper as th
 from mantidimaging.gui.windows.stack_choice.compare_presenter import StackComparePresenter
+from mantidimaging.gui.windows.stack_choice.view import Notification
 
 
 class StackChoicePresenterTest(unittest.TestCase):
@@ -33,6 +34,19 @@ def test_show(self, view):
 
         view.return_value.show.assert_called_once()
 
+    @mock.patch("mantidimaging.gui.windows.stack_choice.compare_presenter.StackChoiceView")
+    def test_do_toggle_lock_histograms(self, view_class_mock):
+        view_instance = view_class_mock.return_value
+
+        view_instance.lockHistograms.isChecked.return_value = True
+        self.presenter = StackComparePresenter(stack_one=self.stack_one, stack_two=self.stack_two, parent=self.parent)
+        self.presenter.notify(Notification.TOGGLE_LOCK_HISTOGRAMS)
+        view_instance.connect_histogram_changes.assert_called_once()
+
+        view_instance.lockHistograms.isChecked.return_value = False
+        self.presenter.notify(Notification.TOGGLE_LOCK_HISTOGRAMS)
+        view_instance.disconnect_histogram_changes.assert_called_once()
+
     @mock.patch("mantidimaging.gui.windows.stack_choice.compare_presenter.StackChoiceView")
     def test_titles_set(self, view: mock.Mock):
         stack_name = "stack_name"

mantidimaging/gui/windows/stack_choice/tests/test_presenter.py

@@ -3,11 +3,12 @@
 
 import unittest
 from unittest import mock
+from unittest.mock import DEFAULT, MagicMock, Mock, patch
 from uuid import uuid4
 
 import mantidimaging.test_helpers.unit_test_helper as th
-from mantidimaging.gui.windows.stack_choice.view import Notification
 from mantidimaging.gui.windows.stack_choice.presenter import StackChoicePresenter
+from mantidimaging.gui.windows.stack_choice.view import Notification
 
 
 class StackChoicePresenterTest(unittest.TestCase):
@@ -48,13 +49,13 @@ def test_notify_choose_original(self):
 
         self.p.do_reapply_original_data.assert_called_once()
 
-    def test_notify_handles_exceptions(self):
-        self.p.do_reapply_original_data = mock.MagicMock()
-        self.p.do_reapply_original_data.side_effect = RuntimeError
-
+    @patch.multiple("mantidimaging.gui.windows.stack_choice.presenter.StackChoicePresenter",
+                    do_reapply_original_data=MagicMock(side_effect=RuntimeError),
+                    show_error=DEFAULT)
+    def test_notify_handles_exceptions(self, _: Mock = Mock(), show_error: Mock = Mock()):
         self.p.notify(Notification.CHOOSE_ORIGINAL)
 
-        self.p.operations_presenter.show_error.assert_called_once()
+        show_error.assert_called_once()
 
     def test_notify_choose_new_data(self):
         self.p.do_clean_up_original_data = mock.MagicMock()
@@ -108,3 +109,12 @@ def test_close_view_sets_done_true(self):
         self.p.close_view()
 
         self.assertTrue(self.p.done)
+
+    def test_do_toggle_lock_histograms(self):
+        self.v.lockHistograms.isChecked.return_value = True
+        self.p.notify(Notification.TOGGLE_LOCK_HISTOGRAMS)
+        self.v.connect_histogram_changes.assert_called_once()
+
+        self.v.lockHistograms.isChecked.return_value = False
+        self.p.notify(Notification.TOGGLE_LOCK_HISTOGRAMS)
+        self.v.disconnect_histogram_changes.assert_called_once()

mantidimaging/gui/windows/stack_choice/tests/test_view.py

@@ -3,6 +3,7 @@
 
 import unittest
 from unittest import mock
+from unittest.mock import DEFAULT, Mock, patch
 from uuid import uuid4
 
 from PyQt5.QtWidgets import QMessageBox
@@ -190,3 +191,32 @@ def test_ensure_range_is_the_same_new_stack_max_original_stack_min(self):
 
         self.v.new_stack.ui.histogram.vb.setRange.assert_called_once_with(yRange=(0, 200))
         self.v.original_stack.ui.histogram.vb.setRange.assert_called_once_with(yRange=(0, 200))
+
+    @patch.multiple('mantidimaging.gui.windows.stack_choice.view.StackChoiceView',
+                    _set_from_old_to_new=DEFAULT,
+                    _set_from_new_to_old=DEFAULT)
+    def test_connect_histogram_changes(self, _set_from_old_to_new: Mock = Mock(), _set_from_new_to_old: Mock = Mock()):
+        self.v.connect_histogram_changes()
+
+        # check this is called once to set the same range on the new histogram as is currently selected on the new one
+        _set_from_old_to_new.assert_called_once()
+
+        expected_emit = (0, 99)
+        _set_from_old_to_new.reset_mock()
+
+        self.v.original_stack.ui.histogram.sigLevelsChanged.emit(expected_emit)
+        _set_from_old_to_new.assert_called_once_with(expected_emit)
+
+        self.v.new_stack.ui.histogram.sigLevelsChanged.emit(expected_emit)
+        _set_from_new_to_old.assert_called_once_with(expected_emit)
+
+        # reset mocks and disconnect signals
+        _set_from_old_to_new.reset_mock()
+        _set_from_new_to_old.reset_mock()
+        self.v.disconnect_histogram_changes()
+
+        self.v.original_stack.ui.histogram.sigLevelsChanged.emit(expected_emit)
+        _set_from_old_to_new.assert_not_called()
+
+        self.v.new_stack.ui.histogram.sigLevelsChanged.emit(expected_emit)
+        _set_from_new_to_old.assert_not_called()

mantidimaging/gui/windows/stack_choice/view.py

@@ -2,12 +2,12 @@
 # SPDX - License - Identifier: GPL-3.0-or-later
 
 from enum import Enum, auto
-from typing import TYPE_CHECKING, Optional, Union
+from typing import TYPE_CHECKING, Optional, Tuple, Union
 
+import numpy as np
 from PyQt5 import QtCore
 from PyQt5.QtCore import Qt
-from PyQt5.QtWidgets import QMainWindow, QMessageBox, QPushButton, QSizePolicy
-import numpy as np
+from PyQt5.QtWidgets import QCheckBox, QMainWindow, QMessageBox, QPushButton, QSizePolicy
 from pyqtgraph import ViewBox
 
 from mantidimaging.core.data.images import Images
@@ -22,11 +22,13 @@
 class Notification(Enum):
     CHOOSE_ORIGINAL = auto()
     CHOOSE_NEW_DATA = auto()
+    TOGGLE_LOCK_HISTOGRAMS = auto()
 
 
 class StackChoiceView(BaseMainWindowView):
     originalDataButton: QPushButton
     newDataButton: QPushButton
+    lockHistograms: QCheckBox
 
     def __init__(self, original_stack: Images, new_stack: Images,
                  presenter: Union['StackComparePresenter', 'StackChoicePresenter'], parent: Optional[QMainWindow]):
@@ -65,6 +67,9 @@ def __init__(self, original_stack: Images, new_stack: Images,
         self.originalDataButton.clicked.connect(lambda: self.presenter.notify(Notification.CHOOSE_ORIGINAL))
         self.newDataButton.clicked.connect(lambda: self.presenter.notify(Notification.CHOOSE_NEW_DATA))
 
+        # Hooks the lock histograms checkbox
+        self.lockHistograms.clicked.connect(lambda: self.presenter.notify(Notification.TOGGLE_LOCK_HISTOGRAMS))
+
         # Hook ROI button into both stacks
         self.roiButton.clicked.connect(self._toggle_roi)
 
@@ -158,3 +163,33 @@ def closeEvent(self, e):
 
         self.original_stack.close()
         self.new_stack.close()
+
+    def _set_from_old_to_new(self):
+        """
+        Signal triggered when the histograms are locked and the contrast values changed.
+        """
+        levels: Tuple[float, float] = self.original_stack.ui.histogram.getLevels()
+        self.new_stack.ui.histogram.setLevels(*levels)
+
+    def _set_from_new_to_old(self):
+        """
+        Signal triggered when the histograms are locked and the contrast values changed.
+        """
+        levels: Tuple[float, float] = self.new_stack.ui.histogram.getLevels()
+        self.original_stack.ui.histogram.setLevels(*levels)
+
+    def connect_histogram_changes(self):
+        self._set_from_old_to_new()
+
+        self.original_stack.ui.histogram.sigLevelsChanged.connect(self._set_from_old_to_new)
+        self.new_stack.ui.histogram.sigLevelsChanged.connect(self._set_from_new_to_old)
+
+        self.new_stack.ui.histogram.vb.linkView(ViewBox.YAxis, self.original_stack.ui.histogram.vb)
+        self.new_stack.ui.histogram.vb.linkView(ViewBox.XAxis, self.original_stack.ui.histogram.vb)
+
+    def disconnect_histogram_changes(self):
+        self.original_stack.ui.histogram.sigLevelsChanged.disconnect(self._set_from_old_to_new)
+        self.new_stack.ui.histogram.sigLevelsChanged.disconnect(self._set_from_new_to_old)
+
+        self.new_stack.ui.histogram.vb.linkView(ViewBox.YAxis, None)
+        self.new_stack.ui.histogram.vb.linkView(ViewBox.XAxis, None)