Refactor ROI Handling: Remove get_list_of_roi_names() and get_roi fro…

…m Model (#2423)

mantidimaging/gui/test/gui_system_spectrum_test.py

@@ -118,7 +118,7 @@ def test_adjust_roi(self):
         roi_widget.movePoint(handle_index, new_position)
         QTest.qWait(SHORT_DELAY)
 
-        updated_roi = self.spectrum_window.presenter.model.get_roi(roi_name)
+        updated_roi = self.spectrum_window.spectrum_widget.get_roi(roi_name)
         self.assertEqual(updated_roi.right, new_position[0])
         self.assertEqual(updated_roi.bottom, new_position[1])
         self.assertEqual(updated_roi.top, 0)

mantidimaging/gui/windows/spectrum_viewer/model.py

@@ -112,12 +112,6 @@ def roi_name_generator(self) -> str:
         self._roi_id_counter += 1
         return new_name
 
-    def get_list_of_roi_names(self) -> list[str]:
-        """
-        Get a list of rois available in the model
-        """
-        return list(self._roi_ranges.keys())
-
     def set_stack(self, stack: ImageStack | None) -> None:
         """
         Sets the stack to be used by the model
@@ -151,17 +145,6 @@ def set_normalise_stack(self, normalise_stack: ImageStack | None) -> None:
     def set_roi(self, roi_name: str, roi: SensibleROI) -> None:
         self._roi_ranges[roi_name] = roi
 
-    def get_roi(self, roi_name: str) -> SensibleROI:
-        """
-        Get the ROI with the given name from the model
-
-        @param roi_name: The name of the ROI to get
-        @return: The ROI with the given name
-        """
-        if roi_name not in self._roi_ranges.keys():
-            raise KeyError(f"ROI {roi_name} does not exist in roi_ranges {self._roi_ranges.keys()}")
-        return self._roi_ranges[roi_name]
-
     def get_averaged_image(self) -> np.ndarray | None:
         """
         Get the averaged image from the stack in the model returning as a numpy array
@@ -370,10 +353,9 @@ def save_single_rits_spectrum(self, path: Path, error_mode: ErrorMode) -> None:
         Saves the spectrum for the RITS ROI to a RITS file.
 
         @param path: The path to save the CSV file to.
-        @param normalized: Whether to save the normalized spectrum.
-        @param error_mode: Which version (standard deviation or propagated) of the error to use in the RITS export
+        @param error_mode: Which version (standard deviation or propagated) of the error to use in the RITS export.
         """
-        self.save_rits_roi(path, error_mode, self.get_roi(ROI_RITS))
+        self.save_rits_roi(path, error_mode, self._roi_ranges[ROI_RITS])
 
     def save_rits_roi(self, path: Path, error_mode: ErrorMode, roi: SensibleROI, normalise: bool = False) -> None:
         """
@@ -453,7 +435,7 @@ def save_rits_images(self,
         Returns:
         None
         """
-        roi = self.get_roi(ROI_RITS)
+        roi = self._roi_ranges[ROI_RITS]
         left, top, right, bottom = roi
         x_iterations = min(ceil((right - left) / step), ceil((right - left - bin_size) / step) + 1)
         y_iterations = min(ceil((bottom - top) / step), ceil((bottom - top - bin_size) / step) + 1)

mantidimaging/gui/windows/spectrum_viewer/presenter.py

@@ -200,14 +200,14 @@ def handle_roi_moved(self, force_new_spectrums: bool = False) -> None:
         """
         Handle changes to any ROI position and size.
         """
-        for name in self.model.get_list_of_roi_names():
-            view_roi = self.view.spectrum_widget.get_roi(name)
-            if force_new_spectrums or view_roi != self.model.get_roi(name):
-                self.model.set_roi(name, view_roi)
+        for name in self.view.spectrum_widget.roi_dict:
+            current_roi = self.view.spectrum_widget.get_roi(name)
+            self.model.set_roi(name, current_roi)
+            if force_new_spectrums:
                 spectrum = self.model.get_spectrum(
-                    view_roi,
+                    current_roi,
                     self.spectrum_mode,
-                    normalise_with_shuttercount=self.view.shuttercount_norm_enabled(),
+                    self.view.shuttercount_norm_enabled(),
                 )
                 self.view.set_spectrum(name, spectrum)
 
@@ -221,27 +221,20 @@ def redraw_spectrum(self, name: str) -> None:
         """
         Redraw the spectrum with the given name
         """
-        roi = self.model.get_roi(name)
-        self.view.set_spectrum(
-            name,
-            self.model.get_spectrum(roi,
-                                    self.spectrum_mode,
-                                    normalise_with_shuttercount=self.view.shuttercount_norm_enabled()))
+        roi = self.view.spectrum_widget.get_roi(name)
+        spectrum = self.model.get_spectrum(roi, self.spectrum_mode, self.view.shuttercount_norm_enabled())
+        self.view.set_spectrum(name, spectrum)
 
     def redraw_all_rois(self) -> None:
         """
         Redraw all ROIs and spectrum plots
         """
-        for name in self.model.get_list_of_roi_names():
-            if name == "all" or not self.view.spectrum_widget.roi_dict[name].isVisible():
+        for roi_name, roi_widget in self.view.spectrum_widget.roi_dict.items():
+            if not roi_widget.isVisible():
                 continue
-            roi = self.view.spectrum_widget.get_roi(name)
-            self.model.set_roi(name, roi)
-            self.view.set_spectrum(
-                name,
-                self.model.get_spectrum(roi,
-                                        self.spectrum_mode,
-                                        normalise_with_shuttercount=self.view.shuttercount_norm_enabled()))
+            widget_roi = self.view.spectrum_widget.get_roi(roi_name)
+            spectrum = self.model.get_spectrum(widget_roi, self.spectrum_mode, self.view.shuttercount_norm_enabled())
+            self.view.set_spectrum(roi_name, spectrum)
 
     def handle_button_enabled(self) -> None:
         """
@@ -325,11 +318,9 @@ def set_shuttercount_error(self, enabled: bool = False) -> None:
 
     def get_roi_names(self) -> list[str]:
         """
-        Return a list of ROI names
-
         @return: list of ROI names
         """
-        return self.model.get_list_of_roi_names()
+        return list(self.view.spectrum_widget.roi_dict.keys())
 
     def do_add_roi(self) -> None:
         """
@@ -338,9 +329,10 @@ def do_add_roi(self) -> None:
         roi_name = self.model.roi_name_generator()
         if roi_name in self.view.spectrum_widget.roi_dict:
             raise ValueError(f"ROI name already exists: {roi_name}")
-
         self.model.set_new_roi(roi_name)
-        roi = self.model.get_roi(roi_name)
+        roi = self.model._roi_ranges.get(roi_name)
+        if roi is None:
+            raise ValueError(f"ROI for {roi_name} is not valid.")
         self.view.spectrum_widget.add_roi(roi, roi_name)
         spectrum = self.model.get_spectrum(roi, self.spectrum_mode, self.view.shuttercount_norm_enabled())
         self.view.set_spectrum(roi_name, spectrum)
@@ -361,7 +353,7 @@ def change_roi_colour(self, roi_name: str, new_colour: tuple[int, int, int]) ->
 
     def add_rits_roi(self) -> None:
         self.model.set_new_roi(ROI_RITS)
-        roi = self.model.get_roi(ROI_RITS)
+        roi = self.model._roi_ranges[ROI_RITS]
         self.view.spectrum_widget.add_roi(roi, ROI_RITS)
         self.view.set_spectrum(ROI_RITS,
                                self.model.get_spectrum(roi, self.spectrum_mode, self.view.shuttercount_norm_enabled()))
@@ -390,21 +382,15 @@ def do_remove_roi(self, roi_name: str | None = None) -> None:
         """
         Remove a given ROI from the table by ROI name or all ROIs from
         the table if no name is passed as an argument
-
         @param roi_name: Name of the ROI to remove
         """
         if roi_name is None:
-            self.view.clear_all_rois()
+            self.view.spectrum_widget.roi_dict.clear()
             for name in self.get_roi_names():
                 self.view.spectrum_widget.remove_roi(name)
             self.model.remove_all_roi()
         else:
-            roi = self.model.get_roi(roi_name)
             self.view.spectrum_widget.remove_roi(roi_name)
-            spectrum = self.model.get_spectrum(roi,
-                                               self.spectrum_mode,
-                                               normalise_with_shuttercount=self.view.shuttercount_norm_enabled())
-            self.view.set_spectrum(roi_name, spectrum)
             self.model.remove_roi(roi_name)
 
     def handle_export_tab_change(self, index: int) -> None: