Merge branch 'main' into 2368_stack_properties_menu

docs/release_notes/next/fix-2486-spec-plot-update

@@ -0,0 +1 @@
+2486: Fix ROI movement not updating spectrum unless selected

mantidimaging/gui/test/gui_system_spectrum_test.py

@@ -40,36 +40,36 @@ def tearDown(self) -> None:
     def test_spectrum_window_opens_with_data_in_default_state(self) -> None:
         self.assertFalse(self.spectrum_window.normaliseCheckBox.isChecked())
         self.assertFalse(self.spectrum_window.normalise_ShutterCount_CheckBox.isEnabled())
-        self.assertEqual(self.spectrum_window.roi_table_model.rowCount(), 1)
-        self.assertEqual(self.spectrum_window.roi_table_model.columnCount(), 3)
+        self.assertEqual(self.spectrum_window.table_view.roi_table_model.rowCount(), 1)
+        self.assertEqual(self.spectrum_window.table_view.roi_table_model.columnCount(), 3)
         self.assertTrue(self.spectrum_window.addBtn.isEnabled())
         self.assertTrue(self.spectrum_window.removeBtn.isEnabled())
         self.assertTrue(self.spectrum_window.exportButton.isEnabled())
-        self.assertIn('roi', self.spectrum_window.roi_table_model.roi_names())
+        self.assertIn('roi', self.spectrum_window.table_view.roi_table_model.roi_names())
         self.assertIn('roi', self.spectrum_window.spectrum_widget.roi_dict)
         QTest.qWait(SHOW_DELAY)
 
     def test_add_roi(self) -> None:
         for i in range(1, 4):
-            initial_roi_count = self.spectrum_window.roi_table_model.rowCount()
+            initial_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount()
             QTest.mouseClick(self.spectrum_window.addBtn, Qt.MouseButton.LeftButton)
             QTest.qWait(SHORT_DELAY)
-            final_roi_count = self.spectrum_window.roi_table_model.rowCount()
+            final_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount()
             self.assertEqual(final_roi_count, initial_roi_count + 1)
-            self.assertIn(f'roi_{i}', self.spectrum_window.roi_table_model.roi_names())
+            self.assertIn(f'roi_{i}', self.spectrum_window.table_view.roi_table_model.roi_names())
             self.assertIn(f'roi_{i}', self.spectrum_window.spectrum_widget.roi_dict)
 
     def test_remove_roi(self) -> None:
         QTest.mouseClick(self.spectrum_window.addBtn, Qt.MouseButton.LeftButton)
         QTest.qWait(SHORT_DELAY)
-        initial_roi_count = self.spectrum_window.roi_table_model.rowCount()
+        initial_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount()
 
         QTest.mouseClick(self.spectrum_window.removeBtn, Qt.MouseButton.LeftButton)
         QTest.qWait(SHORT_DELAY)
-        final_roi_count = self.spectrum_window.roi_table_model.rowCount()
+        final_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount()
 
         self.assertEqual(final_roi_count, initial_roi_count - 1)
-        self.assertNotIn(f'roi_{initial_roi_count - 1}', self.spectrum_window.roi_table_model.roi_names())
+        self.assertNotIn(f'roi_{initial_roi_count - 1}', self.spectrum_window.table_view.roi_table_model.roi_names())
         self.assertNotIn(f'roi_{initial_roi_count - 1}', self.spectrum_window.spectrum_widget.roi_dict)
 
     def test_change_roi_color(self):
@@ -96,7 +96,7 @@ def test_rename_roi(self):
         old_name = 'roi_1'
         new_name = 'roi_renamed'
 
-        table_model = self.spectrum_window.roi_table_model
+        table_model = self.spectrum_window.table_view.roi_table_model
         row = table_model.roi_names().index(old_name)
 
         table_model.set_element(row, 0, new_name)

mantidimaging/gui/ui/spectrum_viewer.ui

@@ -238,7 +238,7 @@
                </property>
                <layout class="QVBoxLayout" name="verticalLayout_5">
                 <item>
-                 <widget class="RemovableRowTableView" name="tableView">
+                 <widget class="ROITableWidget" name="table_view">
                   <attribute name="horizontalHeaderHighlightSections">
                    <bool>true</bool>
                   </attribute>
@@ -545,9 +545,9 @@
    <header>mantidimaging.gui.widgets.dataset_selector</header>
   </customwidget>
   <customwidget>
-   <class>RemovableRowTableView</class>
+   <class>ROITableWidget</class>
    <extends>QTableView</extends>
-   <header>mantidimaging.gui.widgets</header>
+   <header>mantidimaging.gui.windows.spectrum_viewer.view</header>
   </customwidget>
  </customwidgets>
  <resources/>

mantidimaging/gui/windows/spectrum_viewer/presenter.py

@@ -201,24 +201,22 @@ def handle_range_slide_moved(self, tof_range: tuple[float, float] | tuple[int, i
         self.view.spectrum_widget.spectrum_plot_widget.set_tof_range_label(*self.model.tof_plot_range)
         self.update_displayed_image(autoLevels=False)
 
-    def handle_roi_moved(self, force_new_spectrums: bool = False) -> None:
+    def handle_roi_moved(self, roi: SpectrumROI) -> None:
         """
         Handle changes to any ROI position and size.
         """
-        for name in self.view.spectrum_widget.roi_dict:
-            current_roi = self.view.spectrum_widget.get_roi(name)
-            if force_new_spectrums:
-                spectrum = self.model.get_spectrum(
-                    current_roi,
-                    self.spectrum_mode,
-                    self.view.shuttercount_norm_enabled(),
-                )
-                self.view.set_spectrum(name, spectrum)
+        spectrum = self.model.get_spectrum(
+            roi.as_sensible_roi(),
+            self.spectrum_mode,
+            self.view.shuttercount_norm_enabled(),
+        )
+        self.view.set_spectrum(roi.name, spectrum)
+        self.view.spectrum_widget.spectrum.update()
 
     def handle_roi_clicked(self, roi: SpectrumROI) -> None:
         if not roi.name == ROI_RITS:
-            self.view.current_roi_name = roi.name
-            self.view.last_clicked_roi = roi.name
+            self.view.table_view.current_roi_name = roi.name
+            self.view.table_view.last_clicked_roi = roi.name
             self.view.set_roi_properties()
 
     def redraw_spectrum(self, name: str) -> None:
@@ -354,7 +352,7 @@ def change_roi_colour(self, roi_name: str, new_colour: tuple[int, int, int]) ->
         """
         if roi_name in self.view.spectrum_widget.roi_dict:
             self.view.spectrum_widget.roi_dict[roi_name].colour = new_colour
-        self.view.update_roi_color(roi_name, new_colour)
+        self.view.table_view.update_roi_color(roi_name, new_colour)
         self.view.on_visibility_change()
 
     def add_rits_roi(self) -> None:
@@ -393,10 +391,10 @@ def do_remove_roi(self, roi_name: str | None = None) -> None:
         @param roi_name: Name of the ROI to remove
         """
         if roi_name is None:
-            for name in self.get_roi_names():
+            for name in list(self.get_roi_names()):
                 self.view.spectrum_widget.remove_roi(name)
             self.view.spectrum_widget.roi_dict.clear()
-            self.view.roi_table_model.clear_table()
+            self.view.table_view.roi_table_model.clear_table()
             self.model.remove_all_roi()
         else:
             self.view.spectrum_widget.remove_roi(roi_name)
@@ -439,21 +437,21 @@ def change_selected_menu_option(self, opt: str) -> None:
 
     def do_adjust_roi(self) -> None:
         new_roi = self.view.roi_properties_widget.as_roi()
-        self.view.spectrum_widget.adjust_roi(new_roi, self.view.current_roi_name)
+        self.view.spectrum_widget.adjust_roi(new_roi, self.view.table_view.current_roi_name)
 
     def handle_storing_current_roi_name_on_tab_change(self) -> None:
-        old_table_names = self.view.old_table_names
-        old_current_roi_name = self.view.current_roi_name
-        old_last_clicked_roi = self.view.last_clicked_roi
+        old_table_names = self.view.table_view.old_table_names
+        old_current_roi_name = self.view.table_view.current_roi_name
+        old_last_clicked_roi = self.view.table_view.last_clicked_roi
         if self.export_mode == ExportMode.ROI_MODE:
             if old_current_roi_name == ROI_RITS and old_last_clicked_roi in old_table_names:
-                self.view.current_roi_name = old_last_clicked_roi
+                self.view.table_view.current_roi_name = old_last_clicked_roi
             else:
-                self.view.last_clicked_roi = old_current_roi_name
+                self.view.table_view.last_clicked_roi = old_current_roi_name
         elif self.export_mode == ExportMode.IMAGE_MODE:
             if (old_current_roi_name != ROI_RITS and old_current_roi_name in old_table_names
                     and old_last_clicked_roi != old_current_roi_name):
-                self.view.last_clicked_roi = old_current_roi_name
+                self.view.table_view.last_clicked_roi = old_current_roi_name
 
     @staticmethod
     def check_action(action: QAction, param: bool) -> None:

mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py

@@ -138,7 +138,7 @@ class SpectrumWidget(QWidget):
 
     range_changed = pyqtSignal(object)
     roi_clicked = pyqtSignal(object)
-    roi_changed = pyqtSignal()
+    roi_changed = pyqtSignal(object)
     roiColorChangeRequested = pyqtSignal(str, tuple)
 
     spectrum_plot_widget: SpectrumPlotWidget
@@ -211,9 +211,9 @@ def add_roi(self, roi: SensibleROI, name: str) -> None:
         roi_object.colour = self.colour_generator()
         roi_object.sig_colour_change.connect(lambda name, color: self.roiColorChangeRequested.emit(name, color))
 
-        self.roi_dict[name] = roi_object.roi
+        self.roi_dict[name] = roi_object
         self.max_roi_size = roi_object.size()
-        self.roi_dict[name].sigRegionChangeFinished.connect(self.roi_changed.emit)
+        self.roi_dict[name].sigRegionChangeFinished.connect(self._emit_roi_changed)
         self.roi_dict[name].sigClicked.connect(self.roi_clicked.emit)
         self.image.vb.addItem(self.roi_dict[name])
         self.roi_dict[name].hoverPen = mkPen(self.roi_dict[name].colour, width=3)
@@ -277,6 +277,10 @@ def rename_roi(self, old_name: str, new_name: str) -> None:
         self.spectrum_data_dict[new_name] = self.spectrum_data_dict.pop(old_name)
         self.roi_dict[new_name].rename_roi(new_name)
 
+    def _emit_roi_changed(self):
+        sender_roi = self.sender()
+        self.roi_changed.emit(sender_roi)
+
 
 class CustomViewBox(ViewBox):
 

mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py

@@ -7,9 +7,12 @@
 from unittest import mock
 
 import numpy as np
+
+from PyQt5.QtCore import QAbstractTableModel
 from PyQt5.QtWidgets import QPushButton, QActionGroup, QGroupBox, QAction, QCheckBox, QTabWidget, QWidget, QSpinBox
 from parameterized import parameterized
 
+from mantidimaging.gui.widgets import RemovableRowTableView
 from mantidimaging.core.data.dataset import Dataset
 from mantidimaging.core.utility.sensible_roi import SensibleROI
 from mantidimaging.gui.windows.main import MainWindowView
@@ -38,6 +41,9 @@ def setUp(self) -> None:
             "Left": mock.create_autospec(QSpinBox, instance=True),
             "Right": mock.create_autospec(QSpinBox, instance=True)
         }
+        self.view.table_view = mock.create_autospec(RemovableRowTableView, instance=True)
+        self.view.table_view.roi_table_model = mock.create_autospec(QAbstractTableModel, instance=True)
+        self.view.table_view.roi_table_model.clear_table = mock.Mock()
         mock_spectrum_roi_dict = mock.create_autospec(dict, instance=True)
         self.view.spectrum_widget = mock.create_autospec(SpectrumWidget, roi_dict=mock_spectrum_roi_dict, instance=True)
         self.view.spectrum_widget.spectrum_plot_widget = mock.create_autospec(SpectrumPlotWidget,
@@ -278,16 +284,15 @@ def test_WHEN_do_remove_roi_called_THEN_roi_removed(self):
     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_name, "themightyroi")
-        self.assertEqual(self.view.last_clicked_roi, "themightyroi")
+        self.assertEqual(self.view.table_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_name = self.view.last_clicked_roi = "NOT_RITS_ROI"
+        self.view.table_view.current_roi_name = self.view.table_view.last_clicked_roi = "NOT_RITS_ROI"
         roi = SpectrumROI(ROI_RITS, SensibleROI())
         self.presenter.handle_roi_clicked(roi)
-        self.assertEqual(self.view.current_roi_name, "NOT_RITS_ROI")
-        self.assertEqual(self.view.last_clicked_roi, "NOT_RITS_ROI")
+        self.assertEqual(self.view.table_view.current_roi_name, "NOT_RITS_ROI")
+        self.assertEqual(self.view.table_view.last_clicked_roi, "NOT_RITS_ROI")
         self.view.set_roi_properties.assert_not_called()
 
     def test_WHEN_ROI_renamed_THEN_roi_renamed(self):
@@ -395,7 +400,7 @@ def test_WHEN_menu_option_selected_THEN_menu_option_changed(self):
 
     def test_WHEN_roi_changed_via_spinboxes_THEN_roi_adjusted(self):
         self.view.roi_properties_widget.as_roi = mock.Mock(return_value=SensibleROI(10, 10, 20, 30))
-        self.view.current_roi_name = "roi_1"
+        self.view.table_view.current_roi_name = "roi_1"
         self.presenter.do_adjust_roi()
         self.view.spectrum_widget.adjust_roi.assert_called_once_with(SensibleROI(10, 10, 20, 30), "roi_1")
 
@@ -404,13 +409,13 @@ def test_WHEN_roi_changed_via_spinboxes_THEN_roi_adjusted(self):
                            (["roi_1", "roi_2", "roi_3"], ROI_RITS, "roi_2", ExportMode.ROI_MODE, "roi_2")])
     def test_WHEN_change_tab_THEN_current_roi_correct(self, old_table_names, current_roi_name, last_clicked_roi,
                                                       export_mode, expected_roi):
-        self.view.old_table_names = old_table_names
-        self.view.current_roi_name = current_roi_name
-        self.view.last_clicked_roi = last_clicked_roi
+        self.view.table_view.old_table_names = old_table_names
+        self.view.table_view.current_roi_name = current_roi_name
+        self.view.table_view.last_clicked_roi = last_clicked_roi
         self.presenter.export_mode = export_mode
         self.presenter.handle_storing_current_roi_name_on_tab_change()
-        self.assertEqual(self.view.current_roi_name, expected_roi)
-        self.assertEqual(self.view.last_clicked_roi, expected_roi)
+        self.assertEqual(self.view.table_view.current_roi_name, expected_roi)
+        self.assertEqual(self.view.table_view.last_clicked_roi, expected_roi)
 
     def test_WHEN_refresh_spectrum_plot_THEN_spectrum_plot_refreshed(self):
         self.view.spectrum_widget.spectrum = mock.MagicMock()
@@ -436,15 +441,15 @@ def test_WHEN_redraw_all_rois_THEN_rois_set_correctly(self):
     @parameterized.expand([("roi", "roi_clicked", "roi_clicked"), ("roi", ROI_RITS, "roi")])
     def test_WHEN_roi_clicked_THEN_current_and_last_clicked_roi_updated_correctly(self, old_roi, clicked_roi,
                                                                                   expected_roi):
-        self.view.current_roi_name = old_roi
-        self.view.last_clicked_roi = old_roi
+        self.view.table_view.current_roi_name = old_roi
+        self.view.table_view.last_clicked_roi = old_roi
         self.presenter.handle_roi_clicked(SpectrumROI(clicked_roi, SensibleROI(), pos=(0, 0)))
-        self.assertEqual(self.view.current_roi_name, expected_roi)
-        self.assertEqual(self.view.last_clicked_roi, expected_roi)
+        self.assertEqual(self.view.table_view.current_roi_name, expected_roi)
+        self.assertEqual(self.view.table_view.last_clicked_roi, expected_roi)
 
     def test_WHEN_roi_clicked_THEN_roi_properties_set(self):
-        self.view.current_roi_name = ""
-        self.view.last_clicked_roi = ""
+        self.view.table_view.current_roi_name = ""
+        self.view.table_view.last_clicked_roi = ""
         self.presenter.handle_roi_clicked(SpectrumROI("roi_clicked", SensibleROI(), pos=(0, 0)))
         self.view.set_roi_properties.assert_called_once()