Merge pull request #188 from Point72/bug/struct-list

Override Python list modifying methods to capture and preserve changes to Struct list fields

Author: ogarokpeter

cpp/csp/python/CMakeLists.txt

@@ -3,15 +3,17 @@ set(CSPTYPESIMPL_PUBLIC_HEADERS
         CspTypeFactory.h
         PyCspEnum.h
         PyCspType.h
-        PyStruct.h)
+        PyStruct.h
+        PyStructList.h)
 
 add_library(csptypesimpl
             csptypesimpl.cpp
             CspTypeFactory.cpp
             PyCspEnum.cpp
             PyCspType.cpp
             PyStruct.cpp
-            PyStructToJson.cpp)
+            PyStructToJson.cpp
+            PyStructList.hi)
 set_target_properties(csptypesimpl PROPERTIES PUBLIC_HEADER "${CSPTYPESIMPL_PUBLIC_HEADERS}")
 target_compile_definitions(csptypesimpl PUBLIC RAPIDJSON_HAS_STDSTRING=1)
 target_link_libraries(csptypesimpl csp_core csp_types)
@@ -39,7 +41,8 @@ set(CSPIMPL_PUBLIC_HEADERS
         PyOutputAdapterWrapper.h
         PyOutputProxy.h
         PyConstants.h
-        PyStructToJson.h)
+        PyStructToJson.h
+        PyStructList.h)
 
 add_library(cspimpl SHARED
         cspimpl.cpp
@@ -70,6 +73,7 @@ add_library(cspimpl SHARED
         PyManagedSimInputAdapter.cpp
         PyTimerAdapter.cpp
         PyConstants.cpp
+        PyStructList.hi
         ${CSPIMPL_PUBLIC_HEADERS})
 
 set_target_properties(cspimpl PROPERTIES PUBLIC_HEADER "${CSPIMPL_PUBLIC_HEADERS}")

cpp/csp/python/Conversions.h

@@ -12,6 +12,7 @@
 #include <csp/python/PyCspType.h>
 #include <csp/python/PyObjectPtr.h>
 #include <csp/python/PyStruct.h>
+#include <csp/python/PyStructList.h>
 #include <datetime.h>
 #include <Python.h>
 #include <string>
@@ -762,6 +763,29 @@ inline PyObject * toPython( const std::vector<StorageT> & v, const CspType & typ
     return list.release();
 }
 
+template<typename StorageT>
+inline PyObject * toPython( const std::vector<StorageT> & v, const CspType & type, const PyStruct * pystruct )
+{
+    assert( type.type() == CspType::Type::ARRAY );
+
+    const CspTypePtr elemType = static_cast<const CspArrayType &>( type ).elemType();
+    using ElemT = typename CspType::Type::toCArrayElemType<StorageT>::type;
+    size_t sz = v.size();
+
+    // TODO: Implement more efficient list allocation by pre-allocating the space and filling it using PyList_SET_ITEM.
+    // As of now, the problem is that Python is not allowing to resize the list via API, and it cannot allocate the list at the base of PyStructList, it can only allocate it somewhere in memory not under control.
+    PyObject * psl = PyStructList<StorageT>::PyType.tp_alloc( &PyStructList<StorageT>::PyType, 0 );
+    new ( psl ) PyStructList<StorageT>( const_cast<PyStruct *>( pystruct ), const_cast<std::vector<StorageT> &>( v ), *elemType );
+
+    for( size_t index = 0; index < sz; ++index )
+    {
+        PyObjectPtr element = PyObjectPtr::own( toPython<ElemT>( v[ index ], *elemType ) );
+        PyList_Append( ( PyObject * ) psl, element.get() );
+    }
+    
+    return psl;
+}
+
 template<typename StorageT>
 struct FromPython<std::vector<StorageT>>
 {

cpp/csp/python/InitHelper.h

@@ -20,7 +20,7 @@ class __attribute__ ((visibility ("hidden"))) InitHelper
 
     bool registerCallback( InitCallback cb );
 
-    static InitCallback typeInitCallback( PyTypeObject * pyType, std::string name );
+    static InitCallback typeInitCallback( PyTypeObject * pyType, std::string name, PyTypeObject * baseType = nullptr );
     static InitCallback moduleMethodsCallback( PyMethodDef * methods );
     static InitCallback moduleMethod( const char * name, PyCFunction func, int flags, const char * doc );
 
@@ -50,9 +50,11 @@ inline InitHelper & InitHelper::instance()
     return s_instance;
 }
 
-inline InitHelper::InitCallback InitHelper::typeInitCallback( PyTypeObject * pyType, std::string name )
+inline InitHelper::InitCallback InitHelper::typeInitCallback( PyTypeObject * pyType, std::string name, PyTypeObject * baseType )
 {
-    InitCallback cb = [pyType,name]( PyObject * module ) {
+    InitCallback cb = [pyType,name,baseType]( PyObject * module ) {
+        if( baseType )
+            pyType -> tp_base = baseType;
         if( PyType_Ready( pyType ) < 0 )
             return false;
 

cpp/csp/python/PyStruct.cpp

@@ -3,8 +3,8 @@
 #include <csp/python/InitHelper.h>
 #include <csp/python/PyObjectPtr.h>
 #include <csp/python/PyStruct.h>
+#include <csp/python/PyStructList.hi>
 #include <csp/python/PyStructToJson.h>
-
 #include <unordered_set>
 #include <type_traits>
 
@@ -395,8 +395,10 @@ PyTypeObject PyStructMeta::PyType = {
 
 
 //PyStruct
-PyObject * getattr_( const StructField* field, const Struct * struct_ )
+PyObject * getattr_( const StructField * field, const Struct * struct_ )
 {
+    assert( field -> type() -> type() != CspType::Type::ARRAY );
+
     PyObject *v = switchCspType( field -> type(), [ field, struct_ ]( auto tag )
     {
         using CType = typename decltype(tag)::type;
@@ -407,6 +409,21 @@ PyObject * getattr_( const StructField* field, const Struct * struct_ )
     return v;
 }
 
+PyObject * getarrayattr_( const StructField * field, const PyStruct * pystruct )
+{
+    assert( field -> type() -> type() == CspType::Type::ARRAY );
+    
+    const CspArrayType * arrayType = static_cast<const CspArrayType *>( field -> type().get() );
+    PyObject *v = ArraySubTypeSwitch::invoke( arrayType -> elemType(), [ field, pystruct ]( auto tag )
+    {
+        using StorageT  = typename CspType::Type::toCArrayStorageType<typename decltype(tag)::type>::type;
+        using ArrayT    = typename StructField::upcast<std::vector<StorageT>>::type;
+        auto * typedField = static_cast<const ArrayT *>( field );
+        return toPython( typedField -> value( pystruct -> struct_.get()  ), *field -> type(), pystruct );
+    } );
+    return v;
+}
+
 PyObject * PyStruct::getattr( PyObject * attr )
 {
     auto * field = structMeta() -> field( attr );
@@ -424,7 +441,9 @@ PyObject * PyStruct::getattr( PyObject * attr )
         return nullptr;
     }
 
-    return getattr_( field, ( const Struct *)struct_.get() );
+    if( field -> type() -> type() == CspType::Type::ARRAY )
+        return getarrayattr_( field, this );
+    return getattr_( field, ( const Struct * ) struct_.get() );
 }
 
 void PyStruct::setattr( Struct * s, PyObject * attr, PyObject * value )
@@ -997,4 +1016,24 @@ PyTypeObject PyStruct::PyType = {
 REGISTER_TYPE_INIT( &PyStructMeta::PyType, "PyStructMeta" )
 REGISTER_TYPE_INIT( &PyStruct::PyType,     "PyStruct" )
 
+// Instantiate all templates for PyStructList class
+template struct PyStructList<bool>;
+template struct PyStructList<int8_t>;
+template struct PyStructList<uint8_t>;
+template struct PyStructList<int16_t>;
+template struct PyStructList<uint16_t>;
+template struct PyStructList<int32_t>;
+template struct PyStructList<uint32_t>;
+template struct PyStructList<int64_t>;
+template struct PyStructList<uint64_t>;
+template struct PyStructList<double>;
+template struct PyStructList<DateTime>;
+template struct PyStructList<TimeDelta>;
+template struct PyStructList<Date>;
+template struct PyStructList<Time>;
+template struct PyStructList<std::string>;
+template struct PyStructList<DialectGenericType>;
+template struct PyStructList<StructPtr>;
+template struct PyStructList<CspEnum>;
+
 }

cpp/csp/python/PyStructList.h

@@ -0,0 +1,36 @@
+#ifndef _IN_CSP_PYTHON_PYSTRUCTLIST_H
+#define _IN_CSP_PYTHON_PYSTRUCTLIST_H
+
+#include <csp/python/InitHelper.h>
+#include <csp/python/PyStruct.h>
+#include <Python.h>
+#include <vector>
+
+namespace csp::python
+{
+
+template<typename StorageT>
+struct PyStructList : public PyObject
+{
+    using ElemT = typename CspType::Type::toCArrayElemType<StorageT>::type;
+
+    PyStructList<StorageT>( PyStruct * p, std::vector<StorageT> & v, const CspType & type ) : pystruct( p ), vector( v ), field_type( type )
+    {
+        Py_INCREF( pystruct );
+    }
+
+    PyListObject base;                // Inherit from PyListObject
+    PyStruct * pystruct;             // Pointer to PyStruct for proper reference counting
+    std::vector<StorageT> & vector;  // Reference to field value for modifying
+
+    const CspType & field_type;       // We require the type information of any non-primitive type, i.e. Struct or Enum, since they contain a meta
+    static PyTypeObject PyType;
+    static bool s_typeRegister;
+};
+
+template<typename StorageT> bool PyStructList<StorageT>::s_typeRegister = InitHelper::instance().registerCallback( 
+    InitHelper::typeInitCallback( &PyStructList<StorageT>::PyType, "", &PyList_Type ) );
+
+}
+
+#endif