Merge branch 'master' into remove_qt_from_meta_yaml

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

@@ -17,6 +17,23 @@
    <layout class="QVBoxLayout" name="verticalLayout">
     <item>
      <layout class="QHBoxLayout" name="topHorizontalLayout">
+      <item>
+       <layout class="QVBoxLayout" name="topVerticalOriginal">
+        <item>
+         <widget class="QLabel" name="originalStackLabel">
+          <property name="styleSheet">
+           <string notr="true">QLabel{
+margin-left:5px;
+margin-top:3px;
+}</string>
+          </property>
+          <property name="text">
+           <string>Original Stack</string>
+          </property>
+         </widget>
+        </item>
+       </layout>
+      </item>
       <item>
        <widget class="Line" name="line">
         <property name="sizePolicy">
@@ -30,6 +47,22 @@
         </property>
        </widget>
       </item>
+      <item>
+       <layout class="QVBoxLayout" name="topVerticalNew">
+        <item>
+         <widget class="QLabel" name="newStackLabel">
+          <property name="styleSheet">
+           <string notr="true">QLabel{
+margin-top:3px;
+}</string>
+          </property>
+          <property name="text">
+           <string>New Stack</string>
+          </property>
+         </widget>
+        </item>
+       </layout>
+      </item>
      </layout>
     </item>
     <item>
@@ -54,6 +87,19 @@
         </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">
@@ -197,7 +243,7 @@
      <x>0</x>
      <y>0</y>
      <width>1338</width>
-     <height>22</height>
+     <height>25</height>
     </rect>
    </property>
   </widget>

mantidimaging/gui/windows/main/test/view_test.py

@@ -27,6 +27,15 @@ def test_execute_save(self):
 
         self.presenter.notify.assert_called_once_with(PresNotification.SAVE)
 
+    def test_find_images_stack_title(self):
+        images = mock.MagicMock()
+        self.presenter.get_stack_with_images = mock.MagicMock()
+
+        return_value = self.view.find_images_stack_title(images)
+
+        self.presenter.get_stack_with_images.assert_called_once_with(images)
+        self.assertEqual(return_value, self.presenter.get_stack_with_images.return_value.name)
+
     @mock.patch("mantidimaging.gui.windows.main.view.StackSelectorDialog")
     @mock.patch("mantidimaging.gui.windows.main.view.Qt.QFileDialog.getOpenFileName")
     def test_load_180_deg_dialog(self, get_open_file_name: mock.Mock, stack_selector_diag: mock.Mock):

mantidimaging/gui/windows/main/view.py

@@ -307,3 +307,6 @@ def show_stack_select_dialog(self):
 
     def set_images_in_stack(self, uuid: UUID, images: Images):
         self.presenter.set_images_in_stack(uuid, images)
+
+    def find_images_stack_title(self, images: Images) -> str:
+        return self.presenter.get_stack_with_images(images).name

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()
@@ -15,9 +18,16 @@ def __init__(self, stack_one: Images, stack_two: Images, parent):
         self.view.choice_made = True
         self.view.setWindowTitle("Comparing data")
 
+        stack_one_name = parent.find_images_stack_title(stack_one)
+        stack_two_name = parent.find_images_stack_title(stack_two)
+        self.view.originalStackLabel.setText(stack_one_name)
+        self.view.newStackLabel.setText(stack_two_name)
+
     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

@@ -3,9 +3,11 @@
 
 import unittest
 from unittest import mock
+from unittest.mock import call
 
 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):
@@ -31,3 +33,29 @@ def test_show(self, view):
         self.presenter.show()
 
         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"
+        custom_parent = mock.MagicMock()
+        custom_parent.find_images_stack_title.return_value = stack_name
+
+        self.presenter = StackComparePresenter(stack_one=self.stack_one, stack_two=self.stack_two, parent=custom_parent)
+
+        custom_parent.find_images_stack_title.assert_has_calls([call(self.stack_one), call(self.stack_two)])
+        self.assertEqual(2, custom_parent.find_images_stack_title.call_count)
+        view.return_value.originalStackLabel.setText.assert_called_once_with(stack_name)
+        view.return_value.newStackLabel.setText.assert_called_once_with(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()