shiveshkhaitan / pcl_bindings Goto Github PK

The proof of concept looks interesting. I have some questions, comments and suggestions.

Questions

Tooling Choices

• You picked binder and pybind11. From quickly skimming through binder it seems to address the most fundamental problem of automatically identifying which bindings to generate. PCL already uses boost in its pipeline, so there's a natural gravitation from us to adopt boost.python instead pybind11, by default. Do you have a preference for any of them and why?
• Do you know if there are other binder alike tools which perform the same role but integrate with Boost.Python?

State of the Art

There are currently other projects which provide partial and some full PCL bindings:

• https://github.com/strawlab/python-pcl
• https://github.com/dimatura/pypcd
• https://github.com/davidcaron/pclpy
• https://github.com/cmpute/pcl.py

It's important to research them a little and understand what are their current limitations and why. What sort of tooling they use, what bindings are they exactly offering. Believe me when I say that learning a little bit more about them is not wasted time. Present your conclusions in the proposal.

There's also value in understanding how other projects solved the issue:

• OpenCV
• VTK
• Open3D

Comments

Package organization

PCL's modules naturally fall into common python package/module organization. In this proof of concept, the ideal syntax would have been to have TimeTrigger under pcl.common.TimeTrigger

from pcl.common import TimeTrigger # this is my ideal usage

A good rule of thumb you can use to understand how to organize things is to look at header paths e.g.: pcl::PointXYZ is defined under <pcl/point_types.h> and as such I would expect it to reside under pcl.PointXYZ; TimeTrigger on the other hand is under <pcl/common/time_trigger.h> and hence it should reside in pcl.common.TimeTrigger

Build system

PCL uses CMake as its build system and everything you did on your shell script, in theory can be accomplished with CMake: finding dependencies and launching your build commands. I would recommend to migrate the shell script to a proper CMake approach

Next Step Suggestions

1. CMake adoption: what I mentioned earlier, to be merged in to the project, these bindings need to be generated for all our platforms. CMake will help you accomplish that.
2. Make bindings for a full PCL module -> common. This one might be a steep curve but it will help you identify more problems that are yet to be solved.