Remove copy constructor assumption. Manually add copy constructors. Remove dependency on default constructor (some like Optimizers and Marginals don't have the default constructor). Remove cyCreateFromValue. Ignore variable name when checking overload similarity.

gtsam/nonlinear/Marginals.h

@@ -48,6 +48,8 @@ protected:
 
 public:
 
+  Marginals(const Marginals&) = default;
+
   /** Construct a marginals class.
    * @param graph The factor graph defining the full joint density on all variables.
    * @param solution The linearization point about which to compute Gaussian marginals (usually the MLE as obtained from a NonlinearOptimizer).
@@ -91,6 +93,8 @@ protected:
   FastMap<Key, size_t> indices_;
 
 public:
+  JointMarginal(const JointMarginal&) = default;
+
   /** Access a block, corresponding to a pair of variables, of the joint
    * marginal.  Each block is accessed by its "vertical position",
    * corresponding to the variable with nonlinear Key \c iVariable and

wrap/Argument.h

@@ -39,6 +39,12 @@ struct Argument {
       type(t), name(n), is_const(false), is_ref(false), is_ptr(false) {
   }
 
+  bool typesEqual(const Argument& other) const {
+    return type == other.type
+        && is_const == other.is_const && is_ref == other.is_ref
+        && is_ptr == other.is_ptr;
+  }
+
   bool operator==(const Argument& other) const {
     return type == other.type && name == other.name
         && is_const == other.is_const && is_ref == other.is_ref
@@ -98,6 +104,12 @@ struct ArgumentList: public std::vector<Argument> {
 
   ArgumentList expandTemplate(const TemplateSubstitution& ts) const;
 
+  bool typesEqual(const ArgumentList& other) const {
+    for(size_t i = 0; i<size(); ++i) 
+      if (!at(i).typesEqual(other[i])) return false;
+    return true;
+  }
+
   // MATLAB code generation:
 
   /**

wrap/Class.cpp

@@ -475,7 +475,7 @@ void Class::removeInheritedNontemplateMethods(vector<Class>& classes) {
                        [&](boost::tuple<ReturnValue, ArgumentList> const&
                                parentOverload) {
                            return overload.get<0>() == parentOverload.get<0>() &&
-                                  overload.get<1>() == parentOverload.get<1>();
+                                  overload.get<1>().typesEqual(parentOverload.get<1>());
                        }) != parentMethodOverloads.end();
             return found;
         });
@@ -811,24 +811,6 @@ void Class::emit_cython_pyx(FileWriter& pyxFile, const std::vector<Class>& allCl
   pyxInitParentObj(pyxFile, "\t\tret", "other", allClasses);
   pyxFile.oss << "\t\treturn ret" << "\n";
 
-  // Generate cyCreateFromValue by default, although for some classes it can't be used
-  // It's only usable if its copy constructor AND its copy assignment operator exist
-  // Copy assignment operator is needed because Cython might assign the obj to its temp variable.
-  // Some class (e.g. noiseModel::Robust) have copy constructor but no copy assignment operator
-  if (constructor.nrOverloads() >= 1) { 
-    // cyCreateFromValue
-    pyxFile.oss << "\t@staticmethod\n";
-    pyxFile.oss << "\tcdef " << pythonClass() << " cyCreateFromValue(const "
-                << pyxCythonClass() << "& value):\n"
-                << "\t\tcdef " << pythonClass()
-                << " ret = " << pythonClass() << "()\n"
-                << "\t\tret." << pyxCythonObj() << " = " << pyxSharedCythonClass()
-                << "(new " << pyxCythonClass() << "(value))\n";
-    pyxInitParentObj(pyxFile, "\t\tret", "ret." + pyxCythonObj(), allClasses);
-    pyxFile.oss << "\t\treturn ret" << "\n";
-    pyxFile.oss << "\n";
-  }
-
   // Constructors
   constructor.emit_cython_pyx(pyxFile, *this);
   if (constructor.nrOverloads()>0) pyxFile.oss << "\n";

wrap/Constructor.cpp

@@ -131,17 +131,8 @@ bool Constructor::hasDefaultConstructor() const {
 
 /* ************************************************************************* */
 void Constructor::emit_cython_pxd(FileWriter& pxdFile, Str className) const {
-  // HACK HACK: always add the default copy constructor by default
-  pxdFile.oss << "\t\t" << className << "(const " << className
-              << "&) except +\n";
-
   for (size_t i = 0; i < nrOverloads(); i++) {
     ArgumentList args = argumentList(i);
-    // ignore copy constructor, it's generated above by default
-    if (args.size() == 1 && args[0].is_const && args[0].is_ref &&
-        !args[0].is_ptr && (args[0].type.cythonClass() == className ||
-                            args[0].type.cythonClass() == "This"))
-            continue;
 
     // generate the constructor
     pxdFile.oss << "\t\t" << className << "(";

wrap/Method.cpp

@@ -28,6 +28,16 @@
 using namespace std;
 using namespace wrap;
 
+/* ************************************************************************* */
+static const std::array<std::string,2> pythonKeywords{{"print", "lambda"}};
+static std::string pyRename(const std::string& name) {
+    if (std::find(pythonKeywords.begin(), pythonKeywords.end(), name) ==
+        pythonKeywords.end())
+        return name;
+    else
+        return "_" + name;
+}
+
 /* ************************************************************************* */
 bool Method::addOverload(Str name, const ArgumentList& args,
     const ReturnValue& retVal, bool is_const,
@@ -81,18 +91,18 @@ void Method::emit_cython_pxd(FileWriter& file, const Class& cls) const {
   for(size_t i = 0; i < nrOverloads(); ++i) {
     file.oss << "\t\t";
     returnVals_[i].emit_cython_pxd(file, cls.cythonClass());
-    file.oss << ((name_ == "print") ? "_print \"print\"" : name_) << "(";
+    file.oss << pyRename(name_) + " \"" + name_ + "\"" << "(";
+    // ((name_ == "print") ? "_print \"print\"" : name_) << "(";
     argumentList(i).emit_cython_pxd(file, cls.cythonClass());
     file.oss << ")";
     if (is_const_) file.oss << " const";
     file.oss << "\n";
   }
-
 }
 
 /* ************************************************************************* */
 void Method::emit_cython_pyx(FileWriter& file, const Class& cls) const {
-  string funcName = ((name_ == "print") ? "_print" : name_);
+  string funcName = pyRename(name_);
   size_t N = nrOverloads();
   for(size_t i = 0; i < N; ++i) {
     // Function definition

wrap/MethodBase.cpp

@@ -151,12 +151,20 @@ void MethodBase::emit_cython_pyx_function_call(FileWriter& file,
     returnVals_[iOverload].emit_cython_pyx_return_type(file);
     file.oss << " ret = ";
   }
+  if (!returnVals_[iOverload].isPair && !returnVals_[iOverload].type1.isPtr && returnVals_[iOverload].type1.isNonBasicType())
+    file.oss << returnVals_[iOverload].type1.pyxSharedCythonClass() << "(new "
+  << returnVals_[iOverload].type1.pyxCythonClass() << "(";
+
   //... function call
   file.oss << caller << "." << funcName;
   if (templateArgValue_) file.oss << "[" << templateArgValue_->pyxCythonClass() << "]";
   file.oss << "(";
   argumentList(iOverload).emit_cython_pyx_asParams(file);
-  file.oss << ")\n";
+  file.oss << ")";
+
+  if (!returnVals_[iOverload].isPair && !returnVals_[iOverload].type1.isPtr && returnVals_[iOverload].type1.isNonBasicType())
+    file.oss << "))";
+  file.oss << "\n";
 
   // ... casting return value
   if (!returnVals_[iOverload].isVoid()) {

wrap/Qualified.h

@@ -116,6 +116,10 @@ public:
         || name() == "size_t" || name() == "double");
   }
 
+  bool isVoid() const {
+    return name() == "void";
+  }
+
   bool isString() const {
     return name() == "string";
   }
@@ -125,7 +129,7 @@ public:
   }
 
   bool isNonBasicType() const {
-    return !isString() && !isScalar() && !isEigen();
+    return !isString() && !isScalar() && !isEigen() && !isVoid();
   }
 
 public:

wrap/ReturnType.cpp

@@ -77,27 +77,49 @@ void ReturnType::emit_cython_pxd(FileWriter& file, const std::string& className)
   file.oss << cythonType;
 }
 
-/* ************************************************************************* */
-void ReturnType::emit_cython_pyx_return_type(FileWriter& file) const {
+void ReturnType::emit_cython_pyx_return_type_noshared(FileWriter& file) const {
   string retType = pyxCythonClass();
   if (isPtr) retType = "shared_ptr[" + retType + "]";
   file.oss << retType;
 }
 
+
+/* ************************************************************************* */
+void ReturnType::emit_cython_pyx_return_type(FileWriter& file) const {
+  string retType = pyxCythonClass();
+  if (isPtr || isNonBasicType()) retType = "shared_ptr[" + retType + "]";
+  file.oss << retType;
+}
+
+void ReturnType::emit_cython_pyx_casting_noshared(FileWriter& file, const std::string& var) const {
+  if (isEigen())
+    file.oss << "ndarray_copy" << "(" << var << ")";
+  else if (isNonBasicType()) {
+    if (isPtr) 
+      file.oss << pythonClass() << ".cyCreateFromShared" << "(" << var << ")";
+    else {
+      file.oss << pythonClass() << ".cyCreateFromShared("
+      << pyxSharedCythonClass()
+                << "(new " << pyxCythonClass() << "(" << var << ")))";
+    }
+  } else file.oss << var;
+}
+
 /* ************************************************************************* */
 void ReturnType::emit_cython_pyx_casting(FileWriter& file, const std::string& var) const {
   if (isEigen())
-    file.oss << "ndarray_copy";
-  else if (isNonBasicType()){
-    if (isPtr) 
-      file.oss << pythonClass() << ".cyCreateFromShared";
-    else {
-      // if the function return an object, it must be copy constructible and copy assignable
-      // so it's safe to use cyCreateFromValue
-      file.oss << pythonClass() << ".cyCreateFromValue";
-    }
-  }
-  file.oss << "(" << var << ")";
+    file.oss << "ndarray_copy" << "(" << var << ")";
+  else if (isNonBasicType()) {
+    // if (isPtr) 
+      file.oss << pythonClass() << ".cyCreateFromShared" << "(" << var << ")";
+    // else {
+    //   // if the function return an object, it must be copy constructible and copy assignable
+    //   // so it's safe to use cyCreateFromValue
+    //   file.oss << pythonClass() << ".cyCreateFromShared("
+    //   << pyxSharedCythonClass()
+    //             << "(new " << pyxCythonClass() << "(" << var << ")))";
+    // }
+  } else file.oss << var;
 }
 
 /* ************************************************************************* */

wrap/ReturnType.h

@@ -50,6 +50,8 @@ struct ReturnType: public Qualified {
   void emit_cython_pxd(FileWriter& file, const std::string& className) const;
   void emit_cython_pyx_return_type(FileWriter& file) const;
   void emit_cython_pyx_casting(FileWriter& file, const std::string& var) const;
+  void emit_cython_pyx_return_type_noshared(FileWriter& file) const;
+  void emit_cython_pyx_casting_noshared(FileWriter& file, const std::string& var) const;
 
 private:
 

wrap/ReturnValue.cpp

@@ -86,9 +86,9 @@ void ReturnValue::emit_cython_pyx_return_type(FileWriter& file) const {
   if (isVoid()) return;
   if (isPair) {
     file.oss << "pair [";
-    type1.emit_cython_pyx_return_type(file);
+    type1.emit_cython_pyx_return_type_noshared(file);
     file.oss << ",";
-    type2.emit_cython_pyx_return_type(file);
+    type2.emit_cython_pyx_return_type_noshared(file);
     file.oss << "]";
   } else {
     type1.emit_cython_pyx_return_type(file);
@@ -100,9 +100,9 @@ void ReturnValue::emit_cython_pyx_casting(FileWriter& file, const std::string& v
   if (isVoid()) return;
   if (isPair) {
     file.oss << "(";
-    type1.emit_cython_pyx_casting(file, var + ".first");
+    type1.emit_cython_pyx_casting_noshared(file, var + ".first");
     file.oss << ",";
-    type2.emit_cython_pyx_casting(file, var + ".second");
+    type2.emit_cython_pyx_casting_noshared(file, var + ".second");
     file.oss << ")";
   } else {
     type1.emit_cython_pyx_casting(file, var);