From 4a2d322a734196bfd07f3ff03cc5a91b624d1bf0 Mon Sep 17 00:00:00 2001 From: Varun Agrawal Date: Wed, 21 Apr 2021 00:01:05 -0400 Subject: [PATCH] Squashed 'wrap/' changes from b2144a712..0124bcc45 0124bcc45 Merge pull request #97 from borglab/fix/enums-in-classes f818f94d6 Merge pull request #98 from borglab/fix/global-variables ccc84d3bc some cleanup edf141eb7 assign global variable value correctly ad1d6d241 define class instances for enums 963bfdadd prepend full class namespace e9342a43f fix enums defined in classes 35311571b Merge pull request #88 from borglab/doc/git_subtree b9d2ec972 Address review comments 1f7651402 update `update` documentation to not require manual subtree merge command df834d96b capitalization 36dabbef1 git subtree documentation git-subtree-dir: wrap git-subtree-split: 0124bcc45fa83e295750438fbfd11ddface5466f --- README.md | 12 +++ gtwrap/pybind_wrapper.py | 144 ++++++++++++++++++-------- tests/expected/python/enum_pybind.cpp | 41 ++++++-- tests/fixtures/enum.i | 34 ++++-- 4 files changed, 175 insertions(+), 56 deletions(-) diff --git a/README.md b/README.md index 2f5689db7..442fc2f93 100644 --- a/README.md +++ b/README.md @@ -109,3 +109,15 @@ Arguments: include_directories. Again, normally, leave this empty. - `extraMexFlags`: Any _additional_ flags to pass to the compiler when building the wrap code. Normally, leave this empty. + +## Git subtree and Contributing + +**\*WARNING\*: Running the ./update_wrap.sh script from the GTSAM repo creates 2 new commits in GTSAM. Be sure to _NOT_ push these directly to master/develop. Preferably, open up a new PR with these updates (see below).** + +The [wrap library](https://github.com/borglab/wrap) is included in GTSAM as a git subtree. This means that sometimes the wrap library can have new features or changes that are not yet reflected in GTSAM. There are two options to get the most up-to-date versions of wrap: + 1. Clone and install the [wrap repository](https://github.com/borglab/wrap). For external projects, make sure cmake is using the external `wrap` rather than the one pre-packaged with GTSAM. + 2. Run `./update_wrap.sh` from the root of GTSAM's repository to pull in the newest version of wrap to your local GTSAM installation. See the warning above about this script automatically creating commits. + +To make a PR on GTSAM with the most recent wrap updates, create a new branch/fork then pull in the most recent wrap changes using `./update_wrap.sh`. You should find that two new commits have been made: a squash and a merge from master. You can push these (to the non-develop branch) and open a PR. + +For any code contributions to the wrap project, please make them on the [wrap repository](https://github.com/borglab/wrap). diff --git a/gtwrap/pybind_wrapper.py b/gtwrap/pybind_wrapper.py index 7d0244f06..8f8dde224 100755 --- a/gtwrap/pybind_wrapper.py +++ b/gtwrap/pybind_wrapper.py @@ -210,17 +210,24 @@ class PybindWrapper: return res def wrap_variable(self, - module, + namespace, module_var, variable, prefix='\n' + ' ' * 8): """Wrap a variable that's not part of a class (i.e. global) """ - return '{prefix}{module_var}.attr("{variable_name}") = {module}{variable_name};'.format( + variable_value = "" + if variable.default is None: + variable_value = variable.name + else: + variable_value = variable.default + + return '{prefix}{module_var}.attr("{variable_name}") = {namespace}{variable_value};'.format( prefix=prefix, - module=module, module_var=module_var, - variable_name=variable.name) + variable_name=variable.name, + namespace=namespace, + variable_value=variable_value) def wrap_properties(self, properties, cpp_class, prefix='\n' + ' ' * 8): """Wrap all the properties in the `cpp_class`.""" @@ -254,6 +261,45 @@ class PybindWrapper: op.operator)) return res + def wrap_enum(self, enum, class_name='', module=None, prefix=' ' * 4): + """ + Wrap an enum. + + Args: + enum: The parsed enum to wrap. + class_name: The class under which the enum is defined. + prefix: The amount of indentation. + """ + if module is None: + module = self._gen_module_var(enum.namespaces()) + + cpp_class = enum.cpp_typename().to_cpp() + if class_name: + # If class_name is provided, add that as the namespace + cpp_class = class_name + "::" + cpp_class + + res = '{prefix}py::enum_<{cpp_class}>({module}, "{enum.name}", py::arithmetic())'.format( + prefix=prefix, module=module, enum=enum, cpp_class=cpp_class) + for enumerator in enum.enumerators: + res += '\n{prefix} .value("{enumerator.name}", {cpp_class}::{enumerator.name})'.format( + prefix=prefix, enumerator=enumerator, cpp_class=cpp_class) + res += ";\n\n" + return res + + def wrap_enums(self, enums, instantiated_class, prefix=' ' * 4): + """Wrap multiple enums defined in a class.""" + cpp_class = instantiated_class.cpp_class() + module_var = instantiated_class.name.lower() + res = '' + + for enum in enums: + res += "\n" + self.wrap_enum( + enum, + class_name=cpp_class, + module=module_var, + prefix=prefix) + return res + def wrap_instantiated_class( self, instantiated_class: instantiator.InstantiatedClass): """Wrap the class.""" @@ -261,30 +307,54 @@ class PybindWrapper: cpp_class = instantiated_class.cpp_class() if cpp_class in self.ignore_classes: return "" - return ( - '\n py::class_<{cpp_class}, {class_parent}' - '{shared_ptr_type}::shared_ptr<{cpp_class}>>({module_var}, "{class_name}")' - '{wrapped_ctors}' - '{wrapped_methods}' - '{wrapped_static_methods}' - '{wrapped_properties}' - '{wrapped_operators};\n'.format( - shared_ptr_type=('boost' if self.use_boost else 'std'), - cpp_class=cpp_class, - class_name=instantiated_class.name, - class_parent="{instantiated_class.parent_class}, ".format( - instantiated_class=instantiated_class) - if instantiated_class.parent_class else '', - module_var=module_var, - wrapped_ctors=self.wrap_ctors(instantiated_class), - wrapped_methods=self.wrap_methods(instantiated_class.methods, - cpp_class), - wrapped_static_methods=self.wrap_methods( - instantiated_class.static_methods, cpp_class), - wrapped_properties=self.wrap_properties( - instantiated_class.properties, cpp_class), - wrapped_operators=self.wrap_operators( - instantiated_class.operators, cpp_class))) + if instantiated_class.parent_class: + class_parent = "{instantiated_class.parent_class}, ".format( + instantiated_class=instantiated_class) + else: + class_parent = '' + + if instantiated_class.enums: + # If class has enums, define an instance and set module_var to the instance + instance_name = instantiated_class.name.lower() + class_declaration = ( + '\n py::class_<{cpp_class}, {class_parent}' + '{shared_ptr_type}::shared_ptr<{cpp_class}>> ' + '{instance_name}({module_var}, "{class_name}");' + '\n {instance_name}').format( + shared_ptr_type=('boost' if self.use_boost else 'std'), + cpp_class=cpp_class, + class_name=instantiated_class.name, + class_parent=class_parent, + instance_name=instance_name, + module_var=module_var) + module_var = instance_name + + else: + class_declaration = ( + '\n py::class_<{cpp_class}, {class_parent}' + '{shared_ptr_type}::shared_ptr<{cpp_class}>>({module_var}, "{class_name}")' + ).format(shared_ptr_type=('boost' if self.use_boost else 'std'), + cpp_class=cpp_class, + class_name=instantiated_class.name, + class_parent=class_parent, + module_var=module_var) + + return ('{class_declaration}' + '{wrapped_ctors}' + '{wrapped_methods}' + '{wrapped_static_methods}' + '{wrapped_properties}' + '{wrapped_operators};\n'.format( + class_declaration=class_declaration, + wrapped_ctors=self.wrap_ctors(instantiated_class), + wrapped_methods=self.wrap_methods( + instantiated_class.methods, cpp_class), + wrapped_static_methods=self.wrap_methods( + instantiated_class.static_methods, cpp_class), + wrapped_properties=self.wrap_properties( + instantiated_class.properties, cpp_class), + wrapped_operators=self.wrap_operators( + instantiated_class.operators, cpp_class))) def wrap_stl_class(self, stl_class): """Wrap STL containers.""" @@ -315,18 +385,6 @@ class PybindWrapper: stl_class.properties, cpp_class), )) - def wrap_enum(self, enum, prefix='\n' + ' ' * 8): - """Wrap an enum.""" - module_var = self._gen_module_var(enum.namespaces()) - cpp_class = enum.cpp_typename().to_cpp() - res = '\n py::enum_<{cpp_class}>({module_var}, "{enum.name}", py::arithmetic())'.format( - module_var=module_var, enum=enum, cpp_class=cpp_class) - for enumerator in enum.enumerators: - res += '{prefix}.value("{enumerator.name}", {cpp_class}::{enumerator.name})'.format( - prefix=prefix, enumerator=enumerator, cpp_class=cpp_class) - res += ";\n\n" - return res - def _partial_match(self, namespaces1, namespaces2): for i in range(min(len(namespaces1), len(namespaces2))): if namespaces1[i] != namespaces2[i]: @@ -400,9 +458,11 @@ class PybindWrapper: elif isinstance(element, instantiator.InstantiatedClass): wrapped += self.wrap_instantiated_class(element) + wrapped += self.wrap_enums(element.enums, element) + elif isinstance(element, parser.Variable): - module = self._add_namespaces('', namespaces) - wrapped += self.wrap_variable(module=module, + variable_namespace = self._add_namespaces('', namespaces) + wrapped += self.wrap_variable(namespace=variable_namespace, module_var=module_var, variable=element, prefix='\n' + ' ' * 4) diff --git a/tests/expected/python/enum_pybind.cpp b/tests/expected/python/enum_pybind.cpp index 5e792b211..ffc68ece0 100644 --- a/tests/expected/python/enum_pybind.cpp +++ b/tests/expected/python/enum_pybind.cpp @@ -21,13 +21,23 @@ namespace py = pybind11; PYBIND11_MODULE(enum_py, m_) { m_.doc() = "pybind11 wrapper of enum_py"; + py::enum_(m_, "Color", py::arithmetic()) + .value("Red", Color::Red) + .value("Green", Color::Green) + .value("Blue", Color::Blue); - py::enum_(m_, "Kind", py::arithmetic()) - .value("Dog", Kind::Dog) - .value("Cat", Kind::Cat); + + py::class_> pet(m_, "Pet"); + pet + .def(py::init(), py::arg("name"), py::arg("type")) + .def_readwrite("name", &Pet::name) + .def_readwrite("type", &Pet::type); + + py::enum_(pet, "Kind", py::arithmetic()) + .value("Dog", Pet::Kind::Dog) + .value("Cat", Pet::Kind::Cat); pybind11::module m_gtsam = m_.def_submodule("gtsam", "gtsam submodule"); - py::enum_(m_gtsam, "VerbosityLM", py::arithmetic()) .value("SILENT", gtsam::VerbosityLM::SILENT) .value("SUMMARY", gtsam::VerbosityLM::SUMMARY) @@ -39,10 +49,25 @@ PYBIND11_MODULE(enum_py, m_) { .value("TRYDELTA", gtsam::VerbosityLM::TRYDELTA); - py::class_>(m_gtsam, "Pet") - .def(py::init(), py::arg("name"), py::arg("type")) - .def_readwrite("name", >sam::Pet::name) - .def_readwrite("type", >sam::Pet::type); + py::class_> mcu(m_gtsam, "MCU"); + mcu + .def(py::init<>()); + + py::enum_(mcu, "Avengers", py::arithmetic()) + .value("CaptainAmerica", gtsam::MCU::Avengers::CaptainAmerica) + .value("IronMan", gtsam::MCU::Avengers::IronMan) + .value("Hulk", gtsam::MCU::Avengers::Hulk) + .value("Hawkeye", gtsam::MCU::Avengers::Hawkeye) + .value("Thor", gtsam::MCU::Avengers::Thor); + + + py::enum_(mcu, "GotG", py::arithmetic()) + .value("Starlord", gtsam::MCU::GotG::Starlord) + .value("Gamorra", gtsam::MCU::GotG::Gamorra) + .value("Rocket", gtsam::MCU::GotG::Rocket) + .value("Drax", gtsam::MCU::GotG::Drax) + .value("Groot", gtsam::MCU::GotG::Groot); + #include "python/specializations.h" diff --git a/tests/fixtures/enum.i b/tests/fixtures/enum.i index 97a5383e6..9386a33df 100644 --- a/tests/fixtures/enum.i +++ b/tests/fixtures/enum.i @@ -1,4 +1,13 @@ -enum Kind { Dog, Cat }; +enum Color { Red, Green, Blue }; + +class Pet { + enum Kind { Dog, Cat }; + + Pet(const string &name, Kind type); + + string name; + Kind type; +}; namespace gtsam { enum VerbosityLM { @@ -12,12 +21,25 @@ enum VerbosityLM { TRYDELTA }; -class Pet { - enum Kind { Dog, Cat }; +class MCU { + MCU(); - Pet(const string &name, Kind type); + enum Avengers { + CaptainAmerica, + IronMan, + Hulk, + Hawkeye, + Thor + }; + + enum GotG { + Starlord, + Gamorra, + Rocket, + Drax, + Groot + }; - string name; - Kind type; }; + } // namespace gtsam