Merge pull request #106 from neutrons/skip_auto_peak_finder

Skip auto peak finder when reloading the same run in the reduction table

RefRed/calculations/check_list_run_compatibility_thread.py

@@ -107,10 +107,26 @@ def update_lconfigdataset(self):
         self.parent.big_table_data = big_table_data
 
     def loading_lr_data(self):
+        """Updates data table with the new run."""
         wks = self.wks
         lrdata = LRData(wks, parent=self.parent)
         self.lrdata = lrdata
         big_table_data = self.parent.big_table_data
         col_index = 0 if self.is_working_with_data_column else 1
+        # keep existing user config if the new run number is the same as the old one
+        if self.is_reloading_same_run():
+            lrdata.peak = big_table_data[self.row, col_index].peak
+            lrdata.back = big_table_data[self.row, col_index].back
+            lrdata.low_res = big_table_data[self.row, col_index].low_res
         big_table_data[self.row, col_index] = lrdata
         self.parent.big_table_data = big_table_data
+
+    def is_reloading_same_run(self):
+        """Checks if the new run number is the same as the old, i.e. the run is being reloaded."""
+        col_index = 0 if self.is_working_with_data_column else 1
+        big_table_data_run = self.parent.big_table_data[self.row, col_index]
+        if big_table_data_run is not None and (
+            big_table_data_run.run_number == self.parent.ui.reductionTable.item(self.row, self.col).text()
+        ):
+            return True
+        return False

RefRed/main.py

@@ -52,7 +52,7 @@
 class MainGui(QtWidgets.QMainWindow):
     '''Top class that handles the GUI'''
 
-    file_loaded_signal = QtCore.Signal()
+    file_loaded_signal = QtCore.Signal(int, bool, bool)
 
     # default location
     path_ascii = '.'  # ascii file such as scaling factor file

RefRed/reduction_table_handling/reduction_table_check_box.py

@@ -57,6 +57,9 @@ def update_gui(self):
         if self.is_row_selected_checked(_row_selected):
             _big_table_data = self.parent.big_table_data
             _lconfig = _big_table_data[_row_selected, 2]
+            if not _lconfig:
+                # no data loaded on this row
+                return
             if bool(_lconfig.tof_auto_flag):
                 self.parent.ui.dataTOFautoMode.setChecked(True)
             else:

RefRed/reduction_table_handling/update_reduction_table.py

@@ -19,16 +19,16 @@ def __init__(self, parent=None, row=0, col=1, runs=None):
         self.row = row
         self.col = col
 
+        data_type = 'data' if col == 1 else 'norm'
+        self.is_data_displayed = True if (col == 1) else False
+
         item = self.parent.ui.reductionTable.item(row, col)
         if item.text() == '':
             self.clear_cell(row, col)
-            self.parent.file_loaded_signal.emit()
+            self.parent.file_loaded_signal.emit(row, self.is_data_displayed, self.display_of_this_row_checked())
             QApplication.processEvents()
             return
 
-        data_type = 'data' if col == 1 else 'norm'
-        self.is_data_displayed = True if (col == 1) else False
-
         self.raw_runs = str(runs)
         run_breaker = RunSequenceBreaker(run_sequence=self.raw_runs)
         list_run = run_breaker.final_list
@@ -85,6 +85,9 @@ def clear_cell(self, row, col):
         # Note: there is a Mantid process cleaning process elsewhere in the code
         # so we don't have to deal with it here.
         config = self.parent.big_table_data[self.row, 2]
+        if config is None:
+            # no data loaded in this row
+            return
         if col == 1:
             self.parent.big_table_data[self.row, 0] = None
             config.clear_data()

development.yml

@@ -1,12 +1,13 @@
 name: refred
 channels:
-  - mantid
+  - mantid/label/main
   - conda-forge
   - defaults
 dependencies:
-  - mantid==6.4
+  - python=3.8
+  - mantidworkbench>=6.7.0
   - qt>=5.12.9,<6
-  - pyqt>=5.12.3,<5.15
+  - pyqt>=5.12.3
   - qtpy>=1.9.0
   - pip
   - configobj

test/conftest.py

@@ -113,3 +113,21 @@ def handler():
         return window
 
     return _main_gui
+
+
+@pytest.fixture
+def file_finder_find_runs(data_server):
+    """Fixture to add as side effect for mock of FileFinder.findRuns to locate runs in the test data directory
+
+    Usage:
+    @mock.patch("RefRed.calculations.locate_list_run.FileFinder.findRuns")
+    def test_update_reduction_table(mock_file_finder_find_runs, file_finder_find_runs):
+        mock_file_finder_find_runs.side_effect = file_finder_find_runs
+    :return function: function to add as side effect to mock of FileFinder.findRuns
+    """
+
+    def _file_finder_find_runs(file_hint: str):
+        """Get path to file in test data directory"""
+        return data_server.path_to(f"{file_hint}.nxs.h5")
+
+    return _file_finder_find_runs

test/unit/RefRed/reduction_table_handling/test_update_reduction_table.py

@@ -2,84 +2,113 @@
 
 # third party packages
 import pytest
-from qtpy.QtCore import Qt
 import unittest.mock as mock
 
-wait = 200
+wait = 1000
 
 
-class Event(object):
-    val = None
-
-    def __init__(self, val=None):
-        self.val = val
-
-    def key(self):
-        return self.val
-
-
-class MockLocateListRun(object):
-    """Mock return value for LocateListRun"""
-
-    list_run = []
-    list_nexus_found = ['/SNS/REF_L/IPTS-26776/nexus/REF_L_184975.nxs.h5']
-    list_run_found = [184975]
-    list_run_not_found = []
+def load_run_from_reduction_table(window_main, row: int, col: int, run: str):
+    """Add run number in reduction table cell and press Enter to load a run"""
+    window_main.ui.reductionTable.setCurrentCell(row, col)
+    window_main.ui.reductionTable.currentItem().setText(run)
+    window_main.ui.table_reduction_cell_enter_pressed()
 
 
 @pytest.mark.parametrize("is_display_checked", [True, False])
-@mock.patch(
-    "RefRed.calculations.check_list_run_compatibility_thread.CheckListRunCompatibilityThread.updating_reductionTable_metadata"  # noqa E501
-)
-@mock.patch("RefRed.calculations.check_list_run_compatibility_thread.CheckListRunCompatibilityThread.loading_lr_data")
-@mock.patch(
-    "RefRed.calculations.check_list_run_compatibility_thread.CheckListRunCompatibilityThread.update_lconfigdataset"
-)
-@mock.patch("RefRed.calculations.check_list_run_compatibility_thread.AddListNexus")
-@mock.patch("RefRed.main.ReductionTableCheckBox")
 @mock.patch("RefRed.main.DisplayPlots")
-@mock.patch("RefRed.reduction_table_handling.update_reduction_table.LocateListRun")
+@mock.patch("RefRed.calculations.locate_list_run.FileFinder.findRuns")
 def test_update_reduction_table_thread(
-    mock_locate_list_run,
-    mock_display_plots,
-    mock_reduction_table_checkbox,
-    mock_add_list_nexus,
-    mock_update_lconfigdataset,
-    mock_loading_lr_data,
-    mock_updating_reductionTable_metadata,
-    is_display_checked,
-    qtbot,
+    mock_file_finder_find_runs, mock_display_plots, is_display_checked, qtbot, file_finder_find_runs
 ):
-    """Test of the communication between the main thread and CheckListRunCompatibilityThread
-
-    Note: This only tests the signalling between the main thread and the thread spawned by
-    CheckListRunCompatibilityThread. The actual logic updating run data and plots is mocked.
-    """
-    mock_locate_list_run.return_value = MockLocateListRun()
-    mock_add_list_nexus.return_value = mock.Mock(ws=True)
+    """Test of the communication between the main thread and CheckListRunCompatibilityThread"""
+    mock_file_finder_find_runs.side_effect = file_finder_find_runs
 
     window_main = MainGui()
     qtbot.addWidget(window_main)
-    # set the display checkbox to the desired value
-    window_main.ui.reductionTable.cellWidget(0, 0).setChecked(is_display_checked)
-    # set a run number in the reduction table
-    window_main.ui.reductionTable.setCurrentCell(0, 1)
-    window_main.ui.reductionTable.currentItem().setText("184975")
 
-    # press Enter in run number cell to trigger update_reduction_table
-    window_main.ui.reductionTable.keyPressEvent(Event(Qt.Key_Return))
+    window_main.ui.reductionTable.cellWidget(0, 0).setChecked(is_display_checked)
+    load_run_from_reduction_table(window_main, 0, 1, "184975")
     qtbot.wait(wait)
 
-    # check mocked functions in the spawned thread
-    mock_update_lconfigdataset.assert_called_once()
-    mock_loading_lr_data.assert_called_once()
-    mock_updating_reductionTable_metadata.assert_called_once()
-
-    # check that display plots is only called if the checkbox is checked
     if is_display_checked:
         mock_display_plots.assert_called_once()
     else:
         mock_display_plots.assert_not_called()
 
     # check that the reduction table was re-enabled after the thread returned
     assert window_main.ui.reductionTable.isEnabled()
+
+
+@pytest.mark.parametrize("col", [1, 2])  # data run column, normalization run column
+@mock.patch("RefRed.calculations.locate_list_run.FileFinder.findRuns")
+def test_empty_cell_press_enter(mock_file_finder_find_runs, col, qtbot, file_finder_find_runs):
+    """Test pressing Enter in empty cell in the reduction table before and after data has been loaded"""
+    mock_file_finder_find_runs.side_effect = file_finder_find_runs
+
+    window_main = MainGui()
+    qtbot.addWidget(window_main)
+
+    # test pressing Enter in empty cell before any data has been loaded
+    load_run_from_reduction_table(window_main, 0, col, "")
+    qtbot.wait(wait)
+    # check that the reduction table was re-enabled after the function returned
+    assert window_main.ui.reductionTable.isEnabled()
+
+    # test first loading a run, then clearing the cell and pressing Enter to clear loaded data
+    load_run_from_reduction_table(window_main, 0, col, "184975")
+    qtbot.wait(wait)
+    # check that the data has been loaded
+    assert window_main.big_table_data[0, col - 1] is not None
+    # clear the cell and press Enter to clear the data
+    load_run_from_reduction_table(window_main, 0, col, "")
+    qtbot.wait(wait)
+    # check that the data has been cleared
+    assert window_main.big_table_data[0, col - 1] is None
+    assert window_main.ui.reductionTable.isEnabled()
+
+
+@mock.patch("RefRed.main.DisplayPlots")
+@mock.patch("RefRed.calculations.locate_list_run.FileFinder.findRuns")
+def test_load_run_auto_peak_finder(mock_file_finder_find_runs, mock_display_plots, qtbot, file_finder_find_runs):
+    """Test that the auto-peak finder is only enabled if a new run is loaded"""
+    mock_file_finder_find_runs.side_effect = file_finder_find_runs
+
+    window_main = MainGui()
+    qtbot.addWidget(window_main)
+
+    run1 = "188300"
+    expected_peak_run1 = ["130", "141"]
+    expected_back_run1 = ["127", "144"]
+    run2 = "188301"
+    expected_peak_run2 = ["130", "139"]
+    expected_back_run2 = ["127", "142"]
+
+    # load the first run
+    load_run_from_reduction_table(window_main, row=0, col=1, run=run1)
+    qtbot.wait(wait)
+    # check the result of the auto-peak finder
+    assert window_main.big_table_data[0, 0].peak == expected_peak_run1
+    assert window_main.big_table_data[0, 0].back == expected_back_run1
+
+    # modify the peak and background ranges (users can change these in the UI)
+    user_set_peak = ["132", "143"]
+    user_set_back = ["128", "147"]
+    window_main.big_table_data[0, 0].peak = user_set_peak
+    window_main.big_table_data[0, 0].back = user_set_back
+    # reload the same run
+    load_run_from_reduction_table(window_main, row=0, col=1, run=run1)
+    qtbot.wait(wait)
+    # check that the auto-peak finder did not change the peak and background ranges
+    assert window_main.big_table_data[0, 0].peak == user_set_peak
+    assert window_main.big_table_data[0, 0].back == user_set_back
+
+    # load a different run in the cell
+    load_run_from_reduction_table(window_main, row=0, col=1, run=run2)
+    qtbot.wait(wait)
+    # check that the peak and background ranges were updated
+    assert window_main.big_table_data[0, 0].peak == expected_peak_run2
+    assert window_main.big_table_data[0, 0].back == expected_back_run2
+
+
+if __name__ == '__main__':
+    pytest.main([__file__])