Refs #3614. Algorithm exports for [set/get]property along with properties for...

Refs #3614. Algorithm exports for [set/get]property along with properties for basic types. An algorithm unit test as well.

Code/Mantid/Framework/CMakeLists.txt

@@ -106,6 +106,7 @@ add_subdirectory (API)
 set ( MANTIDLIBS ${MANTIDLIBS} API )
 
 add_subdirectory (PythonAPI)
+add_subdirectory (PythonInterface) # Version 2
 
 find_package ( Matlab )
 if( MATLAB_FOUND )

Code/Mantid/Framework/PythonInterface/CMakeLists.txt

 ###########################################################################
+#
 # Python API (version 2)
+#
 ###########################################################################
 
 ###########################################################################
 # Set local dependencies
 ###########################################################################
-
 set ( Boost_LIBRARIES ) # Empty out the variable after previous use
 set ( Boost_USE_DEBUG_PYTHON TRUE )
 find_package ( Boost REQUIRED python )
@@ -13,12 +14,14 @@ add_definitions ( -DBOOST_DEBUG_PYTHON -DBOOST_PYTHON_NO_LIB )
 
 find_package ( Numpy REQUIRED )
 include_directories ( ${PYTHON_NUMPY_INCLUDE_DIR} )
+set ( HEADER_DIR ${CMAKE_CURRENT_SOURCE_DIR}/inc/MantidPythonInterface )
 include_directories ( inc )
 
-# Note: On some Linux systems, seen on various Ubuntu versions, importing Mantid into a standalone python
+# Note: On some Linux systems (seen on various Ubuntu versions) importing Mantid into a standalone python
 # interpreter causes a segfault. It is some issue due to exception handling but the fix is 
 # to ensure that the stdc++ library appears as early in the link list as possible so that it
-# is loaded first, hence the hard coding of it here rather than leaving it to be implicitly defined.
+# is loaded first, hence the hard coding of it here rather than leaving it to be implicitly defined
+# by the linker.
 if ( UNIX ) 
   set ( PYTHON_DEPS stdc++ ${MANTIDLIBS} ${Boost_LIBRARIES} ${PYTHON_LIBRARIES}  )
 else ()
@@ -33,7 +36,7 @@ add_subdirectory ( mantid )
 ###########################################################################
 # tests 
 ###########################################################################
-# cxxtest unit tests
+# cxx unit tests
 set ( TEST_FILES
   test/cpp/PythonObjectInstantiatorTest.h
 )
@@ -49,13 +52,15 @@ if ( CXXTEST_FOUND )
   set_property ( TARGET PythonInterfaceTest PROPERTY FOLDER "UnitTests" )
 endif ()
 
+# python unit tests
 set ( TEST_PY_FILES
+  test/python/AlgorithmTest.py
   test/python/AlgorithmManagerTest.py
-  test/python/PythonAlgorithmTest.py
   test/python/FrameworkManagerTest.py
-  test/python/V3DTest.py
-  test/python/QuatTest.py
   test/python/ImportModuleTest.py
+  test/python/PythonAlgorithmTest.py
+  test/python/QuatTest.py
+  test/python/V3DTest.py
 )
 
 # python unit tests

Code/Mantid/Framework/PythonInterface/inc/MantidPythonInterface/api/PropertyMarshal.h

+#ifndef MANTID_PYTHONINTERFACE_PROPERTYMARSHAL_H_
+#define MANTID_PYTHONINTERFACE_PROPERTYMARSHAL_H_
+
+#include "MantidKernel/Property.h"
+#include <boost/python/object.hpp>
+#include <string>
+
+namespace Mantid
+{
+  namespace PythonInterface
+  {
+
+    /**
+     * Python is dynamically typed so that the type of a variable is not
+     * known until run time. The [set,get]Property methods on an algorithm
+     * expect the value passed/returned to match that of the declared property
+     * type, i.e. an integer property must use alg.setProperty(name, int).
+     *
+     * Boost.Python is able to declare several 'overloaded' functions of the
+     * same name but if the number of arguments are the same and only the
+     * types differ it is not able to figure out the correct function to
+     * call. It instead chooses the last from the exported list that contains
+     * a type that the value can be converted to. This then raises an exception
+     * in Mantid.
+     *
+     * The PropertyMarshal struct declared here deals with calling the correct
+     * function depending on the type passed to it.
+
+     * We will also need more marshaling for these functions as we want be able to
+     * pass numpy arrays seamlessly to algorithms.
+     *
+     * The first argument of each function MUST be an object of type
+     * boost::python::object. This provides access to the object that performed the
+     * method call. It is equivalent to a python method that starts with 'self'
+     */
+    template<typename CType>
+    struct PropertyMarshal
+    {
+      /// Set a named property to a given value
+      static void setProperty(boost::python::object self, const std::string & name,
+                CType value);
+    };
+
+    /// Retrieve a named property
+    Kernel::Property * getProperty(boost::python::object self, const std::string & name);
+
+  }
+}
+
+#endif /* MANTID_PYTHONINTERFACE_PROPERTYMARSHAL_H_ */

Code/Mantid/Framework/PythonInterface/inc/MantidPythonInterface/kernel/PropertyWithValue.h

+#ifndef MANTID_PYTHONINTERFACE_PROPERTY_HPP_
+#define MANTID_PYTHONINTERFACE_PROPERTY_HPP_
+
+#include "MantidKernel/PropertyWithValue.h"
+
+/**
+ * Define a macro to export PropertyWithValue template types
+ */
+#define EXPORT_PROP_W_VALUE(type, suffix)   \
+  class_<Mantid::Kernel::PropertyWithValue<type>, \
+         bases<Mantid::Kernel::Property>, boost::noncopyable>("PropertyWithValue"#suffix, no_init) \
+             .add_property("value", make_function(&Mantid::Kernel::PropertyWithValue<type>::operator(), return_value_policy<copy_const_reference>())) \
+             ;
+
+#endif /* MANTID_PYTHONINTERFACE_PROPERTY_HPP_ */

Code/Mantid/Framework/PythonInterface/mantid/__init__.py

@@ -17,4 +17,4 @@ if not os.environ.has_key('MANTIDPATH') and os.path.exists(os.path.join(_bindir,
 from api import framework_mgr
 
 # Start Mantid (mtd for old times sake)
-mtd = framework_mgr
+mtd = framework_mgr
\ No newline at end of file

Code/Mantid/Framework/PythonInterface/mantid/api/CMakeLists.txt

@@ -14,9 +14,13 @@ set ( EXPORT_FILES
 # Files containing addtional helper code that do not require processing
 set ( SRC_FILES 
   src/AlgorithmWrapper.cpp
+  src/PropertyMarshal.cpp
 )
 
-set ( INC_FILES )
+set ( INC_FILES 
+  ${HEADER_DIR}/api/AlgorithmWrapper.h
+  ${HEADER_DIR}/api/PropertyMarshal.h
+)
 
 set ( PY_FILES 
   __init__.py

Code/Mantid/Framework/PythonInterface/mantid/api/__init__.py

@@ -10,4 +10,53 @@ dlopen.restore_flags(flags)
 # Alias the singleton objects
 framework_mgr = get_framework_mgr() # The first import of this starts the framework
 algorithm_mgr = get_algorithm_mgr()
- 
+
\ No newline at end of file
+# Control what an import * does
+__all__ = dir(_api)
+__all__.extend(['algorithm_mgr', 'framework_mgr'])
+
+
+#########################################################################################
+## Private methods (not in global import) 
+#########################################################################################
+
+def IAlgorithm_dynamic_getattr(self, name):
+    """Dark magic so that there can be a single function call set_property that routes to
+    the appropriate place.
+    
+    The issue is actually only with the basic numeric types. Boost Python selects
+    the last method that was registered when the argument is int, float or bool because
+    Boost Python determines that it is acceptable to convert to the type of that function
+    but Mantid won't accept it because the type then doesn't match the property declaration.
+    For example, assume an algorithm has a property declared as type int:
+      
+      Calling set_property('Name', 10) leads Boost.Python to call the first function
+      it can find that will accept that type. If the C++ setProperty taking a double
+      was exported last then it casts the int to a double and calls that function
+      resulting in Mantid saying there is a type mismatch
+      
+      The problem is simple reversed if you export the int set_property last as then
+      that fails for float arguments
+    
+    This function replaces the __getattribute__ function on IAlgorithm and returns
+    a different method call depending on the type of arguments that have been 
+    passed to the original function call. It is only the basic types that suffer so
+    everything else is routed to the generic set_property    
+    """
+    if name != 'set_property':
+        return object.__getattribute__(self, name)
+
+    def set_property(self, propname, value):
+        if type(value) is int:
+            method = object.__getattribute__(self, name + '_int')
+        elif type(value) is float:
+            method = object.__getattribute__(self, name + '_float')
+        elif type(value) is bool:
+            method = object.__getattribute__(self, name + '_bool')
+        else:
+            method = object.__getattribute__(self, name)
+        method(propname, value)
+    return set_property.__get__(self) 
+
+# Replace the standard __get__attribute method
+IAlgorithm.__getattribute__ = IAlgorithm_dynamic_getattr
\ No newline at end of file

Code/Mantid/Framework/PythonInterface/mantid/api/src/Algorithm.cpp

 #include "MantidPythonInterface/api/AlgorithmWrapper.h"
+#include "MantidPythonInterface/api/PropertyMarshal.h"
 #include "MantidAPI/AlgorithmProxy.h"
 #include <boost/python/class.hpp>
 #include <boost/python/register_ptr_to_python.hpp>
+#include <boost/python/return_internal_reference.hpp>
 
 using Mantid::API::IAlgorithm;
 using Mantid::API::IAlgorithm_sptr;
 using Mantid::API::Algorithm;
 using Mantid::API::AlgorithmProxy;
 using Mantid::PythonInterface::AlgorithmWrapper;
+using Mantid::PythonInterface::PropertyMarshal;
 
 using boost::python::class_;
 using boost::python::register_ptr_to_python;
 using boost::python::bases;
 using boost::python::no_init;
+using boost::python::return_internal_reference;
+
+// Use a macro for the different property types
+#define EXPORT_PROPERTY_ACCESSORS(type, ...) \
+  .def("set_property"#__VA_ARGS__, &PropertyMarshal<type>::setProperty, "Sets a property of type "#type) \
+  //.def("get_property", &PropertyMarshal<type>::getProperty, "Returns a property of type "#type)
 
 void export_algorithm()
 {
@@ -25,6 +34,15 @@ void export_algorithm()
     .def("category", &IAlgorithm::category, "Returns the category containing the algorithm")
     .def("initialize", &IAlgorithm::initialize, "Initializes the algorithm")
     .def("execute", &IAlgorithm::execute, "Runs the algorithm")
+    .def("set_child", &IAlgorithm::setChild,
+        "If true this algorithm is run as a child algorithm. There will be no logging and nothing is stored in the Analysis Data Service")
+    .def("set_logging", &IAlgorithm::setLogging, "Toggle logging on/off.")
+    .def("set_property_value", &IAlgorithm::setPropertyValue, "Sets the value of the named property via a string")
+    .def("get_property", &IAlgorithm::getPointerToProperty, return_internal_reference<>(), "Retrieve the named property")
+    EXPORT_PROPERTY_ACCESSORS(bool,_bool)
+    EXPORT_PROPERTY_ACCESSORS(int,_int)
+    EXPORT_PROPERTY_ACCESSORS(double, _float)
+    EXPORT_PROPERTY_ACCESSORS(std::string)
     ;
 
   register_ptr_to_python<boost::shared_ptr<AlgorithmProxy> >();
@@ -40,3 +58,6 @@ void export_algorithm()
   class_<AlgorithmWrapper, bases<Algorithm>, boost::noncopyable>("PythonAlgorithm", "Base for all Python algorithms")
     ;
 }
+
+// Clean up namespace
+#undef EXPORT_PROPERTY_ACCESSORS

Code/Mantid/Framework/PythonInterface/mantid/api/src/AlgorithmManager.cpp

@@ -21,13 +21,15 @@ using namespace boost::python;
 namespace
 {
   ///@cond
+  //------------------------------------------------------------------------------------------------------
   // A factory function returning a reference to the AlgorithmManager instance so that Python can use it
   AlgorithmManagerImpl & getAlgorithmManager()
   {
     return AlgorithmManager::Instance();
   }
 
-  // A free function to register an algorithm from Python
+  //------------------------------------------------------------------------------------------------------
+  // A function to register an algorithm from Python
   void registerAlgorithm(boost::python::object obj)
   {
     // The current frame should know what a PythonAlgorithm is, or it
@@ -56,6 +58,7 @@ namespace
     AlgorithmFactory::Instance().subscribe(new PythonObjectInstantiator<Algorithm>(classType));
   }
 
+  //------------------------------------------------------------------------------------------------------
   /// Define overload generators
   BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(create_overloads,AlgorithmManagerImpl::create, 1,2);
   BOOST_PYTHON_MEMBER_FUNCTION_OVERLOADS(createUnmanaged_overloads,AlgorithmManagerImpl::createUnmanaged, 1,2);
@@ -72,7 +75,7 @@ void export_AlgorithmManager()
     ;
 
   // Create a factory function to return this in Python
-  def("get_algorithm_mgr", &getAlgorithmManager, return_value_policy<reference_existing_object>(), //Note this policy is really only safe for singletons
+  def("get_algorithm_mgr", &getAlgorithmManager, return_value_policy<reference_existing_object>(), //This policy is really only safe for singletons
         "Returns a reference to the AlgorithmManager singleton");
 
   // The registration function

Code/Mantid/Framework/PythonInterface/mantid/api/src/PropertyMarshal.cpp

+//-----------------------------------------------------------------------------
+// Includes
+//-----------------------------------------------------------------------------
+#include "MantidPythonInterface/api/PropertyMarshal.h"
+#include "MantidAPI/IAlgorithm.h"
+
+#include <boost/python/extract.hpp>
+#include <typeinfo>
+
+namespace Mantid
+{
+  namespace PythonInterface
+  {
+    namespace python = boost::python;
+    using API::IAlgorithm;
+    using Kernel::Property;
+
+    namespace
+    {
+      /**
+       * Extracts a pointer to an algorithm from a python object
+       * @param obj :: A boost::python object
+       * @returns A pointer to an IAlgorithm
+       */
+      IAlgorithm * getAlgorithm(python::object obj)
+      {
+        return python::extract<IAlgorithm*>(obj); // Throws TypeError if incorrect
+      }
+    }
+
+    /**
+     * Set a named property's value
+     * @param self :: A reference to the calling object
+     * @param name :: The name of the property
+     * @param value :: The value of the property as a boost::python object
+     */
+    template<typename CType>
+    void PropertyMarshal<CType>::setProperty(python::object self, const std::string & name,
+        CType value)
+    {
+      IAlgorithm * alg = getAlgorithm(self);
+      alg->setProperty(name, value);
+    }
+
+    //---------------------------------------------------------------------------
+    // Concrete implementations.
+    //---------------------------------------------------------------------------
+
+    ///@cond
+    // Concrete implementations.
+    template DLLExport struct PropertyMarshal<int>;
+    template DLLExport struct PropertyMarshal<bool>;
+    template DLLExport struct PropertyMarshal<double>;
+    template DLLExport struct PropertyMarshal<std::string>;
+    ///@endcond
+
+  }
+}

Code/Mantid/Framework/PythonInterface/mantid/kernel/CMakeLists.txt

@@ -5,11 +5,15 @@
 set ( MODULE_TEMPLATE src/kernel.cpp.in )
 
 set ( EXPORT_FILES
-  src/V3D.cpp
+  src/Property.cpp
   src/Quat.cpp
+  src/V3D.cpp
 )  
 
-set ( INC_FILES )
+set ( INC_FILES 
+  ${HEADER_DIR}/kernel/PythonObjectInstantiator.h
+  ${HEADER_DIR}/kernel/PropertyWithValue.h
+)
 
 set ( PY_FILES 
   __init__.py

Code/Mantid/Framework/PythonInterface/mantid/kernel/src/Property.cpp

+#include "MantidKernel/Property.h"
+#include "MantidPythonInterface/kernel/PropertyWithValue.h"
+
+#include <boost/python/class.hpp>
+#include <boost/python/register_ptr_to_python.hpp>
+#include <boost/python/return_value_policy.hpp>
+#include <boost/python/copy_const_reference.hpp>
+#include <boost/python/enum.hpp>
+#include <boost/python/make_function.hpp>
+
+using Mantid::Kernel::Property;
+using Mantid::Kernel::Direction;
+using namespace boost::python;
+
+
+void export_Property()
+{
+  // Ptr<->Object conversion
+  register_ptr_to_python<Property*>();
+
+  //Direction
+  enum_<Direction::Type>("Direction")
+    .value("Input", Direction::Input)
+    .value("Output", Direction::Output)
+    .value("InOut", Direction::InOut)
+    .value("None", Direction::None)
+    ;
+
+  class_<Property, boost::noncopyable>("Property", no_init)
+    .add_property("name", make_function(&Mantid::Kernel::Property::name, return_value_policy<copy_const_reference>()), "The name of the property")
+    .add_property("isValid", &Mantid::Kernel::Property::isValid, "An empty string if the property is valid, otherwise it contains an error message.")
+    .add_property("allowed_values", &Mantid::Kernel::Property::allowedValues, "A list of allowed values")
+    .add_property("direction", &Mantid::Kernel::Property::direction, "Input, Output, InOut or Unknown. See the Direction enum")
+    .add_property("units", &Mantid::Kernel::Property::units, "The units attached to this property")
+    .add_property("is_default", &Mantid::Kernel::Property::isDefault, "Is the property set at the default value")
+   ;
+}
+
+/**
+ * I see know way around explicitly declaring each we use
+ */
+void export_PropertyWithValue()
+{
+  EXPORT_PROP_W_VALUE(int, _int);
+  EXPORT_PROP_W_VALUE(double, _dbl);
+  EXPORT_PROP_W_VALUE(bool, _bool);
+}

Code/Mantid/Framework/PythonInterface/test/python/AlgorithmTest.py

+import unittest
+
+from mantid.api import algorithm_mgr
+
+class AlgorithmTest(unittest.TestCase):
+  
+    def test_alg_attrs_are_correct(self):
+        alg = algorithm_mgr.create_unmanaged('Load')
+        self.assertTrue(alg.name(), 'Load')
+        self.assertTrue(alg.version(), 1)
+        self.assertTrue(alg.category(), 'DataHandling')
+        
+    def test_alg_set_valid_prop_succeeds(self):
+        alg = algorithm_mgr.create_unmanaged('Load')
+        alg.initialize()
+        alg.set_property('Filename', 'ALF15739.raw')
+        
+    def test_alg_set_invalid_prop_raises_error(self):
+        alg = algorithm_mgr.create_unmanaged('Load')
+        alg.initialize()
+        args = ('Filename', 'XYZ12345.txt')
+        self.assertRaises(RuntimeError, alg.set_property, *args)
+        
+    def test_cannot_execute_with_invalid_properties(self):
+        alg = algorithm_mgr.create_unmanaged('Load')
+        alg.initialize()
+        self.assertRaises(RuntimeError, alg.execute)
+        
+    def test_execute_succeeds_with_valid_props(self):
+        alg = algorithm_mgr.create_unmanaged('Load')
+        alg.initialize()
+        alg.set_property('Filename', 'ALF15739.raw')
+        alg.set_property('SpectrumMax', 10)
+        alg.set_property('OutputWorkspace', 'ALF15739')
+        alg.set_child(True) # Just to keep the output from the data service
+        alg.execute()
+        self.assertEquals(alg.get_property('SpectrumMax').value, 10)
+