Merge pull request #28440 from mantidproject/28275_workbench_plot_dialog_validation

Workbench plot dialog validation

docs/source/release/v5.1.0/mantidworkbench.rst

@@ -8,6 +8,10 @@ MantidWorkbench Changes
 Improvements
 ############
 
+.. figure:: ../../images/Plot1DSelectionDialog5-1.png
+   :align: right
+
+- The plot selection dialog now correctly shows the full range of valid spectra to plot, not just the min to max range.
 - Tile plots are now reloaded correctly by project recovery.
 - Fixed an issue where some scripts were running slower if a  plot was open at the same time.
 
@@ -15,7 +19,8 @@ Improvements
 Bugfixes
 ########
 
-- Fixed a bug where setting columns to Y error in table workspaces wasn't working. The links between the Y error and Y columns weren't being set up properly
+- Fixed a bug where setting columns to Y error in table workspaces wasn't working. The links between the Y error and Y columns weren't being set up properly.
+- Fixed a crash when you selected a spectra to plot that was not present in a workspace.
 - The scale of the color bars on colorfill plots of ragged workspaces now uses the maximum and minimum values of the data.
 
 :ref:`Release 5.1.0 <v5.1.0>`

qt/applications/workbench/workbench/plugins/workspacewidget.py

@@ -90,8 +90,10 @@ class WorkspaceWidget(PluginWidget):
             if not compatible:
                 QMessageBox.warning(self, "", error_msg)
                 return
-
-        plot_from_names(names, errors, overplot)
+        try:
+            plot_from_names(names, errors, overplot)
+        except RuntimeError as re:
+            logger.error(str(re))
 
     def _do_plot_bin(self, names, errors, overplot):
         """

qt/python/mantidqt/dialogs/spectraselectordialog.py

@@ -20,6 +20,7 @@ from mantidqt.utils.qt import load_ui
 # Constants
 RANGE_SPECIFIER = '-'
 PLACEHOLDER_FORMAT = 'valid range: {}' + RANGE_SPECIFIER + '{}'
+PLACEHOLDER_FORMAT_SINGLE_INPUT = 'valid range: {}'
 RED_ASTERISK = None
 
 
@@ -67,12 +68,12 @@ class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
 
         # attributes
         self._workspaces = workspaces
-        self.spec_min, self.spec_max = None, None
         self.wi_min, self.wi_max = None, None
         self.selection = None
         self._ui = None
         self._show_colorfill_button = show_colorfill_btn
         self._overplot = overplot
+        self._plottable_spectra = None
 
         self._init_ui()
         self._set_placeholder_text()
@@ -97,7 +98,6 @@ class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
         self.accept()
 
     # ------------------- Private -------------------------
-
     def _check_number_of_plots(self, selection):
         index_length = len(selection.wksp_indices) if selection.wksp_indices else len(selection.spectra)
         number_of_lines_to_plot = len(selection.workspaces) * index_length
@@ -145,9 +145,24 @@ class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
         plottable = sorted(plottable)
         if len(plottable) == 0:
             raise Exception('Error: Workspaces have no common spectra.')
-        spec_min, spec_max = min(plottable), max(plottable)
-        self._ui.specNums.setPlaceholderText(PLACEHOLDER_FORMAT.format(spec_min, spec_max))
-        self.spec_min, self.spec_max = spec_min, spec_max
+        #store the plottable list for use later
+        self._plottable_spectra = plottable
+        self._ui.specNums.setPlaceholderText(PLACEHOLDER_FORMAT_SINGLE_INPUT.format(self._list_to_ranges(plottable)))
+
+    def _list_to_ranges(self, input):
+        ranges = []
+        first = last = None  # first and last number of current consecutive range
+        for item in sorted(input):
+            if first is None:
+                first = last = item  # bootstrap
+            elif item == last + 1:  # consecutive
+                last = item  # extend the range
+            else:  # not consecutive
+                ranges.append("{0}{1}{2}".format(first, RANGE_SPECIFIER, last) if last > first else "{0}".format(first))
+                first = last = item
+        # the last range ended by iteration end
+        ranges.append("{0}{1}{2}".format(first, RANGE_SPECIFIER, last) if last > first else "{0}".format(first))
+        return ', '.join(ranges)
 
     def _setup_connections(self):
         ui = self._ui
@@ -212,7 +227,7 @@ class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
         self.selection = selection
 
     def _parse_spec_nums(self):
-        spec_nums = parse_selection_str(self._ui.specNums.text(), self.spec_min, self.spec_max)
+        spec_nums = parse_selection_str(self._ui.specNums.text(), allowed_values=self._plottable_spectra)
         if spec_nums:
             selection = SpectraSelection(self._workspaces)
             selection.spectra = spec_nums
@@ -225,13 +240,14 @@ class SpectraSelectionDialog(SpectraSelectionDialogUIBase):
         return self.selection is not None
 
 
-def parse_selection_str(txt, min_val, max_val):
+def parse_selection_str(txt, min_val=None, max_val=None, allowed_values=None):
     """Parse an input string containing plot index selection.
 
     :param txt: A single line of text containing a comma-separated list of values or range of values, i.e.
     3-4,5,6,8,10-11
     :param min_val: The minimum allowed value
     :param max_val: The maximum allowed value
+    :param allowed_values: The list of allowed values, if this is provided then max and min will be ignored
     :returns A list containing each value in the range or None if the string is invalid
     """
 
@@ -247,7 +263,10 @@ def parse_selection_str(txt, min_val, max_val):
         return True
 
     def is_in_range(val):
-        return min_val <= val <= max_val
+        if allowed_values is not None:
+            return val in allowed_values
+        else:
+            return min_val <= val <= max_val
 
     # split up any commas
     comma_separated = txt.split(',')

qt/python/mantidqt/dialogs/test/test_spectraselectiondialog.py

@@ -64,6 +64,30 @@ class SpectraSelectionDialogTest(unittest.TestCase):
         self.assertEqual("valid range: 51-100", dlg._ui.specNums.placeholderText())
         self.assertEqual("valid range: 0-49", dlg._ui.wkspIndices.placeholderText())
 
+    def test_filling_workspace_details_single_workspace_with_spectra_gaps(self):
+        gappy_ws = WorkspaceFactory.Instance().create("Workspace2D", NVectors=50, XLength=1, YLength=1)
+        for i in range(10):
+            gappy_ws.getSpectrum(i).setSpectrumNo(1 + i)
+        for i in range(10, 16):
+            gappy_ws.getSpectrum(i).setSpectrumNo(1 + (2*i))
+        for i in range(17, 20):
+            gappy_ws.getSpectrum(i).setSpectrumNo(1 + i)
+        for i in range(20, gappy_ws.getNumberHistograms()):
+            gappy_ws.getSpectrum(i).setSpectrumNo(51 + i)
+        dlg = SpectraSelectionDialog([gappy_ws])
+        self.assertEqual("valid range: 1-10, 17-21, 23, 25, 27, 29, 31, 71-100", dlg._ui.specNums.placeholderText())
+        self.assertEqual("valid range: 0-49", dlg._ui.wkspIndices.placeholderText())
+
+    def test_filling_workspace_details_multiple_workspace_with_spectra_gaps(self):
+        gappy_ws = WorkspaceFactory.Instance().create("Workspace2D", NVectors=50, XLength=1, YLength=1)
+        for i in range(20):
+            gappy_ws.getSpectrum(i).setSpectrumNo(1 + i)
+        for i in range(20,gappy_ws.getNumberHistograms()):
+            gappy_ws.getSpectrum(i).setSpectrumNo(161 + i)
+        dlg = SpectraSelectionDialog([gappy_ws, self._multi_spec_ws])
+        self.assertEqual("valid range: 1-20, 181-200", dlg._ui.specNums.placeholderText())
+        self.assertEqual("valid range: 0-49", dlg._ui.wkspIndices.placeholderText())
+
     def test_valid_text_in_boxes_activates_ok(self):
         dlg = SpectraSelectionDialog([self._multi_spec_ws])