From 2961a842bc31acf27e8540da9a01e447cc456ba0 Mon Sep 17 00:00:00 2001
From: Martyn Gigg <martyn.gigg@gmail.com>
Date: Fri, 25 Aug 2017 12:15:06 +0100
Subject: [PATCH] Allow PropertyManager creation directly from a Python dict

This allows a PropertyManager to be passed directly to the
PropertyManagerDataService without requiring an algorithm. In addition
a python dict can also be passed directly to the data service without
having to create a PropertyManager first.
---
 .../kernel/Registry/PropertyManagerFactory.h  | 42 ++++++++++++++++++
 .../mantid/kernel/CMakeLists.txt              |  2 +
 .../kernel/src/Exports/PropertyManager.cpp    | 19 ++++----
 .../Exports/PropertyManagerDataService.cpp    | 32 +++++++++++++-
 .../src/Registry/MappingTypeHandler.cpp       | 29 +------------
 .../src/Registry/PropertyManagerFactory.cpp   | 43 +++++++++++++++++++
 .../test/python/mantid/kernel/CMakeLists.txt  |  1 +
 .../kernel/PropertyManagerDataServiceTest.py  | 40 +++++++++++++++++
 .../mantid/kernel/PropertyManagerTest.py      | 18 +++++++-
 9 files changed, 187 insertions(+), 39 deletions(-)
 create mode 100644 Framework/PythonInterface/inc/MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h
 create mode 100644 Framework/PythonInterface/mantid/kernel/src/Registry/PropertyManagerFactory.cpp
 create mode 100644 Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerDataServiceTest.py

diff --git a/Framework/PythonInterface/inc/MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h b/Framework/PythonInterface/inc/MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h
new file mode 100644
index 00000000000..aac23853081
--- /dev/null
+++ b/Framework/PythonInterface/inc/MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h
@@ -0,0 +1,42 @@
+#ifndef MANTID_PYTHONINTERFACE_PROPERTYMANAGERFACTORY_H
+#define MANTID_PYTHONINTERFACE_PROPERTYMANAGERFACTORY_H
+/*
+  Copyright &copy; 2017 ISIS Rutherford Appleton Laboratory, NScD Oak Ridge
+  National Laboratory & European Spallation Source
+
+  This file is part of Mantid.
+
+  Mantid is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 3 of the License, or
+  (at your option) any later version.
+
+  Mantid is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+  File change history is stored at: <https://github.com/mantidproject/mantid>
+  Code Documentation is available at: <http://doxygen.mantidproject.org>
+*/
+#include <boost/shared_ptr.hpp>
+#include <boost/python/dict.hpp>
+
+namespace Mantid {
+namespace Kernel {
+class PropertyManager;
+}
+namespace PythonInterface {
+namespace Registry {
+
+/// Create a C++ PropertyMananager from a Python dictionary
+boost::shared_ptr<Kernel::PropertyManager>
+createPropertyManager(const boost::python::dict &mapping);
+}
+}
+}
+
+#endif // MANTID_PYTHONINTERFACE_PROPERTYMANAGERFACTORY_H
diff --git a/Framework/PythonInterface/mantid/kernel/CMakeLists.txt b/Framework/PythonInterface/mantid/kernel/CMakeLists.txt
index a1690bd727d..a53b47eb01d 100644
--- a/Framework/PythonInterface/mantid/kernel/CMakeLists.txt
+++ b/Framework/PythonInterface/mantid/kernel/CMakeLists.txt
@@ -75,6 +75,7 @@ set ( SRC_FILES
   src/Converters/PyObjectToVMD.cpp
   src/Converters/WrapWithNumpy.cpp
   src/Registry/MappingTypeHandler.cpp
+  src/Registry/PropertyManagerFactory.cpp
   src/Registry/PropertyWithValueFactory.cpp
   src/Registry/SequenceTypeHandler.cpp
   src/Registry/TypeRegistry.cpp
@@ -109,6 +110,7 @@ set ( INC_FILES
   ${HEADER_DIR}/kernel/Policies/VectorToNumpy.h
   ${HEADER_DIR}/kernel/Registry/MappingTypeHandler.h
   ${HEADER_DIR}/kernel/Registry/PropertyValueHandler.h
+  ${HEADER_DIR}/kernel/Registry/PropertyManagerFactory.h
   ${HEADER_DIR}/kernel/Registry/PropertyWithValueFactory.h
   ${HEADER_DIR}/kernel/Registry/SequenceTypeHandler.h
   ${HEADER_DIR}/kernel/Registry/TypedPropertyValueHandler.h
diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManager.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManager.cpp
index e801bcabc29..725aa4ee068 100644
--- a/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManager.cpp
+++ b/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManager.cpp
@@ -4,31 +4,32 @@
                                 // design
 #endif
 #include "MantidPythonInterface/kernel/GetPointer.h"
+#include "MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h"
 #include "MantidKernel/IPropertyManager.h"
 #include "MantidKernel/PropertyManager.h"
 
 #include <boost/python/class.hpp>
+#include <boost/python/make_constructor.hpp>
 
+using Mantid::PythonInterface::Registry::createPropertyManager;
 using Mantid::Kernel::IPropertyManager;
 using Mantid::Kernel::PropertyManager;
+using Mantid::Kernel::PropertyManager_sptr;
 
 using namespace boost::python;
 
 GET_POINTER_SPECIALIZATION(PropertyManager)
 
 void export_PropertyManager() {
-  typedef boost::shared_ptr<PropertyManager> PropertyManager_sptr;
 
   // The second argument defines the actual type held within the Python object.
-  // This means that when a PropertyManager is constructed in Python it actually
-  // used
-  // a shared_ptr to the object rather than a raw pointer. This knowledge is
-  // used by
-  // DataServiceExporter::extractCppValue to assume that it can always extract a
-  // shared_ptr
-  // type
+  // This means that when a PropertyManager is constructed in Python
+  // it actually used a shared_ptr to the object rather than a raw pointer.
+  // This knowledge is used by DataServiceExporter::extractCppValue to assume
+  // that it can always extract a shared_ptr type
   class_<PropertyManager, PropertyManager_sptr, bases<IPropertyManager>,
-         boost::noncopyable>("PropertyManager");
+         boost::noncopyable>("PropertyManager")
+      .def("__init__", make_constructor(&createPropertyManager));
 }
 
 #ifdef _MSC_VER
diff --git a/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManagerDataService.cpp b/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManagerDataService.cpp
index 7113e286462..be80c22864f 100644
--- a/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManagerDataService.cpp
+++ b/Framework/PythonInterface/mantid/kernel/src/Exports/PropertyManagerDataService.cpp
@@ -1,5 +1,6 @@
 #include "MantidPythonInterface/kernel/GetPointer.h"
 #include "MantidPythonInterface/kernel/DataServiceExporter.h"
+#include "MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h"
 
 #include "MantidKernel/PropertyManagerDataService.h"
 #include "MantidKernel/PropertyManager.h"
@@ -10,11 +11,37 @@
 using namespace Mantid::API;
 using namespace Mantid::Kernel;
 using Mantid::PythonInterface::DataServiceExporter;
+using Mantid::PythonInterface::Registry::createPropertyManager;
 using namespace boost::python;
 
 /// Weak pointer to DataItem typedef
 typedef boost::weak_ptr<PropertyManager> PropertyManager_wptr;
 
+namespace {
+/**
+ * Add a dictionary to the data service directly. It creates a PropertyManager
+ * on the way in.
+ * @param self A reference to the PropertyManagerDataService
+ * @param name The name of the object
+ * @param mapping A dict object
+ */
+void addFromDict(PropertyManagerDataServiceImpl &self, const std::string &name,
+                 const dict &mapping) {
+  self.add(name, createPropertyManager(mapping));
+}
+/**
+ * Add or replace a dictionary to the data service directly. It creates
+ * a PropertyManager on the way in.
+ * @param self A reference to the PropertyManagerDataService
+ * @param name The name of the object
+ * @param mapping A dict object
+ */
+void addOrReplaceFromDict(PropertyManagerDataServiceImpl &self,
+                          const std::string &name, const dict &mapping) {
+  self.addOrReplace(name, createPropertyManager(mapping));
+}
+}
+
 GET_POINTER_SPECIALIZATION(PropertyManagerDataServiceImpl)
 
 void export_PropertyManagerDataService() {
@@ -28,5 +55,8 @@ void export_PropertyManagerDataService() {
   pmdType.def("Instance", &PropertyManagerDataService::Instance,
               return_value_policy<reference_existing_object>(),
               "Return a reference to the singleton instance")
-      .staticmethod("Instance");
+      .staticmethod("Instance")
+      // adds an overload from a dictionary
+      .def("add", &addFromDict)
+      .def("addOrReplace", &addOrReplaceFromDict);
 }
diff --git a/Framework/PythonInterface/mantid/kernel/src/Registry/MappingTypeHandler.cpp b/Framework/PythonInterface/mantid/kernel/src/Registry/MappingTypeHandler.cpp
index 4fb274e8d53..0ef7925eae5 100644
--- a/Framework/PythonInterface/mantid/kernel/src/Registry/MappingTypeHandler.cpp
+++ b/Framework/PythonInterface/mantid/kernel/src/Registry/MappingTypeHandler.cpp
@@ -1,16 +1,14 @@
 #include "MantidPythonInterface/kernel/Registry/MappingTypeHandler.h"
+#include "MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h"
 #include "MantidPythonInterface/kernel/Registry/PropertyWithValueFactory.h"
 
 #include "MantidKernel/PropertyManager.h"
 #include "MantidKernel/PropertyManagerProperty.h"
 #include "MantidKernel/PropertyWithValue.h"
 
-#include <boost/make_shared.hpp>
 #include <boost/python/dict.hpp>
-#include <boost/python/extract.hpp>
 
 using boost::python::dict;
-using boost::python::extract;
 using boost::python::handle;
 using boost::python::len;
 using boost::python::object;
@@ -23,31 +21,6 @@ using Kernel::PropertyWithValue;
 namespace PythonInterface {
 namespace Registry {
 
-namespace {
-/**
- * Create a new PropertyManager from the given dict
- * @param mapping A wrapper around a Python dict instance
- * @return A shared_ptr to a new PropertyManager
- */
-PropertyManager_sptr createPropertyManager(const dict &mapping) {
-  auto pmgr = boost::make_shared<PropertyManager>();
-#if PY_MAJOR_VERSION >= 3
-  object view(mapping.attr("items")());
-  object itemIter(handle<>(PyObject_GetIter(view.ptr())));
-#else
-  object itemIter(mapping.attr("iteritems")());
-#endif
-  auto length = len(mapping);
-  for (ssize_t i = 0; i < length; ++i) {
-    const object keyValue(handle<>(PyIter_Next(itemIter.ptr())));
-    const std::string cppkey = extract<std::string>(keyValue[0])();
-    pmgr->declareProperty(PropertyWithValueFactory::create(cppkey, keyValue[1],
-                                                           Direction::Input));
-  }
-  return pmgr;
-}
-}
-
 /**
  * Sets the named property in the PropertyManager by extracting a new
  * PropertyManager from the Python object
diff --git a/Framework/PythonInterface/mantid/kernel/src/Registry/PropertyManagerFactory.cpp b/Framework/PythonInterface/mantid/kernel/src/Registry/PropertyManagerFactory.cpp
new file mode 100644
index 00000000000..7fffa286d4a
--- /dev/null
+++ b/Framework/PythonInterface/mantid/kernel/src/Registry/PropertyManagerFactory.cpp
@@ -0,0 +1,43 @@
+#include "MantidPythonInterface/kernel/Registry/PropertyManagerFactory.h"
+#include "MantidPythonInterface/kernel/Registry/PropertyWithValueFactory.h"
+#include "MantidKernel/PropertyManager.h"
+
+#include <boost/make_shared.hpp>
+#include <boost/python/extract.hpp>
+
+using boost::python::extract;
+using boost::python::handle;
+using boost::python::object;
+
+namespace Mantid {
+using Kernel::Direction;
+using Kernel::PropertyManager;
+
+namespace PythonInterface {
+namespace Registry {
+
+/**
+ * @param mapping A Python dictionary instance
+ * @return A new C++ PropertyManager instance
+ */
+boost::shared_ptr<Kernel::PropertyManager>
+createPropertyManager(const boost::python::dict &mapping) {
+  auto pmgr = boost::make_shared<PropertyManager>();
+#if PY_MAJOR_VERSION >= 3
+  object view(mapping.attr("items")());
+  object itemIter(handle<>(PyObject_GetIter(view.ptr())));
+#else
+  object itemIter(mapping.attr("iteritems")());
+#endif
+  auto length = len(mapping);
+  for (ssize_t i = 0; i < length; ++i) {
+    const object keyValue(handle<>(PyIter_Next(itemIter.ptr())));
+    const std::string cppkey = extract<std::string>(keyValue[0])();
+    pmgr->declareProperty(PropertyWithValueFactory::create(cppkey, keyValue[1],
+                                                           Direction::Input));
+  }
+  return pmgr;
+}
+}
+}
+}
diff --git a/Framework/PythonInterface/test/python/mantid/kernel/CMakeLists.txt b/Framework/PythonInterface/test/python/mantid/kernel/CMakeLists.txt
index 8fe8c0eda2c..a62aac820a5 100644
--- a/Framework/PythonInterface/test/python/mantid/kernel/CMakeLists.txt
+++ b/Framework/PythonInterface/test/python/mantid/kernel/CMakeLists.txt
@@ -27,6 +27,7 @@ set ( TEST_PY_FILES
   PropertyHistoryTest.py
   PropertyWithValueTest.py
   PropertyManagerTest.py
+  PropertyManagerDataServiceTest.py
   PropertyManagerPropertyTest.py
   PythonPluginsTest.py
   StatisticsTest.py
diff --git a/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerDataServiceTest.py b/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerDataServiceTest.py
new file mode 100644
index 00000000000..058984e4fac
--- /dev/null
+++ b/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerDataServiceTest.py
@@ -0,0 +1,40 @@
+from __future__ import (absolute_import, division, print_function)
+
+import unittest
+from mantid.kernel import PropertyManager, PropertyManagerDataService
+
+class PropertyManagerDataServiceTest(unittest.TestCase):
+
+    def test_add_existing_mgr_object(self):
+        name = "PropertyManagerDataServiceTest_test_add_existing_mgr_object"
+        values = {'key': 100.5}
+        mgr = PropertyManager(values)
+        self._do_add_test(name, mgr)
+
+    def test_add_straight_from_dict(self):
+        name = "PropertyManagerDataServiceTest_test_add_straight_from_dict"
+        values = {'key': 100.5}
+        self._do_add_test(name, values)
+
+    def test_addOrReplace_straight_from_dict(self):
+        name = "PropertyManagerDataServiceTest_addOrReplace_straight_from_dict"
+        values = {'key': 100.5}
+        values2 = {'key2': 50}
+        self._do_addOrReplace_test(name, values, values2)
+
+    def _do_add_test(self, name, value):
+        pmds = PropertyManagerDataService.Instance()
+        pmds.add(name, value)
+        self.assertTrue(name in pmds)
+        pmds.remove(name)
+
+    def _do_addOrReplace_test(self, name, value, value2):
+        pmds = PropertyManagerDataService.Instance()
+        pmds.add(name, value)
+        pmds.addOrReplace(name, value2)
+        pmgr = pmds[name]
+        self.assertEquals(value2['key2'], pmgr['key2'].value)
+        pmds.remove(name)
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerTest.py b/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerTest.py
index 05ec479b721..e16f512d933 100644
--- a/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerTest.py
+++ b/Framework/PythonInterface/test/python/mantid/kernel/PropertyManagerTest.py
@@ -4,7 +4,7 @@ import unittest
 from mantid.kernel import PropertyManager, IPropertyManager
 
 class PropertyManagerTest(unittest.TestCase):
-    def test_propertymanager(self):
+    def test_propertymanager_population(self):
         manager = PropertyManager()
 
         # check that it is empty
@@ -64,5 +64,21 @@ class PropertyManagerTest(unittest.TestCase):
         del manager["f"]
         self.assertTrue(len(manager), 2)
 
+    def test_propertymanager_can_be_created_from_dict(self):
+        values = {
+            "int": 5,
+            "float": 20.0,
+            "str": 'a string'
+        }
+        pmgr = PropertyManager(values)
+        self.assertEquals(len(pmgr), 3)
+        self.assertEquals(5, pmgr["int"].value)
+        self.assertEquals(20.0, pmgr["float"].value)
+        self.assertEquals('a string', pmgr["str"].value)
+
+    def test_propertymanager_cannot_be_created_from_arbitrary_sequence(self):
+        with self.assertRaises(Exception):
+            PropertyManager((1,2,3,4,5))
+
 if __name__ == "__main__":
     unittest.main()
-- 
GitLab