address reviewer comments

reflectivity_ui/interfaces/data_handling/data_set.py

@@ -187,6 +187,19 @@ def calculate_reflectivity(self, direct_beam=None, configuration=None, ws_suffix
             The configuration
         ws_suffix: str
             String to add to reflectivity workspace name
+
+        Example
+        -------
+        `ws_suffix` is used when reducing multiple ROIs for the same run and cross-section, to
+        differentiate the workspace names in the Mantid data service
+
+        peak_index = 2
+        # update the active reduction list
+        data_manager.set_active_reduction_list_index(peak_index)
+        # get the first data set of the active reduction list
+        nexus_data = data_manager.reduction_list[0]
+        # calculate the reflectivity for this data set
+        nexus_data.calculate_reflectivity(ws_suffix=str(peak_index))
         """
         if configuration is not None:
             self.configuration = copy.deepcopy(configuration)

reflectivity_ui/interfaces/data_manager.py

@@ -32,9 +32,11 @@ def __init__(self, current_directory):
         # Cache of loaded data: list of NexusData instances
         self._cache = list()  # type: List[NexusData]
 
-        # The following is information about the data to be combined together
-        # List of data sets
+        # Current data tab (ROI)
         self.active_reduction_list_index = 1
+        # Main data structure holding the reduction list for each ROI/peak
+        #    key: reduction list index, corresponds to the reduction table tab in the UI
+        #    value: list of NexusData
         self.peak_reduction_lists = {self.active_reduction_list_index: []}  # type: dict[int, list[NexusData]]
         self.direct_beam_list = []  # type: List[NexusData]
         # List of cross-sections common to all reduced data sets
@@ -66,14 +68,38 @@ def current_file(self):
 
     @property
     def reduction_list(self):
+        """
+        Returns the reduction list for the active data tab
+
+        Returns
+        -------
+        list[NexusData]
+            The reduction list
+        """
         return self.peak_reduction_lists[self.active_reduction_list_index]
 
     @reduction_list.setter
     def reduction_list(self, value):
+        """
+        Sets the reduction list for the active data tab
+
+        Parameters
+        ----------
+        value: list[NexusData]
+            The reduction list
+        """
         self.peak_reduction_lists[self.active_reduction_list_index] = value
 
     @property
     def main_reduction_list(self):
+        """
+        Returns the reduction list for the first (mandatory) data tab
+
+        Returns
+        -------
+        list[NexusData]
+            The reduction list
+        """
         return self.peak_reduction_lists[self.MAIN_REDUCTION_LIST_INDEX]
 
     def get_cachesize(self):
@@ -151,36 +177,13 @@ def is_active(self, data_set):
         """
         return data_set == self._nexus_data
 
-    def is_active_data_compatible(self):
-        r"""
-        @brief Determine if the currently active data set is compatible with the data sets in the reduction list.
-        """
-        # If we are starting a new reduction list, just proceed
-        if self.reduction_list == []:
-            return True
-
-        # First, check that we have the same number of states
-        if not len(self.reduction_states) == len(self.data_sets.keys()):
-            logging.error(
-                "Active data cross-sections ({}) different than those of the"
-                " reduction list ({})".format(self.reduction_states, self.data_sets.keys())
-            )
-            return False
-
-        # Second, make sure the states match
-        for cross_section_state in self.data_sets.keys():
-            if cross_section_state not in self.reduction_states:
-                logging.error(
-                    "Active data cross-section {} not found in those"
-                    " of the reduction list".format(cross_section_state)
-                )
-                return False
-        return True
-
     def is_nexus_data_compatible(self, nexus_data: NexusData, reduction_list: list):
         """
         Determine if the data set is compatible with the data sets in the reduction list.
 
+        A data set is compatible if the polarization cross-section states matches those of the
+        first run in the reduction list, both the same number of states and the same states.
+
         Parameters
         ----------
         nexus_data: NexusData
@@ -295,7 +298,7 @@ def add_active_to_reduction(self, peak_index=MAIN_REDUCTION_LIST_INDEX):
         reduct_list = self.peak_reduction_lists[peak_index]
 
         if self._nexus_data not in reduct_list:
-            if self.is_active_data_compatible():
+            if self.is_nexus_data_compatible(self._nexus_data, reduct_list):
                 if len(reduct_list) == 0:
                     self.reduction_states = list(self.data_sets.keys())
                 is_inserted = False
@@ -870,7 +873,7 @@ def load_data_from_reduced_file(self, file_path, configuration=None, progress=No
         logging.info("DONE: %s sec", time.time() - t_0)
 
     def load_direct_beam_and_data_files(
-        self, db_files, data_files, additional_peaks, configuration=None, progress=None, force=False, t_0=None
+        self, db_files, data_files, additional_peaks=None, configuration=None, progress=None, force=False, t_0=None
     ):
         """
         Load direct beam and data files and add them to the direct beam list and reduction
@@ -882,7 +885,7 @@ def load_direct_beam_and_data_files(
             List of (run_number, run_file, conf) for direct beam files
         data_files: list
             List of (run_number, run_file, conf) for data files
-        additional_peaks: list
+        additional_peaks: list | None
             List of (peak_index, run_number, run_file, conf) for data files for additional peaks
         configuration: Configuration
             Configuration to base the loaded data on
@@ -939,18 +942,19 @@ def load_direct_beam_and_data_files(
         if progress:
             progress.set_value(n_total, message="Done", out_of=n_total)
 
-        # Initialize any additional peak reduction lists
-        for peak_index, r_id, run_file, conf in additional_peaks:
-            if peak_index not in self.peak_reduction_lists:
-                self.peak_reduction_lists[peak_index] = []
-            self.set_active_reduction_list_index(peak_index)
-            # find run in main reduction list and make a copy TODO: what if it is missing?
-            run_index = [i for i, data in enumerate(self.main_reduction_list) if data.number == str(r_id)][0]
-            self._nexus_data = copy.deepcopy(self.main_reduction_list[run_index])
-            configuration.normalization = None
-            self.update_configuration(conf)
-            self.calculate_reflectivity()
-            self.add_active_to_reduction(peak_index)
+        # Initialize any additional peak reduction lists by copying the data from the main reduction list
+        if additional_peaks:
+            for peak_index, r_id, run_file, conf in additional_peaks:
+                if peak_index not in self.peak_reduction_lists:
+                    self.peak_reduction_lists[peak_index] = []
+                self.set_active_reduction_list_index(peak_index)
+                # find run in main reduction list and make a copy TODO: what if it is missing?
+                run_index = [i for i, data in enumerate(self.main_reduction_list) if data.number == str(r_id)][0]
+                self._nexus_data = copy.deepcopy(self.main_reduction_list[run_index])
+                configuration.normalization = None
+                self.update_configuration(conf)
+                self.calculate_reflectivity()
+                self.add_active_to_reduction(peak_index)
 
     @property
     def current_event_files(self):

reflectivity_ui/interfaces/event_handlers/main_handler.py

@@ -35,7 +35,9 @@ class MainHandler(object):
     Event handler for the main application window.
     """
 
+    # Index of the direct beam tab in the reduction table tab widget
     DIRECT_BEAM_TAB_INDEX = 0
+    # Index of the first (and always visible) data tab in the reduction table tab widget
     MAIN_DATA_TAB_INDEX = 1
 
     def __init__(self, main_window):
@@ -68,6 +70,13 @@ def __init__(self, main_window):
 
     @property
     def reduction_table(self):
+        """
+        Returns the active reduction table widget if one of the data tabs is active, else the first one
+
+        Returns
+        -------
+        QTableWidget
+        """
         if self.ui.tabWidget.currentIndex == self.DIRECT_BEAM_TAB_INDEX:
             # get the table for the main data tab
             current_table = self.ui.tabWidget.widget(self.MAIN_DATA_TAB_INDEX).findChild(QtWidgets.QTableWidget)
@@ -79,10 +88,6 @@ def get_reduction_table_by_index(self, tab_index: int) -> QtWidgets.QTableWidget
         """Return the QTableWidget for the data tab with the given index"""
         return self.ui.tabWidget.widget(tab_index).findChild(QtWidgets.QTableWidget)
 
-    def get_active_reduction_table(self):
-        """Return the QTableWidget for the active data tab"""
-        return self.ui.tabWidget.currentWidget().findChild(QtWidgets.QTableWidget)
-
     def new_progress_reporter(self):
         """Return a progress reporter"""
         return ProgressReporter(progress_bar=self.progress_bar, status_bar=self.status_bar_handler)
@@ -271,7 +276,7 @@ def update_tables(self):
         # Update the reduction table if this data set is in it
         idx = self._data_manager.find_active_data_id()
         if idx is not None:
-            table_widget = self.get_active_reduction_table()
+            table_widget = self.reduction_table
             self.update_reduction_table(table_widget, idx, self._data_manager.active_channel)
 
         # Update the direct beam table if this data set is in it
@@ -1044,7 +1049,7 @@ def reduction_table_right_click(self, pos, is_reduction_table=True):
         :param bool is_reduction_table: True if the reduction table is active, False if the direct beam table is active
         """
         if is_reduction_table:
-            table_widget = self.get_active_reduction_table()
+            table_widget = self.reduction_table
             data_table = self._data_manager.reduction_list
         else:
             table_widget = self.ui.normalizeTable

reflectivity_ui/interfaces/main_window.py

@@ -28,6 +28,8 @@
 from enum import Enum
 
 
+# The two modes of the button to add/remove data tabs for additional ROIs/peaks
+# the button is in "add" mode until it reaches the max number of tabs and then switches to "remove"
 DataTabButtonMode = Enum("DataTabButtonMode", ["ADD", "REMOVE"])