Skip to content
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

Fixes for pybind11-stubgen errors #6896

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Conversation

timohl
Copy link
Contributor

@timohl timohl commented Jul 26, 2024

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

This pull request is a follow up for/based on #6869 and contains more fixes for documentation and typing generation.
Related issues are #3052 and #6867.

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description

Typings are generated using:

# Create venv
python -m venv .venv
. .venv/bin/activate
pip install python/requirements.txt
pip install python/requirements_build.txt

# Build open3d and install in .venv
mkdir build && cd build
cmake ..
make -j$(nproc) && make install-pip-package

# Generate typings
pip install pybind11-stubgen
pybind11-stubgen open3d -o typings

@timohl
Copy link
Contributor Author

timohl commented Jul 26, 2024

I split this from #6869 because it contains changes that actually modify the bindings a bit:
Some C++ enums are exposed and the output of some __repr__ methods changes.
Also, some errors are still work in progress.

Might be easier to first merge #6869 and then discuss further changes here (with more detailed explanation from me).

@timohl
Copy link
Contributor Author

timohl commented Aug 14, 2024

Rebased since #6869 was merged.

I moved the integration of stubs into the python package to #6917, since it is kind of separate and might require more investigation to have it work properly.

My plan is to finish most fixes tomorrow, and then I am going to explain all changes in detail.

@timohl
Copy link
Contributor Author

timohl commented Aug 25, 2024

ToGoogleDocString strips any type annotations from function signatures, so every docstring::ClassMethodDocInject(...) call strips typing info.
703bead fixes this, resulting in more types displayed in documentation, as well as, more types in generated stubs.
After 703bead pybind11-stubgen produces new errors (due to now checking all previously stripped methods):

pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.core.Tensor.from_dlpack : Can't find/import 'capsule'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.core.Tensor.to_dlpack : Can't find/import 'capsule'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.__init__ : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.__init__ : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_hybrid_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_knn_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_radius_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.set_matrix_data : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.set_matrix_data : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.detect_planar_patches : Invalid expression 'open3d.geometry.KDTreeSearchParam = KDTreeSearchParamKNN with knn'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.estimate_covariances : Invalid expression 'open3d.geometry.KDTreeSearchParam = KDTreeSearchParamKNN with knn'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.estimate_normals : Invalid expression 'open3d.geometry.KDTreeSearchParam = KDTreeSearchParamKNN with knn'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.estimate_point_covariances : Invalid expression 'open3d.geometry.KDTreeSearchParam = KDTreeSearchParamKNN with knn'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.voxel_down_sample_and_trace : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.voxel_down_sample_and_trace : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.TriangleMesh.deform_as_rigid_as_possible : Invalid expression '<DeformAsRigidAsPossibleEnergy.Spokes: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.TriangleMesh.filter_sharpen : Invalid expression '<FilterScope.All: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.TriangleMesh.filter_smooth_laplacian : Invalid expression '<FilterScope.All: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.TriangleMesh.filter_smooth_simple : Invalid expression '<FilterScope.All: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.TriangleMesh.filter_smooth_taubin : Invalid expression '<FilterScope.All: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.TriangleMesh.simplify_vertex_clustering : Invalid expression '<SimplificationContraction.Average: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.odometry.compute_correspondence : Invalid expression 'open3d.pipelines.odometry.OdometryOption = OdometryOption class. iteration_number_per_pyramid_level = [ 20, 10, 5, ] depth_diff_max = 0.030000 depth_min = 0.000000 depth_max'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.odometry.compute_rgbd_odometry : Invalid expression 'open3d.camera.PinholeCameraIntrinsic = PinholeCameraIntrinsic with width = -1 and height'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.odometry.compute_rgbd_odometry : Invalid expression '-1. Access intrinsics with intrinsic_matrix.'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.odometry.compute_rgbd_odometry : Invalid expression 'open3d.pipelines.odometry.OdometryOption = OdometryOption class. iteration_number_per_pyramid_level = [ 20, 10, 5, ] depth_diff_max = 0.030000 depth_min = 0.000000 depth_max'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_colored_icp : Invalid expression 'open3d.pipelines.registration.TransformationEstimationForColoredICP = TransformationEstimationForColoredICP with lambda_geometric'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_colored_icp : Invalid expression 'open3d.pipelines.registration.ICPConvergenceCriteria = ICPConvergenceCriteria class with relative_fitness'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_fgr_based_on_correspondence : Invalid expression 'open3d.pipelines.registration.FastGlobalRegistrationOption = FastGlobalRegistrationOption class with division_factor= tuple_test={} use_absolute_scale=1.4 decrease_mu=false maximum_correspondence_distance=true iteration_number=0.025 tuple_scale=64 maximum_tuple_count'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_fgr_based_on_feature_matching : Invalid expression 'open3d.pipelines.registration.FastGlobalRegistrationOption = FastGlobalRegistrationOption class with division_factor= tuple_test={} use_absolute_scale=1.4 decrease_mu=false maximum_correspondence_distance=true iteration_number=0.025 tuple_scale=64 maximum_tuple_count'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_generalized_icp : Invalid expression 'open3d.pipelines.registration.TransformationEstimationForGeneralizedICP = TransformationEstimationForGeneralizedICP with epsilon'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_generalized_icp : Invalid expression 'open3d.pipelines.registration.ICPConvergenceCriteria = ICPConvergenceCriteria class with relative_fitness'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_icp : Invalid expression 'open3d.pipelines.registration.ICPConvergenceCriteria = ICPConvergenceCriteria class with relative_fitness'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_ransac_based_on_correspondence : Invalid expression 'open3d.pipelines.registration.RANSACConvergenceCriteria = RANSACConvergenceCriteria class with max_iteration'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.registration_ransac_based_on_feature_matching : Invalid expression 'open3d.pipelines.registration.RANSACConvergenceCriteria = RANSACConvergenceCriteria class with max_iteration'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.t.pipelines.slam.Model.track_frame_to_model : Invalid expression '<Method.PointToPlane: 0>'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.t.pipelines.slam.Model.track_frame_to_model : Invalid expression '[OdometryConvergenceCriteria[max_iteration=6, relative_rmse=1.000000e-06, relative_fitness=1.000000e-06]., OdometryConvergenceCriteria[max_iteration=3, relative_rmse=1.000000e-06, relative_fitness=1.000000e-06]., OdometryConvergenceCriteria[max_iteration=1, relative_rmse=1.000000e-06, relative_fitness=1.000000e-06].]'
pybind11_stubgen - [WARNING] Enum-like str representations were found with no matching mapping to the enum class location.
Use `--enum-class-locations` to specify full path to the following enum(s):
 - DeformAsRigidAsPossibleEnergy
 - SimplificationContraction
 - Method
 - FilterScope
pybind11_stubgen - [WARNING] Raw C++ types/values were found in signatures extracted from docstrings.
Please check the corresponding sections of pybind11 documentation to avoid common mistakes in binding code:
 - https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-cpp-types-in-docstrings
 - https://pybind11.readthedocs.io/en/latest/advanced/functions.html#default-arguments-revisited

I fixed most of these errors so now the remaining pybind11-stubgen errors are:

pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.core.Tensor.from_dlpack : Can't find/import 'capsule'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.core.Tensor.to_dlpack : Can't find/import 'capsule'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.__init__ : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.__init__ : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_hybrid_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_knn_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_radius_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.search_vector_xd : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.set_matrix_data : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.KDTreeFlann.set_matrix_data : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.voxel_down_sample_and_trace : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.geometry.PointCloud.voxel_down_sample_and_trace : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'n'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'm'
pybind11_stubgen - [  ERROR] In open3d.cpu.pybind.pipelines.registration.Feature.data. : Can't find/import 'n'

pybind11-stubgen seems to have problems with numpy annotations using m and n as dimensions, e.g., produced by Eigen::MatrixXd.
Also capsule seems to be problematic.

@timohl
Copy link
Contributor Author

timohl commented Aug 25, 2024

I do not know how to fix the last two errors right now, so for now I will have to leave them and maybe fix them later.
Now I will try to summarize and describe the changes, so review will be easier.

@timohl
Copy link
Contributor Author

timohl commented Aug 25, 2024

All changes can be summarized as follows:

  • Fixed ToGoogleDocString to not strip type information (see 703bead)
  • Fixed parser bug when docstrings contain , (20c1855, see strange default value for robust param in 0.18.0 docs)
  • Change __repr__ methods to output valid constructor call. Otherwise, stubs contain invalid code in default values.
  • Using py::arg_v for enums in default values. Otherwise, enums will be represented as <[Enum.Name]: [Value]> which is invalid in python.
  • Added missing enum/class binding for DtypeCode, TextureHandle

@timohl timohl marked this pull request as ready for review August 25, 2024 11:43
@timohl
Copy link
Contributor Author

timohl commented Aug 31, 2024

Fixed the last remaining errors:

Now pybind11-stubgen is able to run without any errors.
Maybe a CI workflow with IGNORE_STUBGEN_ERRORS=OFF could be added in the future when #6917 is resolved to prevent any regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant