Merge pull request #27986 from martyngigg/workbench-progress-markers

Replace block-style progress reporter with C-api based one

Framework/PythonInterface/core/CMakeLists.txt

@@ -11,6 +11,7 @@ set(
   src/GlobalInterpreterLock.cpp
   src/NDArray.cpp
   src/ReleaseGlobalInterpreterLock.cpp
+  src/UninstallTrace.cpp
   src/WrapperHelpers.cpp
   src/Converters/CloneToNDArray.cpp
   src/Converters/DateAndTime.cpp
@@ -39,6 +40,7 @@ set(
   inc/MantidPythonInterface/core/ReleaseGlobalInterpreterLock.h
   inc/MantidPythonInterface/core/StlExportDefinitions.h
   inc/MantidPythonInterface/core/TypedValidatorExporter.h
+  inc/MantidPythonInterface/core/UninstallTrace.h
   inc/MantidPythonInterface/core/VersionCompat.h
   inc/MantidPythonInterface/core/WrapperHelpers.h
   inc/MantidPythonInterface/core/WrapPython.h

Framework/PythonInterface/core/inc/MantidPythonInterface/core/UninstallTrace.h

+// Mantid Repository : https://github.com/mantidproject/mantid
+//
+// Copyright © 2020 ISIS Rutherford Appleton Laboratory UKRI,
+//     NScD Oak Ridge National Laboratory, European Spallation Source
+//     & Institut Laue - Langevin
+// SPDX - License - Identifier: GPL - 3.0 +
+#ifndef MANTID_PYTHONINTERFACE_UNINSTALLTRACE_H
+#define MANTID_PYTHONINTERFACE_UNINSTALLTRACE_H
+
+#include "MantidPythonInterface/core/DllConfig.h"
+#include "MantidPythonInterface/core/WrapPython.h"
+
+namespace Mantid::PythonInterface {
+
+/**
+ * @brief RAII handler to temporarily remove and reinstall a Python trace
+ * function
+ */
+class MANTID_PYTHONINTERFACE_CORE_DLL UninstallTrace {
+public:
+  UninstallTrace();
+  ~UninstallTrace();
+
+private:
+  Py_tracefunc m_tracefunc;
+  PyObject *m_tracearg;
+};
+
+} // namespace Mantid::PythonInterface
+
+#endif // MANTID_PYTHONINTERFACE_UNINSTALLTRACE_H

Framework/PythonInterface/core/src/UninstallTrace.cpp

+// Mantid Repository : https://github.com/mantidproject/mantid
+//
+// Copyright © 2020 ISIS Rutherford Appleton Laboratory UKRI,
+//     NScD Oak Ridge National Laboratory, European Spallation Source
+//     & Institut Laue - Langevin
+// SPDX - License - Identifier: GPL - 3.0 +
+#include "MantidPythonInterface/core/UninstallTrace.h"
+
+namespace Mantid::PythonInterface {
+
+/**
+ * Saves any function and argument previously set by PyEval_SetTrace
+ * and calls PyEval_SetTrace(nullptr, nullptr) to remove the trace function
+ */
+UninstallTrace::UninstallTrace() {
+  PyThreadState *curThreadState = PyThreadState_GET();
+  m_tracefunc = curThreadState->c_tracefunc;
+  m_tracearg = curThreadState->c_traceobj;
+  Py_XINCREF(m_tracearg);
+  PyEval_SetTrace(nullptr, nullptr);
+}
+
+/**
+ * Reinstates any trace function with PyEval_SetTrace and any saved arguments
+ * from the constructor
+ */
+UninstallTrace::~UninstallTrace() { PyEval_SetTrace(m_tracefunc, m_tracearg); }
+
+} // namespace Mantid::PythonInterface

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

@@ -10,6 +10,7 @@
 #include "MantidKernel/WarningSuppressions.h"
 #include "MantidPythonInterface/core/GetPointer.h"
 #include "MantidPythonInterface/core/PythonObjectInstantiator.h"
+#include "MantidPythonInterface/core/UninstallTrace.h"
 
 #include <boost/python/class.hpp>
 #include <boost/python/def.hpp>
@@ -26,6 +27,7 @@
 using namespace Mantid::API;
 using namespace boost::python;
 using Mantid::PythonInterface::PythonObjectInstantiator;
+using Mantid::PythonInterface::UninstallTrace;
 
 GET_POINTER_SPECIALIZATION(AlgorithmFactoryImpl)
 
@@ -117,6 +119,7 @@ GNU_DIAG_OFF("cast-qual")
  *              or an instance of a class type derived from PythonAlgorithm
  */
 void subscribe(AlgorithmFactoryImpl &self, const boost::python::object &obj) {
+  UninstallTrace uninstallTrace;
   std::lock_guard<std::recursive_mutex> lock(PYALG_REGISTER_MUTEX);
 
   static auto *const pyAlgClass =

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

@@ -13,6 +13,7 @@
 #include "MantidPythonInterface/api/PythonAlgorithm/AlgorithmAdapter.h"
 #include "MantidPythonInterface/core/GetPointer.h"
 #include "MantidPythonInterface/core/PythonObjectInstantiator.h"
+#include "MantidPythonInterface/core/UninstallTrace.h"
 
 #include <boost/python/class.hpp>
 #include <boost/python/def.hpp>
@@ -29,6 +30,7 @@ using Mantid::API::IBackgroundFunction;
 using Mantid::API::IFunction;
 using Mantid::API::IPeakFunction;
 using Mantid::PythonInterface::PythonObjectInstantiator;
+using Mantid::PythonInterface::UninstallTrace;
 
 using namespace boost::python;
 
@@ -172,11 +174,10 @@ std::recursive_mutex FUNCTION_REGISTER_MUTEX;
  * @param classObject A Python class derived from IFunction
  */
 void subscribe(FunctionFactoryImpl &self, PyObject *classObject) {
+  UninstallTrace uninstallTrace;
   std::lock_guard<std::recursive_mutex> lock(FUNCTION_REGISTER_MUTEX);
   static auto *baseClass = const_cast<PyTypeObject *>(
       converter::registered<IFunction>::converters.to_python_target_type());
-  // object mantidapi(handle<>(PyImport_ImportModule("mantid.api")));
-  // object ifunction = mantidapi.attr("IFunction");
 
   // obj should be a class deriving from IFunction
   // PyObject_IsSubclass can set the error handler if classObject

docs/source/release/v4.3.0/mantidworkbench.rst

@@ -86,6 +86,7 @@ Improvements
 - The Save menu action in the workspaces toolbox to save using version 1 of the SaveAscii algorithm has been removed as no one was using it and it only added confusion. The option to save using the most recent version of SaveASCII is still available.
 - You can now search for functions when doing fits.
 - A help button has been added to the fitting add function dialog.
+- The progress reporting for scripts has been vastly improved and now reports at the line level.
 
 Bugfixes
 ########
@@ -103,5 +104,6 @@ Bugfixes
 - The help button in fitting now finds the relevant page.
 - Fixed an issue where fitting a distribution workspace was normalised twice.
 - Overplots will be normalized by bin width if they are overplotting a curve from a workspace which is a distribution.
+- Several bugs in the way Python scripts were parsed and executed, including blank lines after a colon and tabs in strings, have been fixed.
 
 :ref:`Release 4.3.0 <v4.3.0>`

qt/applications/workbench/workbench/config/__init__.py

@@ -18,13 +18,16 @@ and use it to access the settings
 """
 from __future__ import (absolute_import, unicode_literals)
 
+# std imports
+import os
+import sys
+
 # third-party imports
 from qtpy.QtCore import QSettings
 
 # local imports
 from .user import UserConfig
 
-
 # -----------------------------------------------------------------------------
 # Constants
 # -----------------------------------------------------------------------------
@@ -32,6 +35,18 @@ ORGANIZATION = 'mantidproject'
 ORG_DOMAIN = 'mantidproject.org'
 APPNAME = 'mantidworkbench'
 
+DEFAULT_SCRIPT_CONTENT = ""
+if sys.version_info < (3, 0):
+    DEFAULT_SCRIPT_CONTENT += "# The following line helps with future compatibility with Python 3" + os.linesep + \
+                              "# print must now be used as a function, e.g print('Hello','World')" + os.linesep + \
+                              "from __future__ import (absolute_import, division, print_function, unicode_literals)" + \
+                              os.linesep + os.linesep
+
+DEFAULT_SCRIPT_CONTENT += "# import mantid algorithms, numpy and matplotlib" + os.linesep + \
+                          "from mantid.simpleapi import *" + os.linesep + \
+                          "import matplotlib.pyplot as plt" + os.linesep + \
+                          "import numpy as np" + os.linesep + os.linesep
+
 # Iterable containing defaults for each configurable section of the code
 # General application settings are in the main section
 DEFAULTS = {

qt/applications/workbench/workbench/plotting/plotscriptgenerator/__init__.py

@@ -12,7 +12,7 @@ from mantid.plots.mantidaxes import MantidAxes
 
 from mantidqt.widgets.plotconfigdialog import curve_in_ax
 from matplotlib.legend import Legend
-from workbench.plugins.editor import DEFAULT_CONTENT
+from workbench.config import DEFAULT_SCRIPT_CONTENT
 from workbench.plotting.plotscriptgenerator.axes import (generate_axis_limit_commands,
                                                          generate_axis_label_commands,
                                                          generate_set_title_command,
@@ -67,7 +67,7 @@ def generate_script(fig, exclude_headers=False):
     if not plot_commands:
         return
 
-    cmds = [] if exclude_headers else [DEFAULT_CONTENT]
+    cmds = [] if exclude_headers else [DEFAULT_SCRIPT_CONTENT]
     cmds.extend(generate_workspace_retrieval_commands(fig) + [''])
     cmds.append("{}, {} = {}".format(FIG_VARIABLE, AXES_VARIABLE, generate_subplots_command(fig)))
     cmds.extend(plot_commands)

qt/applications/workbench/workbench/plugins/editor.py

@@ -11,8 +11,6 @@ from __future__ import (absolute_import, unicode_literals)
 
 # system imports
 import os.path as osp
-import os
-import sys
 
 # third-party library imports
 from qtpy.QtWidgets import QVBoxLayout
@@ -20,25 +18,13 @@ from qtpy.QtWidgets import QVBoxLayout
 # local package imports
 from mantid.kernel import logger
 from mantidqt.widgets.codeeditor.multifileinterpreter import MultiPythonFileInterpreter
+from ..config import DEFAULT_SCRIPT_CONTENT
 from ..config.fonts import text_font
 from ..plugins.base import PluginWidget
 
 # from mantidqt.utils.qt import toQSettings when readSettings/writeSettings are implemented
 
 
-# Initial content
-DEFAULT_CONTENT = ""
-if sys.version_info < (3,0):
-    DEFAULT_CONTENT += "# The following line helps with future compatibility with Python 3" + os.linesep + \
-                       "# print must now be used as a function, e.g print('Hello','World')" + os.linesep + \
-                       "from __future__ import (absolute_import, division, print_function, unicode_literals)" + \
-                       os.linesep + os.linesep
-
-DEFAULT_CONTENT += "# import mantid algorithms, numpy and matplotlib" + os.linesep + \
-                   "from mantid.simpleapi import *" + os.linesep + \
-                   "import matplotlib.pyplot as plt" + os.linesep + \
-                   "import numpy as np" + os.linesep + os.linesep
-
 # Accepted extensions for drag-and-drop to editor
 ACCEPTED_FILE_EXTENSIONS = ['.py', '.pyw']
 # QSettings key for session tabs
@@ -58,7 +44,7 @@ class MultiFileEditor(PluginWidget):
 
         # layout
         self.editors = MultiPythonFileInterpreter(font=font,
-                                                  default_content=DEFAULT_CONTENT,
+                                                  default_content=DEFAULT_SCRIPT_CONTENT,
                                                   parent=self)
         layout = QVBoxLayout()
         layout.addWidget(self.editors)

qt/python/CMakeLists.txt

@@ -91,11 +91,6 @@ if(ENABLE_WORKBENCH OR ENABLE_WORKBENCH)
     mantidqt/utils/test/test_modal_tester.py
     mantidqt/utils/test/test_qt_utils.py
     mantidqt/utils/test/test_writetosignal.py
-    mantidqt/widgets/codeeditor/test/test_codeeditor.py
-    mantidqt/widgets/codeeditor/test/test_completion.py
-    mantidqt/widgets/codeeditor/test/test_errorformatter.py
-    mantidqt/widgets/codeeditor/test/test_execution.py
-    mantidqt/widgets/codeeditor/test/test_interpreter.py
     mantidqt/widgets/test/test_messagedisplay.py
     mantidqt/widgets/test/test_fitpropertybrowser.py
     mantidqt/widgets/test/test_fitpropertybrowserbase.py
@@ -118,6 +113,11 @@ if(ENABLE_WORKBENCH OR ENABLE_WORKBENCH)
       PYTHON_WIDGET_QT5_ONLY_TESTS
       mantidqt/widgets/algorithmselector/test/observer_test.py
       mantidqt/widgets/algorithmselector/test/test_algorithmselector.py
+      mantidqt/widgets/codeeditor/test/test_codeeditor.py
+      mantidqt/widgets/codeeditor/test/test_completion.py
+      mantidqt/widgets/codeeditor/test/test_execution.py
+      mantidqt/widgets/codeeditor/test/test_errorformatter.py
+      mantidqt/widgets/codeeditor/test/test_interpreter.py
       mantidqt/widgets/codeeditor/test/test_interpreter_view.py
       mantidqt/widgets/codeeditor/test/test_multifileinterpreter.py
       mantidqt/widgets/codeeditor/test/test_multifileinterpreter_view.py

qt/python/mantidqt/_common.sip

@@ -2,6 +2,8 @@
 #include "MantidQtWidgets/Common/Message.h"
 #include "MantidQtWidgets/Common/WorkspaceObserver.h"
 #include "MantidPythonInterface/core/VersionCompat.h"
+#include "frameobject.h"
+
 // Allows suppression of namespaces within the module
 using namespace MantidQt::MantidWidgets;
 using namespace MantidQt::MantidWidgets::Batch;
@@ -104,18 +106,6 @@ public:
   void setSource(const QString &source);
 };
 
-%If (Qt_5_0_0 -)
-class AlgorithmProgressWidget : QWidget {
-%TypeHeaderCode
-#include "MantidQtWidgets/Common/AlgorithmProgress/AlgorithmProgressWidget.h"
-%End
-
-public:
-    AlgorithmProgressWidget(QWidget *parent /TransferThis/ = 0);
-    void blockUpdates(bool block = true);
-};
-%End
-
 class ScriptEditor : QWidget {
 %TypeHeaderCode
 #include "MantidQtWidgets/Common/ScriptEditor.h"
@@ -185,8 +175,6 @@ public:
   void replaceAll(const QString &search, const QString &replace,
                               bool regex, bool caseSensitive, bool matchWords,
                               bool wrap, bool backward);
-
-
 public slots:
   void updateProgressMarker(int lineno, bool error);
   void zoomTo(int level);
@@ -201,6 +189,33 @@ private:
   ScriptEditor(const ScriptEditor&);
 };
 
+%If (Qt_5_0_0 -)
+
+class AlgorithmProgressWidget : QWidget {
+%TypeHeaderCode
+#include "MantidQtWidgets/Common/AlgorithmProgress/AlgorithmProgressWidget.h"
+%End
+
+public:
+  AlgorithmProgressWidget(QWidget *parent /TransferThis/ = 0);
+  void blockUpdates(bool block = true);
+};
+
+class CodeExecution {
+%TypeHeaderCode
+#include "MantidQtWidgets/Common/Python/CodeExecution.h"
+using MantidQt::Widgets::Common::Python::CodeExecution;
+%End
+
+public:
+  CodeExecution(ScriptEditor*);
+  SIP_PYOBJECT execute(const QString &, const QString &, int, SIP_PYOBJECT);
+private:
+  CodeExecution(const CodeExecution&);
+};
+
+%End // end >= Qt5
+
 
 class AlgorithmDialog: QDialog {
 %TypeHeaderCode

qt/python/mantidqt/widgets/codeeditor/execution.py

@@ -11,8 +11,15 @@ from __future__ import absolute_import, unicode_literals
 
 import __future__
 import ast
+
+try:
+    import builtins
+except ImportError:
+    import __main__
+
+    builtins = __main__.__builtins__
+import copy
 import os
-import re
 from io import BytesIO
 from lib2to3.pgen2.tokenize import detect_encoding
 
@@ -21,11 +28,13 @@ from qtpy.QtWidgets import QApplication
 
 from mantidqt.utils import AddedToSysPath
 from mantidqt.utils.asynchronous import AsyncTask, BlockingAsyncTaskWithCallback
-from mantidqt.widgets.codeeditor.inputsplitter import InputSplitter
+from mantidqt.utils.qt import import_qt
+
+# Core object to execute the code with optinal progress tracking
+CodeExecution = import_qt('..._common', 'mantidqt.widgets.codeeditor.execution', 'CodeExecution')
 
 EMPTY_FILENAME_ID = '<string>'
 FILE_ATTR = '__file__'
-COMPILE_MODE = 'exec'
 
 
 def _get_imported_from_future(code_str):
@@ -44,6 +53,7 @@ def _get_imported_from_future(code_str):
         if isinstance(node, ast.ImportFrom):
             if node.module == '__future__':
                 future_imports.extend([import_alias.name for import_alias in node.names])
+                break
     return future_imports
 
 
@@ -73,21 +83,15 @@ class PythonCodeExecution(QObject):
     """
     sig_exec_success = Signal(object)
     sig_exec_error = Signal(object)
-    sig_exec_progress = Signal(int)
 
-    def __init__(self, startup_code=None):
+    def __init__(self, editor=None):
         """Initialize the object"""
         super(PythonCodeExecution, self).__init__()
-
+        self._editor = editor
         self._globals_ns = None
-
         self._task = None
-
         self.reset_context()
 
-        # the code is not executed initially so code completion won't work
-        # on variables until part is executed
-
     @property
     def globals_ns(self):
         return self._globals_ns
@@ -129,34 +133,18 @@ class PythonCodeExecution(QObject):
         is used
         :raises: Any error that the code generates
         """
-        if filename:
-            self.globals_ns[FILE_ATTR] = filename
-        else:
+        if not filename:
             filename = EMPTY_FILENAME_ID
-        flags = get_future_import_compiler_flags(code_str)
-        # This line checks the whole code string for syntax errors, so that no
-        # code blocks are executed if the script has invalid syntax.
-        try:
-            compile(code_str, filename, mode=COMPILE_MODE, dont_inherit=True, flags=flags)
-        except SyntaxError as e:  # Encoding declarations cause issues in compile calls. If found, remove them.
-            if "encoding declaration in Unicode string" in str(e):
-                code_str = re.sub("coding[=:]\s*([-\w.]+)", "", code_str, 1)
-                compile(code_str, filename, mode=COMPILE_MODE, dont_inherit=True, flags=flags)
-            else:
-                raise e
 
+        self.globals_ns[FILE_ATTR] = filename
+        flags = get_future_import_compiler_flags(code_str)
         with AddedToSysPath([os.path.dirname(filename)]):
-            sig_progress = self.sig_exec_progress
-            for block in code_blocks(code_str):
-                sig_progress.emit(block.lineno)
-                # compile so we can set the filename
-                code_obj = compile(block.code_str, filename, mode=COMPILE_MODE,
-                                   dont_inherit=True, flags=flags)
-                exec (code_obj, self.globals_ns, self.globals_ns)
+            executor = CodeExecution(self._editor)
+            executor.execute(code_str, filename, flags, self.globals_ns)
 
     def reset_context(self):
         # create new context for execution
-        self._globals_ns, self._namespace = {}, {}
+        self._globals_ns = copy.copy(builtins.globals())
 
     # --------------------- Callbacks -------------------------------
     def _on_success(self, task_result):
@@ -167,51 +155,6 @@ class PythonCodeExecution(QObject):
         self._reset_task()
         self.sig_exec_error.emit(task_error)
 
-    def _on_progress_updated(self, lineno):
-        self.sig_exec_progress(lineno)
-
     # --------------------- Private -------------------------------
     def _reset_task(self):
         self._task = None
-
-
-class CodeBlock(object):
-    """Holds an executable code object. It also stores the line number
-    of the first line within a larger group of code blocks"""
-
-    def __init__(self, code_str, lineno):
-        self.code_str = code_str
-        self.lineno = lineno
-
-
-def code_blocks(code_str):
-    """Generator to produce blocks of executable code
-    from the given code string.
-    """
-    lineno_cur = 0
-    lines = code_str.splitlines()
-    line_count = len(lines)
-    isp = InputSplitter()
-    for line in lines:
-        lineno_cur += 1
-        # IPython InputSplitter assumes that indentation is 4 spaces, not tabs.
-        # Accounting for that here, rather than using script-level "tabs to spaces"
-        # allows the user to keep tabs in their script if they wish.
-        line = line.replace("\t", " "*4)
-        isp.push(line)
-        # If we need more input to form a complete statement
-        # or we are not at the end of the code then keep
-        # going
-        if isp.push_accepts_more() and lineno_cur != line_count:
-            continue
-        else:
-            # Now we have a complete set of executable statements
-            # throw them at the execution engine
-            code = isp.source
-            isp.reset()
-            yield CodeBlock(code, lineno_cur)
-            # In order to keep the line numbering in error stack traces
-            # consistent each executed block needs to have the statements
-            # on the same line as they are in the real code so we prepend
-            # blank lines to make this so
-            isp.push('\n' * lineno_cur)

qt/python/mantidqt/widgets/codeeditor/inputsplitter.py

-# Mantid Repository : https://github.com/mantidproject/mantid
-#
-# Copyright © 2017 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 mantidqt package
-#
-#
-from __future__ import (absolute_import, unicode_literals)
-
-# std imports
-
-# 3rd party imports
-from IPython.core.inputsplitter import InputSplitter as IPyInputSplitter
-
-# local imports
-
-
-class InputSplitter(IPyInputSplitter):
-    r"""A specialized version of IPython's input splitter.
-
-    The major differences between this and the base version are:
-
-      - push considers a SyntaxError as incomplete code
-      - push_accepts_more returns False when the indentation has return flush
-        regardless of whether the statement is a single line
-    """
-
-    def push(self, lines):
-        """Push one or more lines of input.
-
-        This stores the given lines and returns a status code indicating
-        whether the code forms a complete Python block or not.
-
-        Any exceptions generated in compilation are swallowed, but if an
-        exception was produced, the method returns True.
-
-        Parameters
-        ----------
-        lines : string
-          One or more lines of Python input.
-
-        Returns
-        -------
-        is_complete : boolean
-          True if the current input source (the result of the current input
-          plus prior inputs) forms a complete Python execution block.  Note that
-          this value is also stored as a private attribute (``_is_complete``), so it
-          can be queried at any time.
-        """
-        self._store(lines)
-        source = self.source
-
-        # Before calling _compile(), reset the code object to None so that if an
-        # exception is raised in compilation, we don't mislead by having
-        # inconsistent code/source attributes.
-        self.code, self._is_complete = None, None
-
-        # Honor termination lines properly
-        if source.endswith('\\\n'):
-            return False
-
-        try:
-            self._update_indent(lines)
-        except TypeError: # _update_indent was changed in IPython 6.0
-            self._update_indent()
-        except AttributeError: # changed definition in IPython 6.3
-            self.get_indent_spaces()
-        try:
-            self.code = self._compile(source, symbol="exec")
-        # Invalid syntax can produce any of a number of different errors from
-        # inside the compiler, so we have to catch them all.  Syntax errors
-        # immediately produce a 'ready' block, so the invalid Python can be
-        # sent to the kernel for evaluation with possible ipython
-        # special-syntax conversion.
-        except (SyntaxError, OverflowError, ValueError, TypeError,
-                MemoryError):
-            self._is_complete = False
-        else:
-            # Compilation didn't produce any exceptions (though it may not have
-            # given a complete code object)
-            self._is_complete = self.code is not None
-
-        return self._is_complete
-
-    def push_accepts_more(self):
-        """Return whether a block of input can accept more input.
-
-        This method is meant to be used by line-oriented frontends, who need to
-        guess whether a block is complete or not based solely on prior and
-        current input lines.  The InputSplitter considers it has a complete
-        block and will not accept more input when either:
-
-        * A SyntaxError is raised
-
-        * The code is complete and consists of a single line or a single
-          non-compound statement
-
-        * The code is complete and has a blank line at the end
-        """
-
-        # With incomplete input, unconditionally accept more
-        # A syntax error also sets _is_complete to True - see push()
-        if not self._is_complete:
-            return True
-
-        # If there's just a single line or AST node, and we're flush left, as is
-        # the case after a simple statement such as 'a=1', we want to execute it
-        # straight away.
-        if self.indent_spaces == 0:
-            return False
-
-        # General fallback - accept more code
-        return True

qt/python/mantidqt/widgets/codeeditor/interpreter.py

@@ -136,7 +136,7 @@ class PythonFileInterpreter(QWidget):
         self.setLayout(self.layout)
         self._setup_editor(content, filename)
 
-        self._presenter = PythonFileInterpreterPresenter(self, PythonCodeExecution(content))
+        self._presenter = PythonFileInterpreterPresenter(self, PythonCodeExecution(self.editor))
         self.code_commenter = CodeCommenter(self.editor)
         self.code_completer = CodeCompleter(self.editor, self._presenter.model.globals_ns)
 
@@ -146,7 +146,6 @@ class PythonFileInterpreter(QWidget):
         self.setAttribute(Qt.WA_DeleteOnClose, True)
 
         # Connect the model signals to the view's signals so they can be accessed from outside the MVP
-        self._presenter.model.sig_exec_progress.connect(self.sig_progress)
         self._presenter.model.sig_exec_error.connect(self.sig_exec_error)
         self._presenter.model.sig_exec_success.connect(self.sig_exec_success)
 
@@ -285,7 +284,6 @@ class PythonFileInterpreterPresenter(QObject):
         # connect signals
         self.model.sig_exec_success.connect(self._on_exec_success)
         self.model.sig_exec_error.connect(self._on_exec_error)
-        self.model.sig_exec_progress.connect(self._on_progress_update)
 
         # starts idle
         self.view.set_status_message(IDLE_STATUS_MSG)
@@ -340,7 +338,14 @@ class PythonFileInterpreterPresenter(QObject):
         if hasattr(exc_value, 'lineno'):
             lineno = exc_value.lineno + self._code_start_offset
         elif exc_stack is not None:
-            lineno = exc_stack[-1][1] + self._code_start_offset
+            try:
+                lineno = exc_stack[0].lineno + self._code_start_offset
+            except (AttributeError, IndexError):
+                # Python 2 fallback
+                try:
+                    lineno = exc_stack[-1][1] + self._code_start_offset
+                except IndexError:
+                    lineno = -1
         else:
             lineno = -1
         sys.stderr.write(self._error_formatter.format(exc_type, exc_value, exc_stack) + os.linesep)

qt/python/mantidqt/widgets/codeeditor/test/test_execution.py

@@ -21,6 +21,7 @@ from qtpy.QtCore import QCoreApplication, QObject
 # local imports
 from mantid.py3compat import StringIO
 from mantid.py3compat.mock import patch
+
 from mantidqt.utils.qt.testing import start_qapplication
 from mantidqt.widgets.codeeditor.execution import PythonCodeExecution, _get_imported_from_future
 
@@ -38,34 +39,21 @@ class Receiver(QObject):
         self.error_stack = traceback.extract_tb(task_result.stack)
 
 
-class ReceiverWithProgress(Receiver):
-
-    def __init__(self):
-        super(ReceiverWithProgress, self).__init__()
-        self.lines_received = []
-
-    def on_progess_update(self, lineno):
-        self.lines_received.append(lineno)
-
-
 @start_qapplication
 class PythonCodeExecutionTest(unittest.TestCase):
 
-    def test_default_construction_yields_empty_context(self):
+    def test_default_construction_context_contains_builtins(self):
         executor = PythonCodeExecution()
-        self.assertEqual(0, len(executor.globals_ns))
+        self.assertTrue('__builtins__' in executor.globals_ns)
 
-    def test_reset_context_clears_context(self):
+    def test_reset_context_remove_user_content(self):
         executor = PythonCodeExecution()
-        globals_len = len(executor.globals_ns)
         executor.execute("x = 1")
-        self.assertTrue(globals_len + 1, len(executor.globals_ns))
+        self.assertTrue('x' in executor.globals_ns)
         executor.reset_context()
-        self.assertEqual(0, len(executor.globals_ns))
 
-    def test_startup_code_not_executed_by_default(self):
-        executor = PythonCodeExecution(startup_code="x=100")
         self.assertFalse('x' in executor.globals_ns)
+        self.assertTrue('__builtins__' in executor.globals_ns)
 
     # ---------------------------------------------------------------------------
     # Successful execution tests
@@ -84,10 +72,10 @@ class PythonCodeExecutionTest(unittest.TestCase):
         self.assertTrue('__file__' in executor.globals_ns)
         self.assertEqual(test_filename, executor.globals_ns['__file__'])
 
-    def test_empty_filename_does_not_set__file__attr(self):
+    def test_empty_filename_sets_identifier(self):
         executor = PythonCodeExecution()
         executor.execute('x=1')
-        self.assertTrue('__file__' not in executor.globals_ns)
+        self.assertTrue('__file__' in executor.globals_ns)
 
     def test_execute_async_calls_success_signal_on_completion(self):
         code = "x=1+2"
@@ -131,6 +119,8 @@ class PythonCodeExecutionTest(unittest.TestCase):
         executor.execute(code)
         self.assertEqual("This should have no brackets\n", mock_stdout.getvalue())
 
+    @unittest.skipIf(sys.version_info < (3,),
+                     "Unable to get this working on Python 2 and we are closing to dropping it")
     @patch('sys.stdout', new_callable=StringIO)
     def test_scripts_can_print_unicode_if_unicode_literals_imported(self, mock_stdout):
         code = ("from __future__ import unicode_literals\n"
@@ -171,7 +161,7 @@ def foo():
     bar()
 foo()
 """
-        executor, recv = self._run_async_code(code, with_progress=True)
+        executor, recv = self._run_async_code(code)
         self.assertFalse(recv.success_cb_called)
         self.assertTrue(recv.error_cb_called)
         self.assertTrue(isinstance(recv.task_exc, NameError),
@@ -184,68 +174,6 @@ foo()
         self.assertEqual(7, recv.error_stack[3][1])
         self.assertEqual(5, recv.error_stack[4][1])
 
-    # ---------------------------------------------------------------------------
-    # Progress tests
-    # ---------------------------------------------------------------------------
-    def test_progress_cb_is_not_called_for_empty_string(self):
-        code = ""
-        executor, recv = self._run_async_code(code, with_progress=True)
-        self.assertEqual(0, len(recv.lines_received))
-
-    def test_progress_cb_is_not_called_for_code_with_syntax_errors(self):
-        code = """x = 1
-y =
-"""
-        executor, recv = self._run_async_code(code, with_progress=True)
-        self.assertEqual(0, len(recv.lines_received))
-        self.assertFalse(recv.success_cb_called)
-        self.assertTrue(recv.error_cb_called)
-        self.assertEqual(0, len(recv.lines_received))
-
-    def test_progress_cb_is_called_for_single_line(self):
-        code = "x = 1"
-        executor, recv = self._run_async_code(code, with_progress=True)
-        if not recv.success_cb_called:
-            self.assertTrue(recv.error_cb_called)
-            self.fail("Execution failed with error:\n" + str(recv.task_exc))
-
-        self.assertEqual([1], recv.lines_received)
-
-    def test_progress_cb_is_called_for_multiple_single_lines(self):
-        code = """x = 1
-y = 2
-"""
-        executor, recv = self._run_async_code(code, with_progress=True)
-        if not recv.success_cb_called:
-            self.assertTrue(recv.error_cb_called)
-            self.fail("Execution failed with error:\n" + str(recv.task_exc))
-
-        self.assertEqual([1, 2], recv.lines_received)
-
-    def test_progress_cb_is_called_for_mix_single_lines_and_blocks(self):
-        code = """x = 1
-# comment line
-
-sum = 0
-for i in range(10):
-    if i %2 == 0:
-        sum += i
-
-squared = sum*sum
-"""
-        executor, recv = self._run_async_code(code, with_progress=True)
-        if not recv.success_cb_called:
-            if recv.error_cb_called:
-                self.fail("Unexpected error found: " + str(recv.task_exc))
-            else:
-                self.fail("No callback was called!")
-
-        context = executor.globals_ns
-        self.assertEqual(20, context['sum'])
-        self.assertEqual(20*20, context['squared'])
-        self.assertEqual(1, context['x'])
-        self.assertEqual([1, 2, 3, 4, 9], recv.lines_received)
-
     # -------------------------------------------------------------------------
     # Filename checks
     # -------------------------------------------------------------------------
@@ -274,13 +202,9 @@ squared = sum*sum
         executor = PythonCodeExecution()
         self.assertRaises(expected_exc_type, executor.execute, code)
 
-    def _run_async_code(self, code, with_progress=False, filename=None):
+    def _run_async_code(self, code, filename=None):
         executor = PythonCodeExecution()
-        if with_progress:
-            recv = ReceiverWithProgress()
-            executor.sig_exec_progress.connect(recv.on_progess_update)
-        else:
-            recv = Receiver()
+        recv = Receiver()
         executor.sig_exec_success.connect(recv.on_success)
         executor.sig_exec_error.connect(recv.on_error)
         task = executor.execute_async(code, filename)

qt/widgets/common/CMakeLists.txt

@@ -132,6 +132,7 @@ set(
   src/QtPropertyBrowser/StringDialogEditor.cpp
   src/QtPropertyBrowser/StringEditorFactory.cpp
   src/QtPropertyBrowser/WorkspaceEditorFactory.cpp
+  src/Python/CodeExecution.cpp
   src/Python/Sip.cpp
   src/Python/QHashToDict.cpp
 )
@@ -284,6 +285,7 @@ set(QT5_INC_FILES
     inc/MantidQtWidgets/Common/WorkspacePresenter/WorkspaceProvider.h
     inc/MantidQtWidgets/Common/WorkspacePresenter/WorkspaceProviderNotifiable.h
     inc/MantidQtWidgets/Common/AlgorithmProgress/AlgorithmProgressModel.h
+    inc/MantidQtWidgets/Common/Python/CodeExecution.h
     inc/MantidQtWidgets/Common/Python/Sip.h
     inc/MantidQtWidgets/Common/Python/Object.h
     inc/MantidQtWidgets/Common/Python/QHashToDict.h)
@@ -903,6 +905,7 @@ mtd_add_qt_library(
   SYSTEM_INCLUDE_DIRS ${Boost_INCLUDE_DIRS}
   LINK_LIBS
     ${TARGET_LIBRARIES}
+    PythonInterfaceCore
     ${Boost_LIBRARIES}
     ${PYTHON_LIBRARIES}
   QT5_LINK_LIBS

qt/widgets/common/inc/MantidQtWidgets/Common/Python/CodeExecution.h

+// Mantid Repository : https://github.com/mantidproject/mantid
+//
+// Copyright © 2020 ISIS Rutherford Appleton Laboratory UKRI,
+//     NScD Oak Ridge National Laboratory, European Spallation Source
+//     & Institut Laue - Langevin
+// SPDX - License - Identifier: GPL - 3.0 +
+#ifndef MANTIDQTWIDGETS_LINETRACKINGEXECUTOR_H
+#define MANTIDQTWIDGETS_LINETRACKINGEXECUTOR_H
+
+#include "MantidPythonInterface/core/WrapPython.h"
+#include "MantidQtWidgets/Common/DllOption.h"
+#include <QString>
+
+class ScriptEditor;
+
+namespace MantidQt::Widgets::Common::Python {
+
+/**
+ * The CodeExecution class support execution of arbitrary Python code
+ * with the option to install a trace handler to track lines executed and
+ * tell an editor to mark them appropriately.
+ */
+class EXPORT_OPT_MANTIDQT_COMMON CodeExecution {
+public:
+  CodeExecution(ScriptEditor *editor);
+  PyObject *execute(const QString &codeStr, const QString &filename, int flags,
+                    PyObject *globals) const;
+
+private:
+  ScriptEditor *m_editor{nullptr};
+};
+
+} // namespace MantidQt::Widgets::Common::Python
+
+#endif // MANTIDQTWIDGETS_LINETRACKINGEXECUTOR_H

qt/widgets/common/inc/MantidQtWidgets/Common/ScriptEditor.h

@@ -123,6 +123,8 @@ public slots:
   void padMargin();
   /// Set the marker state
   void setMarkerState(bool enabled);
+  /// Update the progress marker potentially from a separate thread
+  void updateProgressMarkerFromThread(int lineno, bool error = false);
   /// Update the progress marker
   void updateProgressMarker(int lineno, bool error = false);
   /// Mark the progress arrow as an error

qt/widgets/common/src/Python/CodeExecution.cpp

+// Mantid Repository : https://github.com/mantidproject/mantid
+//
+// Copyright © 2020 ISIS Rutherford Appleton Laboratory UKRI,
+//     NScD Oak Ridge National Laboratory, European Spallation Source
+//     & Institut Laue - Langevin
+// SPDX - License - Identifier: GPL - 3.0 +
+#include "MantidQtWidgets/Common/Python/CodeExecution.h"
+#include "MantidPythonInterface/core/GlobalInterpreterLock.h"
+#include "MantidPythonInterface/core/VersionCompat.h"
+#include "MantidQtWidgets/Common/ScriptEditor.h"
+#include <QHash>
+#include <QString>
+#include <frameobject.h>
+
+using Mantid::PythonInterface::GlobalInterpreterLock;
+
+namespace {
+
+// Map co_filename objects from PyCodeObject to an editor object
+QHash<PyObject *, ScriptEditor *> EDITOR_LOOKUP;
+
+/**
+ * A callback, set using PyEval_SetTrace, that is called by Python
+ * to allow inspection into the current execution frame. It is currently
+ * used to emit the line number of the frame that is being executed.
+ * @param obj :: A reference to the object passed as the second argument
+ * of PyEval_SetTrace. Assumed nullptr and unused
+ * @param frame :: A reference to the current frame object
+ * @param event :: An integer defining the event type, see
+ * http://docs.python.org/c-api/init.html#profiling-and-tracing
+ * @param arg :: Meaning varies depending on event type, see
+ * http://docs.python.org/c-api/init.html#profiling-and-tracing
+ */
+int traceLineNumber(PyObject *obj, PyFrameObject *frame, int event,
+                    PyObject *arg) {
+  Q_UNUSED(obj);
+  Q_UNUSED(arg);
+  if (event != PyTrace_LINE)
+    return 0;
+  auto iter = EDITOR_LOOKUP.constFind(frame->f_code->co_filename);
+  if (iter != EDITOR_LOOKUP.constEnd()) {
+    iter.value()->updateProgressMarkerFromThread(frame->f_lineno, false);
+  }
+  return 0;
+}
+} // namespace
+
+namespace MantidQt::Widgets::Common::Python {
+
+/**
+ * Construct a LineTrackingExecutor for a given editor
+ * @param editor A pointer to an editor. Can be nullptr. Disables progress
+ * tracking.
+ */
+CodeExecution::CodeExecution(ScriptEditor *editor) : m_editor(editor) {}
+
+/**
+ * Execute the code string from the given filename and return the result
+ * @param codeStr A string containing the source code
+ * @param filename A string containing the filename of the source code
+ * @param flags An OR-ed combination of compiler flags
+ * @param globals A dictionary containing the current globals mapping
+ */
+PyObject *CodeExecution::execute(const QString &codeStr,
+                                 const QString &filename, int flags,
+                                 PyObject *globals) const {
+  GlobalInterpreterLock gil;
+  PyCompilerFlags compileFlags;
+  compileFlags.cf_flags = flags;
+  auto compiledCode = Py_CompileStringFlags(codeStr.toUtf8().constData(),
+                                            filename.toUtf8().constData(),
+                                            Py_file_input, &compileFlags);
+  if (compiledCode) {
+    if (m_editor) {
+      const auto coFileObject = ((PyCodeObject *)compiledCode)->co_filename;
+      const auto posIter = EDITOR_LOOKUP.insert(coFileObject, m_editor);
+      PyEval_SetTrace((Py_tracefunc)&traceLineNumber, nullptr);
+      const auto result =
+          PyEval_EvalCode(CODE_OBJECT(compiledCode), globals, globals);
+      PyEval_SetTrace(nullptr, nullptr);
+      EDITOR_LOOKUP.erase(posIter);
+      return result;
+    } else {
+      return PyEval_EvalCode(CODE_OBJECT(compiledCode), globals, globals);
+    }
+  } else {
+    return nullptr;
+  }
+}
+
+} // namespace MantidQt::Widgets::Common::Python

qt/widgets/common/src/ScriptEditor.cpp

@@ -29,6 +29,7 @@
 #include <QSettings>
 #include <QShortcut>
 #include <QTextStream>
+#include <QThread>
 
 // Qscintilla
 #include <Qsci/qsciapis.h>
@@ -378,6 +379,23 @@ void ScriptEditor::setMarkerState(bool enabled) {
   }
 }
 
+/**
+ * Update the arrow marker to point to the correct line and colour it
+ * depending on the error state. If the call is from a thread other than the
+ * application thread then the call is reperformed on that thread
+ * @param lineno :: The line to place the marker at. A negative number will
+ * clear all markers
+ * @param error :: If true, the marker will turn red
+ */
+void ScriptEditor::updateProgressMarkerFromThread(int lineno, bool error) {
+  if (QThread::currentThread() != QApplication::instance()->thread()) {
+    QMetaObject::invokeMethod(this, "updateProgressMarker", Qt::AutoConnection,
+                              Q_ARG(int, lineno), Q_ARG(bool, error));
+  } else {
+    updateProgressMarker(lineno, error);
+  }
+}
+
 /**
  * Update the arrow marker to point to the correct line and colour it
  * depending on the error state
@@ -386,6 +404,7 @@ void ScriptEditor::setMarkerState(bool enabled) {
  * @param error :: If true, the marker will turn red
  */
 void ScriptEditor::updateProgressMarker(int lineno, bool error) {
+
   m_currentExecLine = lineno;
   if (error) {
     setMarkerBackgroundColor(g_error_colour, m_progressArrowKey);