From 30c7001522ee2c0cb4be4a975355c42a395670ac Mon Sep 17 00:00:00 2001
From: Robert Applin <40830825+robertapplin@users.noreply.github.com>
Date: Tue, 4 Jun 2019 15:06:37 +0100
Subject: [PATCH] Refs #23476. Previewplot point data and errors on Muon ALC

---
 .../Muon/ALCDataLoadingView.cpp               |  8 ++-
 qt/widgets/common/CMakeLists.txt              |  3 +-
 .../Common/Python/QHashToDict.h               | 58 ++----------------
 qt/widgets/common/src/Python/QHashToDict.cpp  | 60 +++++++++++++++++++
 .../inc/MantidQtWidgets/MplCpp/MantidAxes.h   | 16 +++--
 qt/widgets/mplcpp/src/MantidAxes.cpp          | 27 ++++++---
 .../Plotting/Mpl/PreviewPlot.h                | 16 ++---
 .../Plotting/Qwt/PreviewPlot.h                | 16 ++---
 qt/widgets/plotting/src/Mpl/PreviewPlot.cpp   | 20 +++++--
 qt/widgets/plotting/src/Qwt/PreviewPlot.cpp   | 13 +++-
 10 files changed, 147 insertions(+), 90 deletions(-)
 create mode 100644 qt/widgets/common/src/Python/QHashToDict.cpp

diff --git a/qt/scientific_interfaces/Muon/ALCDataLoadingView.cpp b/qt/scientific_interfaces/Muon/ALCDataLoadingView.cpp
index 162bc66e149..09198b96a71 100644
--- a/qt/scientific_interfaces/Muon/ALCDataLoadingView.cpp
+++ b/qt/scientific_interfaces/Muon/ALCDataLoadingView.cpp
@@ -146,8 +146,14 @@ ALCDataLoadingView::timeRange() const {
 
 void ALCDataLoadingView::setDataCurve(MatrixWorkspace_sptr &workspace,
                                       std::size_t const &workspaceIndex) {
+  // These kwargs ensure only the data points are plotted with no line
+  QHash<QString, QVariant> kwargs;
+  kwargs.insert("linestyle", QString("None").toLatin1().constData());
+  kwargs.insert("marker", QString(".").toLatin1().constData());
+
   m_ui.dataPlot->clear();
-  m_ui.dataPlot->addSpectrum("Data", workspace, workspaceIndex, Qt::black);
+  m_ui.dataPlot->addSpectrum("Data", workspace, workspaceIndex, Qt::black, true,
+                             kwargs);
 }
 
 void ALCDataLoadingView::displayError(const std::string &error) {
diff --git a/qt/widgets/common/CMakeLists.txt b/qt/widgets/common/CMakeLists.txt
index 54459fc39ba..ed3006bbee5 100644
--- a/qt/widgets/common/CMakeLists.txt
+++ b/qt/widgets/common/CMakeLists.txt
@@ -119,7 +119,8 @@ set(QT5_SRC_FILES
     src/QtPropertyBrowser/StringDialogEditor.cpp
     src/QtPropertyBrowser/StringEditorFactory.cpp
     src/QtPropertyBrowser/WorkspaceEditorFactory.cpp
-    src/Python/Sip.cpp)
+    src/Python/Sip.cpp
+    src/Python/QHashToDict.cpp)
 
 set(
   QT5_MOC_FILES
diff --git a/qt/widgets/common/inc/MantidQtWidgets/Common/Python/QHashToDict.h b/qt/widgets/common/inc/MantidQtWidgets/Common/Python/QHashToDict.h
index 866eec9850c..4e2170e9652 100644
--- a/qt/widgets/common/inc/MantidQtWidgets/Common/Python/QHashToDict.h
+++ b/qt/widgets/common/inc/MantidQtWidgets/Common/Python/QHashToDict.h
@@ -4,14 +4,13 @@
 //     NScD Oak Ridge National Laboratory, European Spallation Source
 //     & Institut Laue - Langevin
 // SPDX - License - Identifier: GPL - 3.0 +
-
 #ifndef MANTIDQTWIDGETS_QHASHTODICT_H
 #define MANTIDQTWIDGETS_QHASHTODICT_H
-
+#include "MantidQtWidgets/Common/DllOption.h"
 #include "MantidQtWidgets/Common/Python/Object.h"
-#include "MantidQtWidgets/Common/Python/Sip.h"
 
 #include <QHash>
+#include <QVariant>
 
 namespace MantidQt {
 namespace Widgets {
@@ -20,60 +19,11 @@ namespace Python {
 
 using KwArgs = QHash<QString, QVariant>;
 
-Python::Object qHashToDict(const KwArgs &hash) {
-  auto *d = PyDict_New();
-
-  if (!d)
-    return Python::Object();
-
-  auto it = hash.constBegin();
-  auto end = hash.constEnd();
-  auto sipAPI = Detail::sipAPI();
-
-  while (it != end) {
-    auto *k = new KwArgs::key_type(it.key());
-    auto *kobj = sipAPI->api_convert_from_new_type(
-        k, sipAPI->api_find_type("QString"), Py_None);
-
-    if (!kobj) {
-      delete k;
-      Py_DECREF(d);
-
-      return Python::Object();
-    }
-
-    auto *v = new KwArgs::mapped_type(it.value());
-    auto *vobj = sipAPI->api_convert_from_new_type(
-        v, sipAPI->api_find_type("QVariant"), Py_None);
-
-    if (!vobj) {
-      delete v;
-      Py_DECREF(kobj);
-      Py_DECREF(d);
-
-      return Python::Object();
-    }
-
-    auto rc = PyDict_SetItem(d, kobj, vobj);
-
-    Py_DECREF(vobj);
-    Py_DECREF(kobj);
-
-    if (rc < 0) {
-      Py_DECREF(d);
-
-      return Python::Object();
-    }
-
-    ++it;
-  }
-
-  return Python::NewRef(d);
-}
+EXPORT_OPT_MANTIDQT_COMMON Python::Dict qHashToDict(const KwArgs &hash);
 
 } // namespace Python
 } // namespace Common
 } // namespace Widgets
 } // namespace MantidQt
 
-#endif /* MPLCPP_QHASHTODICT_H */
\ No newline at end of file
+#endif /* MANTIDQTWIDGETS_QHASHTODICT_H */
diff --git a/qt/widgets/common/src/Python/QHashToDict.cpp b/qt/widgets/common/src/Python/QHashToDict.cpp
new file mode 100644
index 00000000000..f978300423c
--- /dev/null
+++ b/qt/widgets/common/src/Python/QHashToDict.cpp
@@ -0,0 +1,60 @@
+// Mantid Repository : https://github.com/mantidproject/mantid
+//
+// Copyright &copy; 2019 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/QHashToDict.h"
+#include "MantidQtWidgets/Common/Python/Sip.h"
+
+namespace MantidQt {
+namespace Widgets {
+namespace Common {
+namespace Python {
+
+Python::Dict qHashToDict(const KwArgs &hash) {
+  auto d = Python::Dict();
+
+  auto it = hash.constBegin();
+  auto end = hash.constEnd();
+  auto sipAPI = Detail::sipAPI();
+
+  while (it != end) {
+    auto *k = new KwArgs::key_type(it.key());
+    auto *kobj = sipAPI->api_convert_from_new_type(
+        k, sipAPI->api_find_type("QString"), Py_None);
+
+    if (!kobj) {
+      delete k;
+      return Python::Dict();
+    }
+
+    auto *v = new KwArgs::mapped_type(it.value());
+    auto *vobj = sipAPI->api_convert_from_new_type(
+        v, sipAPI->api_find_type("QVariant"), Py_None);
+
+    if (!vobj) {
+      delete v;
+      Py_DECREF(kobj);
+      return Python::Dict();
+    }
+
+    auto rc = PyDict_SetItem(d.ptr(), kobj, vobj);
+
+    Py_DECREF(vobj);
+    Py_DECREF(kobj);
+
+    if (rc < 0) {
+      return Python::Dict();
+    }
+
+    ++it;
+  }
+
+  return d;
+}
+
+} // namespace Python
+} // namespace Common
+} // namespace Widgets
+} // namespace MantidQt
diff --git a/qt/widgets/mplcpp/inc/MantidQtWidgets/MplCpp/MantidAxes.h b/qt/widgets/mplcpp/inc/MantidQtWidgets/MplCpp/MantidAxes.h
index 72a5c7e4e14..c034c70bddd 100644
--- a/qt/widgets/mplcpp/inc/MantidQtWidgets/MplCpp/MantidAxes.h
+++ b/qt/widgets/mplcpp/inc/MantidQtWidgets/MplCpp/MantidAxes.h
@@ -11,6 +11,11 @@
 #include "MantidQtWidgets/MplCpp/Axes.h"
 #include "MantidQtWidgets/MplCpp/ErrorbarContainer.h"
 
+#include <boost/optional.hpp>
+
+#include <QHash>
+#include <QVariant>
+
 namespace MantidQt {
 namespace Widgets {
 namespace MplCpp {
@@ -26,10 +31,13 @@ public:
   ///@{
   Line2D plot(const Mantid::API::MatrixWorkspace_sptr &workspace,
               const size_t wkspIndex, const QString lineColour,
-              const QString label);
-  ErrorbarContainer errorbar(const Mantid::API::MatrixWorkspace_sptr &workspace,
-                             const size_t wkspIndex, const QString lineColour,
-                             const QString label);
+              const QString label,
+              const boost::optional<QHash<QString, QVariant>> &otherKwargs);
+  ErrorbarContainer
+  errorbar(const Mantid::API::MatrixWorkspace_sptr &workspace,
+           const size_t wkspIndex, const QString lineColour,
+           const QString label,
+           const boost::optional<QHash<QString, QVariant>> &otherKwargs);
   ///@}
 
   /// @name Artist removal/replacement
diff --git a/qt/widgets/mplcpp/src/MantidAxes.cpp b/qt/widgets/mplcpp/src/MantidAxes.cpp
index 111b5919da0..554c9b79dcb 100644
--- a/qt/widgets/mplcpp/src/MantidAxes.cpp
+++ b/qt/widgets/mplcpp/src/MantidAxes.cpp
@@ -6,6 +6,8 @@
 // SPDX - License - Identifier: GPL - 3.0 +
 #include "MantidQtWidgets/MplCpp/MantidAxes.h"
 
+#include "MantidQtWidgets/Common/Python/QHashToDict.h"
+
 using Mantid::API::MatrixWorkspace_sptr;
 using Mantid::PythonInterface::GlobalInterpreterLock;
 
@@ -27,18 +29,23 @@ MantidAxes::MantidAxes(Python::Object pyObj) : Axes{std::move(pyObj)} {}
  * @param wkspIndex The workspace index to plot
  * @param lineColour Set the line colour to this string name
  * @param label A label for the curve
+ * @param otherKwargs Other kwargs to use for the line
  * @return A new Line2D artist object
  */
-Line2D MantidAxes::plot(const Mantid::API::MatrixWorkspace_sptr &workspace,
-                        const size_t wkspIndex, const QString lineColour,
-                        const QString label) {
+Line2D
+MantidAxes::plot(const Mantid::API::MatrixWorkspace_sptr &workspace,
+                 const size_t wkspIndex, const QString lineColour,
+                 const QString label,
+                 const boost::optional<QHash<QString, QVariant>> &otherKwargs) {
   GlobalInterpreterLock lock;
   const auto wksp = Python::NewRef(MatrixWorkpaceToPython()(workspace));
   const auto args = Python::NewRef(Py_BuildValue("(O)", wksp.ptr()));
-  Python::Dict kwargs;
+
+  Python::Dict kwargs = Python::qHashToDict(otherKwargs.get());
   kwargs["wkspIndex"] = wkspIndex;
   kwargs["color"] = lineColour.toLatin1().constData();
   kwargs["label"] = label.toLatin1().constData();
+
   return Line2D{pyobj().attr("plot")(*args, **kwargs)[0]};
 }
 
@@ -50,17 +57,19 @@ Line2D MantidAxes::plot(const Mantid::API::MatrixWorkspace_sptr &workspace,
  * @param label A label for the curve
  * @return A new ErrorbarContainer object
  */
-ErrorbarContainer
-MantidAxes::errorbar(const Mantid::API::MatrixWorkspace_sptr &workspace,
-                     const size_t wkspIndex, const QString lineColour,
-                     const QString label) {
+ErrorbarContainer MantidAxes::errorbar(
+    const Mantid::API::MatrixWorkspace_sptr &workspace, const size_t wkspIndex,
+    const QString lineColour, const QString label,
+    const boost::optional<QHash<QString, QVariant>> &otherKwargs) {
   GlobalInterpreterLock lock;
   const auto wksp = Python::NewRef(MatrixWorkpaceToPython()(workspace));
   const auto args = Python::NewRef(Py_BuildValue("(O)", wksp.ptr()));
-  Python::Dict kwargs;
+
+  Python::Dict kwargs = Python::qHashToDict(otherKwargs.get());
   kwargs["wkspIndex"] = wkspIndex;
   kwargs["color"] = lineColour.toLatin1().constData();
   kwargs["label"] = label.toLatin1().constData();
+
   return ErrorbarContainer{pyobj().attr("errorbar")(*args, **kwargs)};
 }
 
diff --git a/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Mpl/PreviewPlot.h b/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Mpl/PreviewPlot.h
index 695c06461a4..d0d1c0717e6 100644
--- a/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Mpl/PreviewPlot.h
+++ b/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Mpl/PreviewPlot.h
@@ -47,13 +47,15 @@ public:
   ~PreviewPlot();
 
   void watchADS(bool on);
-  void addSpectrum(const QString &lineLabel,
-                   const Mantid::API::MatrixWorkspace_sptr &ws,
-                   const size_t wsIndex = 0,
-                   const QColor &lineColour = QColor());
-  void addSpectrum(const QString &lineName, const QString &wsName,
-                   const size_t wsIndex = 0,
-                   const QColor &lineColour = QColor());
+  void addSpectrum(
+      const QString &lineLabel, const Mantid::API::MatrixWorkspace_sptr &ws,
+      const size_t wsIndex = 0, const QColor &lineColour = QColor(),
+      const bool errorBars = false,
+      const QHash<QString, QVariant> &plotKwargs = QHash<QString, QVariant>());
+  void addSpectrum(
+      const QString &lineName, const QString &wsName, const size_t wsIndex = 0,
+      const QColor &lineColour = QColor(), const bool errorBars = false,
+      const QHash<QString, QVariant> &plotKwargs = QHash<QString, QVariant>());
   void removeSpectrum(const QString &lineName);
   void setAxisRange(const QPair<double, double> &range,
                     AxisID axisID = AxisID::XBottom);
diff --git a/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Qwt/PreviewPlot.h b/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Qwt/PreviewPlot.h
index 51e1c6533de..38b382b1861 100644
--- a/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Qwt/PreviewPlot.h
+++ b/qt/widgets/plotting/inc/MantidQtWidgets/Plotting/Qwt/PreviewPlot.h
@@ -71,13 +71,15 @@ public:
   getCurveRange(const Mantid::API::MatrixWorkspace_sptr ws);
   QPair<double, double> getCurveRange(const QString &curveName);
 
-  void addSpectrum(const QString &curveName,
-                   const Mantid::API::MatrixWorkspace_sptr &ws,
-                   const size_t wsIndex = 0,
-                   const QColor &curveColour = QColor());
-  void addSpectrum(const QString &curveName, const QString &wsName,
-                   const size_t wsIndex = 0,
-                   const QColor &curveColour = QColor());
+  void addSpectrum(
+      const QString &curveName, const Mantid::API::MatrixWorkspace_sptr &ws,
+      const size_t wsIndex = 0, const QColor &curveColour = QColor(),
+      const bool errorBars = false,
+      const QHash<QString, QVariant> &plotKwargs = QHash<QString, QVariant>());
+  void addSpectrum(
+      const QString &curveName, const QString &wsName, const size_t wsIndex = 0,
+      const QColor &curveColour = QColor(), const bool errorBars = false,
+      const QHash<QString, QVariant> &plotKwargs = QHash<QString, QVariant>());
 
   void removeSpectrum(const Mantid::API::MatrixWorkspace_sptr ws);
   void removeSpectrum(const QString &curveName);
diff --git a/qt/widgets/plotting/src/Mpl/PreviewPlot.cpp b/qt/widgets/plotting/src/Mpl/PreviewPlot.cpp
index c7c9401d8ab..09d739dcffb 100644
--- a/qt/widgets/plotting/src/Mpl/PreviewPlot.cpp
+++ b/qt/widgets/plotting/src/Mpl/PreviewPlot.cpp
@@ -94,7 +94,9 @@ void PreviewPlot::watchADS(bool on) {
  */
 void PreviewPlot::addSpectrum(const QString &lineName,
                               const Mantid::API::MatrixWorkspace_sptr &ws,
-                              const size_t wsIndex, const QColor &lineColour) {
+                              const size_t wsIndex, const QColor &lineColour,
+                              const bool errorBars,
+                              const QHash<QString, QVariant> &plotKwargs) {
   if (lineName.isEmpty()) {
     g_log.warning("Cannot plot with empty line name");
     return;
@@ -104,8 +106,16 @@ void PreviewPlot::addSpectrum(const QString &lineName,
     return;
   }
   removeSpectrum(lineName);
+
   auto axes = m_canvas->gca<MantidAxes>();
-  axes.plot(ws, wsIndex, lineColour.name(QColor::HexRgb), lineName);
+  if (!errorBars) {
+    axes.plot(ws, wsIndex, lineColour.name(QColor::HexRgb), lineName,
+              plotKwargs);
+  } else {
+    axes.errorbar(ws, wsIndex, lineColour.name(QColor::HexRgb), lineName,
+                  plotKwargs);
+  }
+
   regenerateLegend();
   axes.relim();
   m_canvas->draw();
@@ -119,11 +129,13 @@ void PreviewPlot::addSpectrum(const QString &lineName,
  * @param lineColour Defines the color of the line
  */
 void PreviewPlot::addSpectrum(const QString &lineName, const QString &wsName,
-                              const size_t wsIndex, const QColor &lineColour) {
+                              const size_t wsIndex, const QColor &lineColour,
+                              const bool errorBars,
+                              const QHash<QString, QVariant> &plotKwargs) {
   addSpectrum(lineName,
               AnalysisDataService::Instance().retrieveWS<MatrixWorkspace>(
                   wsName.toStdString()),
-              wsIndex, lineColour);
+              wsIndex, lineColour, errorBars, plotKwargs);
 }
 
 /**
diff --git a/qt/widgets/plotting/src/Qwt/PreviewPlot.cpp b/qt/widgets/plotting/src/Qwt/PreviewPlot.cpp
index 678bd669f28..9706490abcc 100644
--- a/qt/widgets/plotting/src/Qwt/PreviewPlot.cpp
+++ b/qt/widgets/plotting/src/Qwt/PreviewPlot.cpp
@@ -251,7 +251,12 @@ QPair<double, double> PreviewPlot::getCurveRange(const QString &curveName) {
  */
 void PreviewPlot::addSpectrum(const QString &curveName,
                               const MatrixWorkspace_sptr &ws,
-                              const size_t wsIndex, const QColor &curveColour) {
+                              const size_t wsIndex, const QColor &curveColour,
+                              const bool errorBars,
+                              const QHash<QString, QVariant> &plotKwargs) {
+  UNUSED_ARG(errorBars);
+  UNUSED_ARG(plotKwargs);
+
   if (curveName.isEmpty()) {
     g_log.warning("Cannot plot with empty curve name");
     return;
@@ -307,7 +312,9 @@ void PreviewPlot::addSpectrum(const QString &curveName,
  * @param curveColour Colour of curve to plot
  */
 void PreviewPlot::addSpectrum(const QString &curveName, const QString &wsName,
-                              const size_t wsIndex, const QColor &curveColour) {
+                              const size_t wsIndex, const QColor &curveColour,
+                              const bool errorBars,
+                              const QHash<QString, QVariant> &plotKwargs) {
   if (wsName.isEmpty()) {
     g_log.error("Cannot plot with empty workspace name");
     return;
@@ -322,7 +329,7 @@ void PreviewPlot::addSpectrum(const QString &curveName, const QString &wsName,
     throw std::runtime_error(
         wsNameStr + " is not a MatrixWorkspace, not supported by PreviewPlot.");
 
-  addSpectrum(curveName, ws, wsIndex, curveColour);
+  addSpectrum(curveName, ws, wsIndex, curveColour, errorBars, plotKwargs);
 }
 
 /**
-- 
GitLab