Skip to content

Commit 487facb

Browse files
authored
Merge pull request #1 from PointCloudLibrary/devel/python_bindings
Changes from the development at PCL repo
2 parents 5d34035 + d84b908 commit 487facb

File tree

15 files changed

+1777
-0
lines changed

15 files changed

+1777
-0
lines changed

.github/workflows/black.yml

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
name: Black Formatting
2+
3+
on: [push]
4+
5+
# A workflow run is made up of one or more jobs that can run sequentially or in parallel
6+
jobs:
7+
Black:
8+
# The type of runner that the job will run on
9+
runs-on: ubuntu-latest
10+
11+
steps:
12+
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
13+
- uses: actions/checkout@v2
14+
15+
# Setup python version
16+
- name: Set up Python
17+
uses: actions/setup-python@v2
18+
with:
19+
python-version: "3.8"
20+
21+
# Install pip and packages
22+
- name: Install pip
23+
run: python -m pip install --upgrade pip
24+
25+
- name: Install black
26+
run: pip install black
27+
28+
# Format with black
29+
- name: Format with black
30+
run: |
31+
cd bindings/python
32+
black .
33+
34+
# Diff check
35+
- name: Check git diff
36+
# Output the diff in ${GITHUB_WORKSPACE}
37+
run: git diff > black_formatting_diff.patch
38+
39+
# Exit if diff
40+
- name: Set job exit status
41+
run: "[ ! -s black_formatting_diff.patch ]"
42+
43+
# Artifacts
44+
- name: Upload formatting diff
45+
uses: actions/upload-artifact@v2
46+
with:
47+
# We are in ${GITHUB_WORKSPACE}
48+
# ${GITHUB_SHA} won't work: use ${{ github.sha }}
49+
name: black_formatting_diff-${{ github.sha }}
50+
path: black_formatting_diff.patch
51+
# Use always() to always run this step to publish test results when there are test failures
52+
if: ${{ always() }}

.github/workflows/pytest.yml

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
name: Pytest
2+
3+
on: [push]
4+
5+
# A workflow run is made up of one or more jobs that can run sequentially or in parallel
6+
jobs:
7+
Pytest:
8+
# The type of runner that the job will run on
9+
runs-on: ubuntu-latest
10+
strategy:
11+
matrix:
12+
python-version: [3.6, 3.7, 3.8]
13+
14+
steps:
15+
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
16+
- uses: actions/checkout@v2
17+
18+
# Setup python version
19+
- name: Set up Python ${{ matrix.python-version }}
20+
uses: actions/setup-python@v2
21+
with:
22+
python-version: ${{ matrix.python-version }}
23+
- name: Display python version
24+
run: python -c "import sys; print(sys.version)"
25+
26+
# Install pip and packages
27+
- name: Install pip
28+
run: python -m pip install --upgrade pip
29+
30+
- name: Install pytest
31+
run: pip install pytest
32+
33+
# Install clang and it's python inteface via apt
34+
- name: Add llvm keys
35+
run: |
36+
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
37+
echo 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-11 main' | sudo tee -a /etc/apt/sources.list
38+
echo 'deb-src http://apt.llvm.org/bionic/ llvm-toolchain-bionic-11 main' | sudo tee -a /etc/apt/sources.list
39+
- name: Install libclang and its python bindings
40+
run: |
41+
sudo apt-get update
42+
sudo apt-get install -y libclang-11-dev python3-clang-11
43+
44+
# Add dist-package to path to enable apt installed python3-clang import
45+
- name: Add dist-packages to PYTHONPATH
46+
run: echo "::set-env name=PYTHONPATH::${PYTHON_PATH}:/usr/lib/python3/dist-packages"
47+
- name: Display PYTHONPATH
48+
run: python -c "import sys; print('\n'.join(sys.path))"
49+
50+
# Test with pytest
51+
- name: Test with pytest
52+
run: cd ${_PYTHON_BINDINGS_PATH} && pytest --junitxml=${GITHUB_WORKSPACE}/result_${{ matrix.python-version }}.xml
53+
env:
54+
_PYTHON_BINDINGS_PATH: bindings/python
55+
56+
# Artifacts
57+
- name: Upload pytest test results
58+
uses: actions/upload-artifact@v2
59+
with:
60+
# We are in ${GITHUB_WORKSPACE}
61+
# ${GITHUB_SHA} won't work: use ${{ github.sha }}
62+
name: pytest_py${{ matrix.python-version }}-${{ github.sha }}
63+
path: result_${{ matrix.python-version }}.xml
64+
# Use always() to always run this step to publish test results when there are test failures
65+
if: ${{ always() }}

bindings/discussion.md

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
## Python bindings
2+
3+
### Shared pointer usage
4+
#### [05-06-2020]
5+
- > When we want `cloud = pcl.PointCloud[pcl.PointXYZ]()` to be a shared_ptr by default, we set the holder class as shared_ptr. This is needed in some cases because the interface otherwise would be non-idomatic:
6+
> ```py
7+
> import pcl
8+
>
9+
> filter = pcl.filters.PassThrough[pcl.PointXYZ]()
10+
> filter.setInput(cloud)
11+
> ```
12+
>
13+
> Here, cloud needs to be a shared_ptr. That can be done in 2 ways
14+
> 1. `cloud = pcl.PointCloud[pcl.PointXYZ]()` and the holder class as shared_ptr
15+
> 2. `cloud = pcl.make_shared[pcl.PointXYZ]()` and holder class as void
16+
>
17+
> The issue is the ease-of-use and expected semantics
18+
> ```py
19+
> cloud2 = cloud1 # Python user will assume this is a shallow copy
20+
> ```
21+
>
22+
> This will only be true for a variable held in a shared_ptr. This is the semantics in Python.
23+
>
24+
> However, wrapping everything in shared_ptr has downsides for C++ wrapper:
25+
> ```py
26+
> import pcl
27+
>
28+
> point = pcl.PointXYZ()
29+
> cloud[100] = point
30+
> ```
31+
>
32+
> If PointXYZ is held in a shared_ptr... things go south. If not, things go south
33+
34+
### Handling unions
35+
#### [04-06-2020]
36+
- > given `assert(&(union.r) == &(union.rgb));` does the following hold:
37+
> `assert id(union_wrapper.r) == id(union_wrapper.rgb) ?`
38+
Yes. Tested.
39+
- Working example:
40+
```cpp
41+
#include <pybind11/pybind11.h>
42+
namespace py = pybind11;
43+
44+
union RGB {
45+
int rgb;
46+
struct {
47+
int r;
48+
int g;
49+
int b;
50+
};
51+
};
52+
53+
PYBIND11_MODULE(pcl, m)
54+
{
55+
py::class_<RGB>(m, "RGB")
56+
.def(py::init<>())
57+
.def_property(
58+
"rgb",
59+
[](RGB& self) -> int { return self.rgb; },
60+
[](RGB& self, int value) { self.rgb = value; })
61+
.def_property(
62+
"r",
63+
[](RGB& self) -> int { return self.r; },
64+
[](RGB& self, int value) { self.r = value; })
65+
.def_property(
66+
"g",
67+
[](RGB& self) -> int { return self.g; },
68+
[](RGB& self, int value) { self.g = value; })
69+
.def_property(
70+
"b",
71+
[](RGB& self) -> int { return self.b; },
72+
[](RGB& self, int value) { self.b = value; });
73+
}
74+
```
75+
76+
### General
77+
#### [05-06-2020]
78+
- MetaCPP relies on Clang's LibTooling to generate all the metadata: https://github.com/mlomb/MetaCPP
79+
80+
#### [04-06-2020]
81+
- > Was reading this yesterday: https://peerj.com/articles/cs-149.pdf
82+
>
83+
> Summary:
84+
> - They too, automatically generate Python bindings for C++ code using Boost::Python.
85+
> - The whole process is python based.
86+
> - Same design as to what we have in mind i.e., parse, process, and generate.
87+
> - Their data structure of choice for the whole process is Abstract Semantic Graph: https://github.com/StatisKit/AutoWIG/blob/master/src/py/autowig/asg.py
88+
> - The architecture is plugin-based, and somebody added a pybind plugin some months back: https://github.com/StatisKit/AutoWIG/blob/master/src/py/autowig/pybind11_generator.py
89+
> - They use libclang for frontend parsing(python API). The project was done some time back so they wrote their own py-libclang code: https://github.com/StatisKit/AutoWIG/blob/master/src/py/autowig/libclang_parser.py
90+
> - Repo: https://github.com/StatisKit/AutoWIG
91+
>
92+
> I think it can act as a good reference for the project. Have a look at the pdf and the source if you wish.
93+
> The libclang python part can be explored from their repo as of now (as python-libclang has no documentation whatsoever and the 1-2 example articles are outdated)
94+
- Problems:
95+
> Templates:
96+
> * suffixing doesn't work well. Unless you're a fan of the pseudo-Hungarian notation espoused by older devs (and by MS). It's ok for 1 (or maybe 2) arguments.
97+
> * Templates in Python represent an instantiation of C++ template. It should be easy to add/remove an instantiation without affecting needless code. It should also be visually easy to switch the template type without having to lookup the notation or count underscores
98+
> * Python has a strong syntax for this: index lookup via __getitem__ and __setitem__
99+
> * Using strings as keys is bad because the editor can't help if spelling is wrong. Pandas MultiKey failed here.
100+
- Use of a templating engine for pybind11 code gen (jinja2>mako)
101+
102+
#### [03-06-2020]
103+
- Ambiguity in the phrase: "full control over clang's AST"
104+
105+
#### [02-06-2020]
106+
- Use of python bindings of libclang, for faster prototyping: https://github.com/llvm/llvm-project/blob/master/clang/bindings/python/clang/cindex.py
107+
- > protyping and exploring python bindings in which everything is runtime and can be done interactively would usually be my first approach
108+
109+
#### [28-05-2020]
110+
- > While reading lsst's documentation, came to find out they use a __str__ method:
111+
> ```py
112+
> cls.def("__str__", [](Class const& self) {
113+
> std::ostringstream os;
114+
> os << self;
115+
> return os.str();
116+
> });
117+
> ```
118+
- > the << operator with ostreams is the most common way in C++ of extracting a string representation of a given object (I have no idea why there's no practice of implementing the cast to string method),
119+
That being said I believe you can use << with std::stringstreams, effectively allowing you to fetch a string representation of PCL objects which have operator<< (std::ostream, ....) implemented.
120+
121+
#### [15-05-2020]
122+
- > You can create docstring from \brief part and copy the function signature via libtooling.
123+
124+
#### [09-05-2020]
125+
- Start with binding PointTypes.
126+
- AST parsing helps in cases of convoluted code.
127+
- > We can keep 2 approaches in parallel:
128+
> 1. header parser on a limited number of files
129+
> 2. libtooling to replace it
130+
> 1st will allow the pipeline to be developed
131+
> 2nd will replace that
132+
- > We can make a prototype which works on manually provided API points
133+
- > From my understanding:
134+
> 1. Code -> AST -> JSON: use some tool for it first, then replace with libtooling
135+
> 2. JSON -> cpp: Python tool, language dependent
136+
> 3. CMake + compile_database.json: rest of toolchain
137+
> 4. organize properly so usage in LANG is nice
138+
139+
#### [05-05-2020]
140+
- > I'd put PyPi integration immediately after we get 1 module working. That'd allow us to keep shipping improved bindings after GSoC (in case the timeline gets delayed)
141+
The order in which modules are tackled should be the dependency order (because we don't have the popularity data from our users)
142+
143+
***
144+
145+
## Javascript Bindings
146+
#### [05-05-2020]
147+
- Webassembly as an option: https://en.wikipedia.org/wiki/WebAssembly
148+
- Emscripten as an option: https://emscripten.org/
149+
- > * Getting clang to compile to WebAsm will be the best "performance".
150+
> * Using Emscripten on the other hand is a well-travelled road, but the performance will be similar to hand written JS (or worse).
151+
> Both approaches need clang so that's a milestone we need to conquer first.
152+
153+
***
154+
155+
## Jupyter Visualisation
156+
#### [05-05-2020]
157+
- > WebGL view straddles JS bindings and Python bindings. It should come before JS bindings in terms of priority keeping in mind the popularity of Python as a second language for the kind of C++ users PCL attracts (academia)
158+
- > https://jupyter.org/widgets has pythreejs which is embedding js in python. .... That's 1 step above webgl, involves using JS in Py, but is faster to get up and running.... We can tackle this based on time when we reach some stage

bindings/python/CMakeLists.txt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
cmake_minimum_required(VERSION 3.5)
2+
project(bindings)
3+
4+
# find_package(PCL REQUIRED)
5+
find_package(PCL COMPONENTS common REQUIRED)
6+
7+
# We can replace `find_package` with `add_subdirectory`, depending on usage.
8+
# https://pybind11.readthedocs.io/en/stable/compiling.html#find-package-vs-add-subdirectory
9+
find_package(pybind11)
10+
11+
pybind11_add_module(pcl ${CMAKE_CURRENT_SOURCE_DIR}/pybind11-gen/common/include/pcl/impl/point_types.cpp)
12+
13+
target_link_libraries(pcl PRIVATE ${PCL_LIBRARIES})
14+
# add_dependencies(pcl_demo some_other_target)

bindings/python/README.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
## Python bindings for PCL
2+
3+
### common-archive
4+
- `point_types.json`: contains JSON based meta of `point_types.h` and `point_types.hpp`.
5+
- `py_point_types.cpp`: file generated by `generate_bindings.py` taking meta info from `point_types.json`.
6+
- `union_test.cpp`: testing file for handling of unions.
7+
8+
### generate_bindings.py
9+
- Converts JSON data to pybind11 C++ code.
10+
- Run:
11+
```py
12+
python3 generate_bindings.py <path/to/json>
13+
```
14+
15+
### CMakeLists.txt
16+
- Finding PCL and pybind11, then adding the python module(s) via `pybind11_add_module` (which is a wrapper over `add_library`).
17+
18+
### setup.py
19+
- For using setuptools. Uses `CMakeLists.txt`.
20+
21+
### libclang.py
22+
- Using python libclang to parse C++ code, for generation of metadata by static analysis.
23+
- Run:
24+
```py
25+
python3 libclang.py <path/to/file>
26+
```
27+
28+
### json/*
29+
- JSON output generated by `libclang.py`
30+
31+
### pybind11/*
32+
- pybind11 C++ outupt generated by `generate_bindings.py`

bindings/python/scripts/__init__.py

Whitespace-only changes.

bindings/python/scripts/context.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import os
2+
import sys
3+
4+
sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..")))
5+
6+
import scripts

0 commit comments

Comments
 (0)