-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split pybind declarations and definitions #6869
Conversation
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
All except for This removes 80 C++ types from python binding docs, if I see correctly. Sorry for the huge number of changes required for this, but it is basically just a lot of movement of code. Declarations contain py::module m_camera = m.def_submodule("camera");
py::class_<PinholeCameraIntrinsic> pinhole_intr(
m_camera, "PinholeCameraIntrinsic", "<docs...>");
py::enum_<PinholeCameraIntrinsicParameters> pinhole_intr_params(
m_camera, "PinholeCameraIntrinsicParameters", py::arithmetic(),
"PinholeCameraIntrinsicParameters");
pinhole_intr_params
.value("PrimeSenseDefault",
PinholeCameraIntrinsicParameters::PrimeSenseDefault,
"Default camera intrinsic parameter for PrimeSense.")
... Definitions refer to modules and classes using auto m_camera = static_cast<py::module>(m.attr("camera"));
// open3d.camera.PinholeCameraIntrinsic
auto pinhole_intr = static_cast<py::class_<PinholeCameraIntrinsic>>(
m_camera.attr("PinholeCameraIntrinsic"));
py::detail::bind_default_constructor<PinholeCameraIntrinsic>(pinhole_intr);
py::detail::bind_copy_functions<PinholeCameraIntrinsic>(pinhole_intr);
pinhole_intr
.def(py::init<int, int, const Eigen::Matrix3d>(), "width"_a,
"height"_a, "intrinsic_matrix"_a) Next steps for me will be running style checks and unit tests. |
Applied style checker, run tests on my machine (no extra flags though like CUDA, ...), and added entry to changelog. Please tell me if any action is required on my part or any open questions come up. |
Failed checks:
All other checks were successful. |
Thanks @timohl for this huge effort! This looks like a pretty useful update. I'll check it out. Can you paste some before and after screenshots of how the docs look better after this PR, and how a code editor is able to better do auto-complete with |
PS: Also please consider contributing the script for |
The o3d.core.(blob,kernel,linalg,scalar,sycl) files are split now.
The o3d.core.(blob,kernel,linalg,scalar,sycl) files are split now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @timohl Looks good.
Next step: Package the generated stubs with Open3D.
Type
Motivation and Context
As explained in #6867 there are several instances where C++ types are shown in the Python binding documentation.
Some of those instances can be solved by splitting the
py::_class
andpy::enum_
declarations from the method/function definitions using.def(...)
.This ensures, that all types are properly declared before usage.
Checklist:
python util/check_style.py --apply
to apply Open3D code styleto my code.
updated accordingly.
results (e.g. screenshots or numbers) here.
Description
Split modules:
Style+Testing: