From d69e4a5a960459343c05526276ef97a97592f829 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Thu, 6 Jun 2024 15:44:55 +0100 Subject: [PATCH 1/3] SpectrumROI: avoid needing pos argument Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan --- .../gui/windows/spectrum_viewer/spectrum_widget.py | 4 ++-- .../windows/spectrum_viewer/test/spectrum_test.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py index 8b112a1ab31..77f3956e73e 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py +++ b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py @@ -30,11 +30,11 @@ class SpectrumROI(ROI): sig_colour_change = pyqtSignal(str, tuple) def __init__(self, name: str, sensible_roi: SensibleROI, *args, **kwargs): + kwargs["pos"] = sensible_roi.left, sensible_roi.top + kwargs["size"] = sensible_roi.width, sensible_roi.height super().__init__(*args, **kwargs) self._name = name self._colour = (0, 0, 0, 255) - self.setPos((sensible_roi.left, sensible_roi.top)) - self.setSize((sensible_roi.width, sensible_roi.height)) self.maxBounds = self.parentBounds() self.addScaleHandle([1, 1], [0, 0]) self.addScaleHandle([1, 0], [0, 1]) diff --git a/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py b/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py index ee6aa6b31a4..74659d08845 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py +++ b/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py @@ -7,6 +7,8 @@ from unittest import mock from parameterized import parameterized +from pyqtgraph import Point + from mantidimaging.gui.windows.main import MainWindowView from mantidimaging.gui.windows.spectrum_viewer import SpectrumViewerWindowView, SpectrumViewerWindowPresenter from mantidimaging.gui.windows.spectrum_viewer.spectrum_widget import SpectrumWidget, SpectrumPlotWidget @@ -15,6 +17,16 @@ from mantidimaging.gui.windows.spectrum_viewer.spectrum_widget import SpectrumROI +@start_qapplication +class SpectrumROITest(unittest.TestCase): + + def test_WHEN_initialise_THEN_pos_and_size_correct(self): + roi = SensibleROI(10, 20, 30, 40) + spectrum_roi = SpectrumROI("", roi) + self.assertEqual(spectrum_roi.getState()["pos"], Point(10, 20)) + self.assertEqual(spectrum_roi.getState()["size"], Point(20, 20)) + + @mock_versions @start_qapplication class SpectrumWidgetTest(unittest.TestCase): From c4ce799487725a9971ba4ff0ecb439b41973091a Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Thu, 6 Jun 2024 15:49:37 +0100 Subject: [PATCH 2/3] SpectrumROI: remove unneed pos argument Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan --- .../spectrum_viewer/spectrum_widget.py | 2 +- .../spectrum_viewer/test/spectrum_test.py | 56 +++---------------- 2 files changed, 9 insertions(+), 49 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py index 77f3956e73e..2f57a22bc22 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py +++ b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py @@ -196,7 +196,7 @@ def add_roi(self, roi: SensibleROI, name: str) -> None: @param roi: The ROI to add. @param name: The name of the ROI. """ - roi_object = SpectrumROI(name, roi, pos=(0, 0), rotatable=False, scaleSnap=True, translateSnap=True) + roi_object = SpectrumROI(name, roi, rotatable=False, scaleSnap=True, translateSnap=True) roi_object.colour = self.colour_generator() roi_object.sig_colour_change.connect(lambda name, color: self.roiColorChangeRequested.emit(name, color)) diff --git a/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py b/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py index 74659d08845..719d038e98f 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py +++ b/mantidimaging/gui/windows/spectrum_viewer/test/spectrum_test.py @@ -58,12 +58,7 @@ def test_WHEN_colour_generator_called_THEN_different_colours_returned(self): self.assertEqual(len(colour_list), len({tuple(c) for c in colour_list})) def test_WHEN_change_roi_colour_called_THEN_roi_colour_changed(self): - spectrum_roi = SpectrumROI("roi", - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI("roi", self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict["roi"] = spectrum_roi self.spectrum_widget.spectrum_data_dict["roi"] = np.array([0, 0, 0, 0]) roi_colour = self.spectrum_widget.roi_dict["roi"].pen.color().getRgb() @@ -73,12 +68,7 @@ def test_WHEN_change_roi_colour_called_THEN_roi_colour_changed(self): @parameterized.expand([("Visible", "visible_roi", 1), ("Invisible", "invisible_roi", 0)]) def test_WHEN_set_roi_visibility_flags_called_THEN_roi_Visibility_flags_toggled(self, _, name, alpha): - spectrum_roi = SpectrumROI(name, - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI(name, self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict[name] = spectrum_roi self.spectrum_widget.spectrum_data_dict[name] = np.array([0, 0, 0, 0]) self.spectrum_widget.set_roi_visibility_flags(name, alpha) @@ -86,12 +76,7 @@ def test_WHEN_set_roi_visibility_flags_called_THEN_roi_Visibility_flags_toggled( @parameterized.expand([("Visible", "visible_roi", 1), ("Invisible", "invisible_roi", 0)]) def test_WHEN_set_roi_alpha_called_THEN_roi_alpha_updated(self, _, name, alpha): - spectrum_roi = SpectrumROI(name, - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI(name, self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict[name] = spectrum_roi self.spectrum_widget.spectrum_data_dict[name] = np.array([0, 0, 0, 0]) self.spectrum_widget.set_roi_alpha(name, alpha) @@ -100,12 +85,7 @@ def test_WHEN_set_roi_alpha_called_THEN_roi_alpha_updated(self, _, name, alpha): @parameterized.expand([("Visible", "visible_roi", 1), ("Invisible", "invisible_roi", 0)]) def test_WHEN_set_roi_alpha_called_THEN_set_roi_visibility_flags_called(self, _, name, alpha): - spectrum_roi = SpectrumROI(name, - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI(name, self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict[name] = spectrum_roi self.spectrum_widget.spectrum_data_dict[name] = np.array([0, 0, 0, 0]) with mock.patch.object(SpectrumWidget, "set_roi_visibility_flags") as mock_set_roi_visibility_flags: @@ -120,12 +100,7 @@ def test_WHEN_add_range_called_THEN_region_and_label_set_correctly(self, _, rang f"ToF range: {range_min:.3f} - {range_max:.3f}") def test_WHEN_get_roi_called_THEN_SensibleROI_returned(self): - spectrum_roi = SpectrumROI("roi", - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI("roi", self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict["roi"] = spectrum_roi roi = self.spectrum_widget.get_roi("roi") self.assertIsInstance(type(roi), type(SensibleROI)) @@ -136,12 +111,7 @@ def test_WHEN_get_roi_called_with_invalid_roi_name_THEN_raise_key_error(self): self.spectrum_widget.get_roi("invalid_roi") def test_WHEN_remove_roi_called_THEN_roi_removed_from_roi_dict(self): - spectrum_roi = SpectrumROI("roi_1", - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI("roi_1", self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict["roi_1"] = spectrum_roi self.assertIn("roi_1", self.spectrum_widget.roi_dict) self.spectrum_widget.remove_roi("roi_1") @@ -153,12 +123,7 @@ def test_WHEN_set_tof_range_called_THEN_range_control_set_correctly(self): self.assertEqual(self.spectrum_plot_widget._tof_range_label.text, "ToF range: 0.000 - 100.000") def test_WHEN_rename_roi_called_THEN_roi_renamed(self): - spectrum_roi = SpectrumROI("roi_1", - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI("roi_1", self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict["roi_1"] = spectrum_roi self.spectrum_widget.spectrum_data_dict["roi_1"] = np.array([0, 0, 0, 0]) self.assertIn("roi_1", self.spectrum_widget.roi_dict.keys()) @@ -167,12 +132,7 @@ def test_WHEN_rename_roi_called_THEN_roi_renamed(self): self.assertIn("roi_2", self.spectrum_widget.roi_dict) def test_WHEN_rename_roi_called_with_default_roi_THEN_roi_name_not_changed(self): - spectrum_roi = SpectrumROI("roi_1", - self.sensible_roi, - pos=(0, 0), - rotatable=False, - scaleSnap=True, - translateSnap=True) + spectrum_roi = SpectrumROI("roi_1", self.sensible_roi, rotatable=False, scaleSnap=True, translateSnap=True) self.spectrum_widget.roi_dict["roi"] = spectrum_roi self.spectrum_widget.roi_dict["roi_1"] = spectrum_roi self.spectrum_widget.spectrum_data_dict["roi_1"] = np.array([0, 0, 0, 0]) From d0f137a5ef91f577c5d0b0d5c4b47cbca22382d6 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Thu, 6 Jun 2024 16:07:42 +0100 Subject: [PATCH 3/3] Add tests for SpectrumViewerWindowPresenter.handle_roi_clicked() Co-authored-by: JackEAllen Co-authored-by: Mike Sullivan --- .../gui/windows/spectrum_viewer/presenter.py | 3 ++- .../spectrum_viewer/test/presenter_test.py | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/presenter.py b/mantidimaging/gui/windows/spectrum_viewer/presenter.py index a082c0f7a7c..b5b1d20d8a2 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/presenter.py +++ b/mantidimaging/gui/windows/spectrum_viewer/presenter.py @@ -20,6 +20,7 @@ if TYPE_CHECKING: from mantidimaging.gui.windows.spectrum_viewer.view import SpectrumViewerWindowView # pragma: no cover from mantidimaging.gui.windows.main.view import MainWindowView # pragma: no cover + from mantidimaging.gui.windows.spectrum_viewer.spectrum_widget import SpectrumROI from mantidimaging.core.data import ImageStack from uuid import UUID @@ -191,7 +192,7 @@ def handle_roi_moved(self, force_new_spectrums: bool = False) -> None: self.model.set_roi(name, roi) self.view.set_spectrum(name, self.model.get_spectrum(name, self.spectrum_mode)) - def handle_roi_clicked(self, roi) -> None: + def handle_roi_clicked(self, roi: SpectrumROI) -> None: if not roi.name == ROI_RITS: self.view.current_roi = roi.name self.view.last_clicked_roi = roi.name diff --git a/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py b/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py index 7af33b70aca..5f55c710a59 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py +++ b/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py @@ -10,10 +10,11 @@ from parameterized import parameterized from mantidimaging.core.data.dataset import StrictDataset, MixedDataset +from mantidimaging.core.utility.sensible_roi import SensibleROI from mantidimaging.gui.windows.main import MainWindowView from mantidimaging.gui.windows.spectrum_viewer import SpectrumViewerWindowView, SpectrumViewerWindowPresenter -from mantidimaging.gui.windows.spectrum_viewer.model import ErrorMode, ToFUnitMode -from mantidimaging.gui.windows.spectrum_viewer.spectrum_widget import SpectrumWidget, SpectrumPlotWidget +from mantidimaging.gui.windows.spectrum_viewer.model import ErrorMode, ToFUnitMode, ROI_RITS +from mantidimaging.gui.windows.spectrum_viewer.spectrum_widget import SpectrumWidget, SpectrumPlotWidget, SpectrumROI from mantidimaging.test_helpers import mock_versions, start_qapplication from mantidimaging.test_helpers.unit_test_helper import generate_images @@ -240,6 +241,21 @@ def test_WHEN_do_remove_roi_called_THEN_roi_removed(self): self.presenter.do_remove_roi("roi_1") self.assertEqual(["all", "roi"], self.presenter.model.get_list_of_roi_names()) + def test_WHEN_roi_clicked_THEN_roi_updated(self): + roi = SpectrumROI("themightyroi", SensibleROI()) + self.presenter.handle_roi_clicked(roi) + self.assertEqual(self.view.current_roi, "themightyroi") + self.assertEqual(self.view.last_clicked_roi, "themightyroi") + self.view.set_roi_properties.assert_called_once() + + def test_WHEN_rits_roi_clicked_THEN_rois_not_updated(self): + self.view.current_roi = self.view.last_clicked_roi = "NOT_RITS_ROI" + roi = SpectrumROI(ROI_RITS, SensibleROI()) + self.presenter.handle_roi_clicked(roi) + self.assertEqual(self.view.current_roi, "NOT_RITS_ROI") + self.assertEqual(self.view.last_clicked_roi, "NOT_RITS_ROI") + self.view.set_roi_properties.assert_not_called() + def test_WHEN_ROI_renamed_THEN_roi_renamed(self): self.presenter.model.set_stack(generate_images()) self.presenter.do_add_roi()