Items are now edited and changes propagated to the TableWS, re #24007

TableWorkspace has all columns editable by default. PeaksWS has all columns read-only by default.

Values in editable cells can be changed. Changes are reflected in the underlying WS with the correct type.

TWD Model uses same "allowed workspaces" idea as MatrixWorkspaceDisplay

Framework/PythonInterface/mantid/api/src/Exports/ITableWorkspace.cpp

@@ -296,6 +296,28 @@ PyObject *row(ITableWorkspace &self, int row) {
   return result;
 }
 
+/**
+ * Access a cell and return a corresponding Python type
+ * @param self A reference to the TableWorkspace python object that we were
+ * called on
+ * @param row An integer giving the row
+ */
+PyObject *columnTypes(ITableWorkspace &self) {
+  int numCols = static_cast<int>(self.columnCount());
+
+  PyObject *list = PyList_New(numCols);
+
+  for (int col = 0; col < numCols; col++) {
+    Mantid::API::Column_const_sptr column = self.getColumn(col);
+    const std::type_info &typeID = column->get_type_info();
+    if (PyList_SetItem(list, col, PyString_FromString(typeID.name()))) {
+      throw std::runtime_error("Error while building list");
+    }
+  }
+
+  return list;
+}
+
 /**
  * Adds a new row in the table, where the items are given in a dictionary
  * object mapping {column name:value}. It must contain a key-value entry for
@@ -629,6 +651,9 @@ void export_ITableWorkspace() {
       .def("row", &row, (arg("self"), arg("row")),
            "Return all values of a specific row as a dict.")
 
+      .def("columnTypes", &columnTypes, arg("self"),
+           "Return the types of the columns as a list")
+
       // FromSequence must come first since it takes an object parameter
       // Otherwise, FromDict will never be called as object accepts anything
       .def("addRow", &addRowFromSequence, (arg("self"), arg("row_items_seq")),

buildconfig/CMake/Bootstrap.cmake

@@ -10,7 +10,7 @@ if( MSVC )
   include ( ExternalProject )
   set( EXTERNAL_ROOT ${PROJECT_SOURCE_DIR}/external CACHE PATH "Location to clone third party dependencies to" )
   set( THIRD_PARTY_GIT_URL "https://github.com/mantidproject/thirdparty-msvc2015.git" )
-  set ( THIRD_PARTY_GIT_SHA1 cee87b544761b0311dd3664f9dbbd2f11e0a780a )
+  set ( THIRD_PARTY_GIT_SHA1 11e8ce35bcfe21c26b503d09f424a96c70590523 )
   set ( THIRD_PARTY_DIR ${EXTERNAL_ROOT}/src/ThirdParty )
   # Generates a script to do the clone/update in tmp
   set ( _project_name ThirdParty )

qt/python/mantidqt/widgets/tableworkspacedisplay/model.py

@@ -10,35 +10,73 @@
 #
 from __future__ import (absolute_import, division, print_function)
 
+
 from mantid.dataobjects import PeaksWorkspace, TableWorkspace
-from mantid.py3compat import Enum
-#  TODO remove transient mock dependency -> it is not available on Ubuntu fresh install
-# take MWD approach
-from mantidqt.widgets.matrixworkspacedisplay.test_helpers.matrixworkspacedisplay_common import MockWorkspace
 from mantidqt.widgets.tableworkspacedisplay.marked_columns import MarkedColumns
+from mantid.kernel import V3D
 
+import six
 
-class TableDisplayColumnType(Enum):
-    NUMERIC = 1
-    TEST = 123
+if six.PY2:
+    from functools32 import lru_cache
+else:
+    from functools import lru_cache
 
 
 class TableWorkspaceDisplayModel:
     SPECTRUM_PLOT_LEGEND_STRING = '{}-{}'
     BIN_PLOT_LEGEND_STRING = '{}-bin-{}'
 
+    ALLOWED_WORKSPACE_TYPES = [PeaksWorkspace, TableWorkspace]
+
     def __init__(self, ws):
-        if not isinstance(ws, TableWorkspace) \
-                and not isinstance(ws, PeaksWorkspace) \
-                and not isinstance(ws, MockWorkspace):
-            raise ValueError("The workspace type is not supported: {0}".format(type(ws)))
+        if not any(isinstance(ws, allowed_type) for allowed_type in self.ALLOWED_WORKSPACE_TYPES):
+            raise ValueError("The workspace type is not supported: {0}".format(ws))
 
         self.ws = ws
         self.ws_num_rows = self.ws.rowCount()
         self.ws_num_cols = self.ws.columnCount()
+        self.ws_column_types = self.ws.columnTypes()
+        self.convert_types = self.map_from_type_name(self.ws_column_types)
         self.marked_columns = MarkedColumns()
         self._original_column_headers = self.get_column_headers()
 
+    def _get_bool_from_str(self, string):
+        string = string.lower()
+        if string == "true":
+            return True
+        elif string == "false":
+            return False
+        else:
+            raise ValueError("'{}' is not a valid bool string.".format(string))
+
+    def _get_v3d_from_str(self, string):
+        if '[' in string and ']' in string:
+            string = string[1:-1]
+        if ',' in string:
+            return V3D(*[float(x) for x in string.split(',')])
+        else:
+            raise RuntimeError("'{}' is not a valid V3D string.".format(string))
+
+    def map_from_type_name(self, column_types):
+        convert_types = []
+        for type in column_types:
+            type = type.lower()
+            if 'int' in type:
+                convert_types.append(int)
+            elif 'double' in type or 'float' in type:
+                convert_types.append(float)
+            elif 'string' in type:
+                convert_types.append(str)
+            elif 'bool' in type:
+                convert_types.append(self._get_bool_from_str)
+            elif 'v3d' in type:
+                convert_types.append(self._get_v3d_from_str)
+            else:
+                raise ValueError("Trying to set data for unknown column type {}".format(type))
+
+        return convert_types
+
     def original_column_headers(self):
         return self._original_column_headers[:]
 
@@ -62,3 +100,11 @@ class TableWorkspaceDisplayModel:
 
     def get_column_header(self, index):
         return self.get_column_headers()[index]
+
+    @lru_cache(maxsize=1)
+    def is_peaks_workspace(self):
+        return isinstance(self.ws, PeaksWorkspace)
+
+    def set_cell_data(self, row, col, data):
+        data = self.convert_types[col](data)
+        self.ws.setCell(row, col, data)

qt/python/mantidqt/widgets/tableworkspacedisplay/presenter.py

@@ -14,6 +14,7 @@ from functools import partial
 from qtpy.QtCore import Qt
 from qtpy.QtWidgets import QTableWidgetItem
 
+from mantid.kernel import V3D
 from mantid.simpleapi import DeleteTableRows, StatisticsOfTableWorkspace
 from mantidqt.widgets.common.table_copying import copy_cells, show_mouse_toast, show_no_selection_to_copy_toast
 from mantidqt.widgets.tableworkspacedisplay.error_column import ErrorColumn
@@ -22,13 +23,35 @@ from .model import TableWorkspaceDisplayModel
 from .view import TableWorkspaceDisplayView
 
 
-class TableItem(QTableWidgetItem):
+class WorkbenchTableWidgetItem(QTableWidgetItem):
+    def __init__(self, data, editable=False):
+        # if not editable just initialise the ItemWidget as string
+        if not editable:
+            QTableWidgetItem.__init__(self, str(data))
+            self.setFlags(self.flags() & ~Qt.ItemIsEditable)
+            return
+
+        QTableWidgetItem.__init__(self)
+
+        if isinstance(data, V3D) or isinstance(data, float):
+            data = str(data)
+
+        self.original_data = data
+        # this will correctly turn all number cells into number types
+        self.reset()
+
     def __lt__(self, other):
         try:
             # if the data can be parsed as numbers then compare properly, otherwise default to the Qt implementation
             return float(self.data(Qt.DisplayRole)) < float(other.data(Qt.DisplayRole))
         except:
-            return super(TableItem, self).__lt__(other)
+            return super(WorkbenchTableWidgetItem, self).__lt__(other)
+
+    def reset(self):
+        self.setData(Qt.DisplayRole, self.original_data)
+
+    def update(self):
+        self.original_data = self.data(Qt.DisplayRole)
 
 
 class TableWorkspaceDisplay(object):
@@ -51,6 +74,28 @@ class TableWorkspaceDisplay(object):
         self.update_column_headers()
         self.load_data(self.view)
 
+        # connect to cellChanged signal after the data has been loaded
+        # all consecutive triggers will be from user actions
+        self.view.itemChanged.connect(self.handleItemChanged)
+
+    def handleItemChanged(self, item):
+        """
+        :type item: WorkbenchTableWidgetItem
+        :param item:
+        :return:
+        """
+        print("Changed data in cell:", item, "ops:", dir(item))
+
+        try:
+            self.model.set_cell_data(item.row(), item.column(), item.data(Qt.DisplayRole))
+            item.update()
+        except ValueError:
+            show_mouse_toast("Error: Trying to set non-numeric data into a numeric column.")
+        except Exception as x:
+            show_mouse_toast("Unknown error occurred: {}".format(x))
+        finally:
+            item.reset()
+
     def update_column_headers(self):
         """
         :param extra_labels: Extra labels to be appended to the column headers.
@@ -75,10 +120,14 @@ class TableWorkspaceDisplay(object):
         table.setRowCount(num_rows)
 
         num_cols = self.model.get_number_of_columns()
+
+        # the table should be editable if the ws is not PeaksWS
+        editable = not self.model.is_peaks_workspace()
+
         for col in range(num_cols):
             column_data = self.model.get_column(col)
             for row in range(num_rows):
-                item = TableItem(str(column_data[row]))
+                item = WorkbenchTableWidgetItem(column_data[row], editable=editable)
                 table.setItem(row, col, item)
 
     def action_copy_cells(self):
@@ -272,7 +321,7 @@ class TableWorkspaceDisplay(object):
             except ValueError as e:
                 #     TODO log error?
                 self.view.show_warning(
-                    "One or more of the columns being plotted contain invalid data for MatPlotLib." \
+                    "One or more of the columns being plotted contain invalid data for MatPlotLib."
                     "\n\nError message:\n{}".format(e), "Invalid data - Mantid Workbench")
                 return
 

qt/python/mantidqt/widgets/tableworkspacedisplay/test/test_tableworkspacedisplay_model.py

@@ -18,10 +18,11 @@ from mantidqt.widgets.matrixworkspacedisplay.test_helpers.matrixworkspacedisplay
 from mantidqt.widgets.tableworkspacedisplay.model import TableWorkspaceDisplayModel
 
 
-# from mantid.simpleapi import CreateSampleWorkspace
-
-
 class TableWorkspaceDisplayModelTest(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls):
+        # Allow the MockWorkspace to work within the model
+        TableWorkspaceDisplayModel.ALLOWED_WORKSPACE_TYPES.append(MockWorkspace)
 
     def test_get_name(self):
         ws = MockWorkspace()
@@ -35,17 +36,26 @@ class TableWorkspaceDisplayModelTest(unittest.TestCase):
         self.assertRaises(ValueError, lambda: TableWorkspaceDisplayModel([]))
         self.assertRaises(ValueError, lambda: TableWorkspaceDisplayModel(1))
         self.assertRaises(ValueError, lambda: TableWorkspaceDisplayModel("test_string"))
- 
-    # def test_no_raise_with_supported_workspace(self):
-    #     ws = MockWorkspace()
-    #     expected_name = "TEST_WORKSPACE"
-    #     ws.name = Mock(return_value=expected_name)
-    #
-    #     # no need to assert anything - if the constructor raises the test will fail
-    #     TableWorkspaceDisplayModel(ws)
-    #
-    #     ws = CreateSampleWorkspace(NumBanks=1, BankPixelWidth=4, NumEvents=10)
-    #     TableWorkspaceDisplayModel(ws)
+
+
+class TableWorkspaceDisplayModelTestWithFramework(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        # Allow the MockWorkspace to work within the model
+        TableWorkspaceDisplayModel.ALLOWED_WORKSPACE_TYPES.append(MockWorkspace)
+
+    def test_no_raise_with_supported_workspace(self):
+        from mantid.simpleapi import CreateSampleWorkspace  # noqa
+        ws = MockWorkspace()
+        expected_name = "TEST_WORKSPACE"
+        ws.name = Mock(return_value=expected_name)
+
+        # no need to assert anything - if the constructor raises the test will fail
+        TableWorkspaceDisplayModel(ws)
+
+        ws = CreateSampleWorkspace(NumBanks=1, BankPixelWidth=4, NumEvents=10)
+        TableWorkspaceDisplayModel(ws)
 
 
 if __name__ == '__main__':

qt/python/mantidqt/widgets/tableworkspacedisplay/test_helpers/__init__.py

-# Mantid Repository : https://github.com/mantidproject/mantid
-#
-# Copyright © 2018 ISIS Rutherford Appleton Laboratory UKRI,
-#     NScD Oak Ridge National Laboratory, European Spallation Source
-#     & Institut Laue - Langevin
-# SPDX - License - Identifier: GPL - 3.0 +
-#  This file is part of the mantid workbench.
-#
-#

qt/python/mantidqt/widgets/tableworkspacedisplay/test_helpers/mock_tableworkspacedisplay.py

-# Mantid Repository : https://github.com/mantidproject/mantid
-#
-# Copyright © 2018 ISIS Rutherford Appleton Laboratory UKRI,
-#     NScD Oak Ridge National Laboratory, European Spallation Source
-#     & Institut Laue - Langevin
-# SPDX - License - Identifier: GPL - 3.0 +
-#  This file is part of the mantid workbench.
-#
-#
-from mock import Mock
-
-from mantidqt.widgets.TableWorkspacedisplay.table_view_model import TableWorkspaceTableViewModelType
-
-
-class MockQTableHeader(object):
-    def __init__(self):
-        self.addAction = Mock()
-
-
-class MockQSelection:
-    def __init__(self):
-        self.mock_item_range = MockQItemRange()
-        self.first = Mock(return_value=self.mock_item_range)
-
-
-class MockQItemRange(object):
-    def __init__(self):
-        self.top = Mock(return_value=0)
-        self.bottom = Mock(return_value=2)
-        self.left = Mock(return_value=0)
-        self.right = Mock(return_value=2)
-
-
-class MockQSelectionModel:
-    def __init__(self):
-        self.hasSelection = Mock()
-        self.selectedRows = None
-        self.selectedColumns = None
-        self.currentIndex = None
-        self.mock_selection = MockQSelection()
-        self.selection = Mock(return_value=self.mock_selection)
-
-
-class MockQTableViewModel:
-    def __init__(self):
-        self.type = TableWorkspaceTableViewModelType.x
-
-
-class MockQTableView:
-    def __init__(self):
-        self.setContextMenuPolicy = Mock()
-        self.addAction = Mock()
-        self.mock_horizontalHeader = MockQTableHeader()
-        self.mock_verticalHeader = MockQTableHeader()
-        self.horizontalHeader = Mock(return_value=self.mock_horizontalHeader)
-        self.verticalHeader = Mock(return_value=self.mock_verticalHeader)
-        self.setModel = Mock()
-        self.mock_model = MockQTableViewModel()
-        self.model = Mock(return_value=self.mock_model)
-
-        self.mock_selection_model = MockQSelectionModel()
-
-        self.selectionModel = Mock(return_value=self.mock_selection_model)
-
-
-class MockTableWorkspaceDisplayView:
-    def __init__(self):
-        self.set_context_menu_actions = Mock()
-        self.table_x = MockQTableView()
-        self.table_y = MockQTableView()
-        self.table_e = MockQTableView()
-        self.set_model = Mock()
-        self.copy_to_clipboard = Mock()
-        self.show_mouse_toast = Mock()
-        self.ask_confirmation = None
-
-
-class MockTableWorkspaceDisplayModel:
-    def __init__(self):
-        self.get_spectrum_plot_label = Mock()
-        self.get_bin_plot_label = Mock()

qt/python/mantidqt/widgets/tableworkspacedisplay/view.py

@@ -43,6 +43,7 @@ class TableWorkspaceDisplayView(QTableWidget):
         self.resize(600, 400)
         self.show()
 
+
     def doubleClickedHeader(self):
         print("Double clicked WOO")
 
@@ -114,8 +115,8 @@ class TableWorkspaceDisplayView(QTableWidget):
         set_as_y = QAction(self.TBD, "Set as Y", menu_main)
         set_as_y.triggered.connect(self.presenter.action_set_as_y)
 
-        set_as_nein = QAction(self.TBD, "Set as None", menu_main)
-        set_as_nein.triggered.connect(self.presenter.action_set_as_none)
+        set_as_none = QAction(self.TBD, "Set as None", menu_main)
+        set_as_none.triggered.connect(self.presenter.action_set_as_none)
 
         statistics_on_columns = QAction(self.STATISTICS_ON_ROW, "Statistics on Columns", menu_main)
         statistics_on_columns.triggered.connect(self.presenter.action_statistics_on_columns)
@@ -137,21 +138,24 @@ class TableWorkspaceDisplayView(QTableWidget):
         menu_main.addAction(set_as_x)
         menu_main.addAction(set_as_y)
 
-        # get the columns marked as Y
         marked_y_cols = self.presenter.get_columns_marked_as_y()
         num_y_cols = len(marked_y_cols)
+
+        # If any columns are marked as Y then generate the set error menu
         if num_y_cols > 0:
             menu_set_as_y_err = QMenu("Set error for Y...")
             for col in range(num_y_cols):
-                # get the real index of the column
-                # display the real index in the menu
                 set_as_y_err = QAction(self.TBD, "Y{}".format(col), menu_main)
+                # the column index of the column relative to the whole table, this is necessary
+                # so that later the data of the column marked as error can be retrieved
                 real_column_index = marked_y_cols[col]
+                # col here holds the index in the LABEL (multiple Y columns have labels Y0, Y1, YN...)
+                # this is NOT the same as the column relative to the WHOLE table
                 set_as_y_err.triggered.connect(partial(self.presenter.action_set_as_y_err, real_column_index, col))
                 menu_set_as_y_err.addAction(set_as_y_err)
             menu_main.addMenu(menu_set_as_y_err)
 
-        menu_main.addAction(set_as_nein)
+        menu_main.addAction(set_as_none)
         menu_main.addAction(self.make_separator(menu_main))
         menu_main.addAction(statistics_on_columns)
         menu_main.addAction(self.make_separator(menu_main))
@@ -189,4 +193,4 @@ class TableWorkspaceDisplayView(QTableWidget):
         return True if reply == QMessageBox.Yes else False
 
     def show_warning(self, message, title="Mantid Workbench"):
-        QMessageBox.warning(self, title, message)
\ No newline at end of file
+        QMessageBox.warning(self, title, message)