Fix issue with replacing data in reduction table (#105)

move the statement causing the crash (updating the GUI plots) from worker thread to main thread

RefRed/calculations/check_list_run_compatibility_and_display_thread.py → RefRed/calculations/check_list_run_compatibility_thread.py

@@ -10,17 +10,18 @@
 # import logging
 import RefRed.colors
 from qtpy import QtCore, QtGui
-from qtpy.QtWidgets import QApplication
+from qtpy.QtCore import Signal
 
 from RefRed.reduction_table_handling.check_list_run_compatibility import CheckListRunCompatibility
 from RefRed.calculations.add_list_nexus import AddListNexus
 from RefRed.lconfigdataset import LConfigDataset
 from RefRed.calculations.lr_data import LRData
-from RefRed.plot.display_plots import DisplayPlots
 from RefRed.calculations.update_reduction_table_metadata import UpdateReductionTableMetadata
 
 
-class CheckListRunCompatibilityAndDisplayThread(QtCore.QThread):
+class CheckListRunCompatibilityThread(QtCore.QThread):
+
+    updated_data = Signal(int, bool, bool)
 
     runs_are_compatible = False
     wks = None
@@ -72,11 +73,8 @@ def run(self):
         # if runs_are_compatible:
         self.loading_lr_data()
         self.updating_reductionTable_metadata()
-        if self.is_display_requested:
-            self.display_plots()
 
-        self.parent.file_loaded_signal.emit()
-        QApplication.processEvents()
+        self.updated_data.emit(self.row, self.is_working_with_data_column, self.is_display_requested)
 
     def updating_reductionTable_metadata(self):
         is_working_with_data_column = self.is_working_with_data_column
@@ -116,6 +114,3 @@ def loading_lr_data(self):
         col_index = 0 if self.is_working_with_data_column else 1
         big_table_data[self.row, col_index] = lrdata
         self.parent.big_table_data = big_table_data
-
-    def display_plots(self):
-        DisplayPlots(parent=self.parent, row=self.row, is_data=self.is_working_with_data_column)

RefRed/main.py

@@ -27,6 +27,7 @@
 from RefRed.load_reduced_data_set.load_reduced_data_set_handler import LoadReducedDataSetHandler
 from RefRed.load_reduced_data_set.reduced_ascii_data_right_click import ReducedAsciiDataRightClick
 from RefRed.metadata.metadata_finder import MetadataFinder
+from RefRed.plot.display_plots import DisplayPlots
 from RefRed.plot.single_click_plot import SingleClickPlot
 from RefRed.plot.home_plot_button_clicked import HomePlotButtonClicked
 from RefRed.plot.mouse_leave_plot import MouseLeavePlot
@@ -212,8 +213,10 @@ def logy_toggle_data_stitching(self, checked):
     def reduction_table_visibility_changed_test(self, state, row):
         ReductionTableCheckBox(parent=self, row_selected=row)
 
-    def file_loaded(self):
-        """Event call-back used to re-enable the reduction table after loading"""
+    def file_loaded(self, row, is_data_displayed, is_display_requested):
+        """Event call-back used to display plots and re-enable the reduction table after loading"""
+        if is_display_requested:
+            DisplayPlots(parent=self, row=row, is_data=is_data_displayed)
         self.ui.reductionTable.setEnabled(True)
 
     def table_reduction_cell_enter_pressed(self):

RefRed/reduction_table_handling/update_reduction_table.py

@@ -2,8 +2,8 @@
 from qtpy.QtWidgets import QApplication
 
 from RefRed.calculations.run_sequence_breaker import RunSequenceBreaker
-from RefRed.calculations.check_list_run_compatibility_and_display_thread import (
-    CheckListRunCompatibilityAndDisplayThread,
+from RefRed.calculations.check_list_run_compatibility_thread import (
+    CheckListRunCompatibilityThread,
 )
 import RefRed.colors
 from RefRed.calculations.locate_list_run import LocateListRun
@@ -59,7 +59,7 @@ def __init__(self, parent=None, row=0, col=1, runs=None):
 
         list_nexus_found = list_run_object.list_nexus_found
         thread_index = (self.col - 1) + 2 * self.row
-        self.parent.loading_nxs_thread[thread_index] = CheckListRunCompatibilityAndDisplayThread()
+        self.parent.loading_nxs_thread[thread_index] = CheckListRunCompatibilityThread()
         self.parent.loading_nxs_thread[thread_index].setup(
             parent=self.parent,
             list_run=list_run_found,
@@ -68,6 +68,7 @@ def __init__(self, parent=None, row=0, col=1, runs=None):
             is_working_with_data_column=self.is_data_displayed,
             is_display_requested=self.display_of_this_row_checked(),
         )
+        self.parent.loading_nxs_thread[thread_index].updated_data.connect(self.parent.file_loaded)
         self.parent.loading_nxs_thread[thread_index].start()
 
     def clear_cell(self, row, col):

test/unit/RefRed/reduction_table_handling/test_update_reduction_table.py

@@ -0,0 +1,85 @@
+from RefRed.main import MainGui
+
+# third party packages
+import pytest
+from qtpy.QtCore import Qt
+import unittest.mock as mock
+
+wait = 200
+
+
+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 = []
+
+
+@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")
+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,
+):
+    """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)
+
+    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))
+    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()