trelau / pyOCCT_binder

Binding generator for pyOCCT project
GNU Lesser General Public License v2.1
7 stars 1 forks source link

Should bind_SomeClass.hxx functions check for duplicates? #11

Closed frmdstryr closed 4 years ago

frmdstryr commented 4 years ago

I started hitting https://github.com/pybind/pybind11/issues/439

For whatever reason (I probably deleted a line by accident) in BOPTools.cxx it's calling bind_BOPTools_BoxSelector with different dimensions:

// TYPEDEF: BOPTOOLS_BOX2DTREESELECTOR
bind_BOPTools_BoxSelector<2>(mod, "BOPTools_Box2dTreeSelector", py::module_local(false));

// TYPEDEF: BOPTOOLS_BOX2DPAIRSELECTOR
/*
bind_BOPTools_PairSelector<2>(mod, "BOPTools_Box2dPairSelector", py::module_local(false));
*/

// TYPEDEF: BOPTOOLS_BOXTREE
/*
bind_BOPTools_BoxSet<double, 3, int>(mod, "BOPTools_BoxTree", py::module_local(false));
*/

// TYPEDEF: BOPTOOLS_BOXTREESELECTOR
bind_BOPTools_BoxSelector<3>(mod, "BOPTools_BoxTreeSelector", py::module_local(false));

bind_BOPTools_BoxSelector calls bind_BVH_Traverse, which calls bind_BVH_Traverse which finally calls bind_BVH_BaseTraverse with a fixed name of "BVH_Traverse_Base" .

This leads to the following error in import:

----> 1 import OCCT.BOPTools

ImportError: generic_type: cannot initialize type "BVH_Traverse_Base": an object with that name is already defined

So I think a patch that checks for duplicates in the binder is needed


template <typename MetricType>
void bind_BVH_BaseTraverse(py::module &mod, std::string const &name, py::module_local const &local){

if (py::hasattr(mod, name.c_str())) return; // Don't try making a duplicate
py::class_<BVH_BaseTraverse<MetricType>, std::unique_ptr<BVH_BaseTraverse<MetricType>, py::nodelete>> cls_BVH_BaseTraverse(mod, name.c_str(), "Abstract class implementing the base Traverse interface required for selection of the elements from BVH tree.", local);

// Methods"Standard_Character"
cls_BVH_BaseTraverse.def("IsMetricBetter", (Standard_Boolean (BVH_BaseTraverse<MetricType>::*)(const MetricType &, const MetricType &) const) &BVH_BaseTraverse<MetricType>::IsMetricBetter, "Compares the two metrics and chooses the best one. Returns true if the first metric is better than the second, false otherwise.", py::arg(""), py::arg(""));
cls_BVH_BaseTraverse.def("RejectMetric", (Standard_Boolean (BVH_BaseTraverse<MetricType>::*)(const MetricType &) const) &BVH_BaseTraverse<MetricType>::RejectMetric, "Rejects the node by the metric", py::arg(""));
cls_BVH_BaseTraverse.def("Stop", (Standard_Boolean (BVH_BaseTraverse<MetricType>::*)() const) &BVH_BaseTraverse<MetricType>::Stop, "Returns the flag controlling the tree descend. Returns true if the tree descend should be stopped.");

}

I'm not sure if it makes sense to add this everywhere or just patch that one binder?

frmdstryr commented 4 years ago

Well tried that, it fixed that error but hit another of the same thing.

test/test_BRepAlgoAPI.py:22: in <module>
    from OCCT.BRepAlgoAPI import BRepAlgoAPI_Fuse
E   ImportError: ImportError: ImportError: generic_type: type "BOPTools_BoxTreeSelector" referenced unknown base type "BVH_Traverse<double, 3, BVH_BoxSet<double, 3, int>, bool>"

I think it needs to use generated names based on the template params, just going to remove that function for now.

frmdstryr commented 4 years ago

Ah, found the typo, looks like something got pasted where it shouldn't. It's working fine with that function disabled.