[GSoC 2020] Extending PCL for use with Python: Bindings generation using Pybind11 #4366

diivm · 2020-08-29T15:44:41Z

PR for the project "Extending PCL for use with Python: Bindings generation using Pybind11" during GSoC 2020.

…ng now

- add type bounds - comments - named args

`generate.py` changes

kunaltyagi

Mark as default till other issues are cleared on your fork?

kunaltyagi · 2020-08-29T16:07:30Z

.github/workflows/black.yml

+      - name: Format with black
+        run: |
+          cd bindings/python
+          black .


Can we run black on .dev/scripts too?

kunaltyagi · 2020-08-29T16:08:22Z

.github/workflows/black.yml

+      - name: Set up Python
+        uses: actions/setup-python@v2
+        with:
+          python-version: "3.8"


Is this needed? Can we have something like "latest" so we don't need to update the version?

kunaltyagi · 2020-08-29T16:09:17Z

.github/workflows/black.yml

@@ -0,0 +1,52 @@
+name: Black Formatting
+
+on: [push]


Restrict to directories with python files

kunaltyagi · 2020-08-29T16:10:32Z

bindings/discussion.md

+  > 
+  > Here, cloud needs to be a shared_ptr. That can be done in 2 ways
+  > 1. `cloud = pcl.PointCloud[pcl.PointXYZ]()` and the holder class as shared_ptr
+  > 2. `cloud = pcl.make_shared[pcl.PointXYZ]()` and holder class as void


cloud = pcl.make_shared[pcl.PointCloud[pcl.PointXYZ]]()

kunaltyagi · 2020-08-29T16:10:56Z

bindings/discussion.md

+  > 
+  > Here, cloud needs to be a shared_ptr. That can be done in 2 ways
+  > 1. `cloud = pcl.PointCloud[pcl.PointXYZ]()` and the holder class as shared_ptr
+  > 2. `cloud = pcl.make_shared[pcl.PointXYZ]()` and holder class as void


void and shared_ptr (back tick the classes)

kunaltyagi · 2020-08-29T16:21:05Z

bindings/discussion.md

@@ -0,0 +1,158 @@
+## Python bindings


Should this file be included in the PR as is or modified?

kunaltyagi · 2020-08-29T16:22:06Z

bindings/python/README.md

+	```
+
+### json/*
+- JSON o/p generated by `libclang.py`


kunaltyagi · 2020-08-29T16:22:22Z

bindings/python/README.md

+- JSON o/p generated by `libclang.py`
+
+### pybind11/* 
+- pybind11 cpp o/p generated by `generate_bindings.py`


aPonza · 2020-08-30T17:51:17Z

.github/workflows/pytest.yml

+          echo 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-11 main' | sudo tee -a /etc/apt/sources.list
+          echo 'deb-src http://apt.llvm.org/bionic/ llvm-toolchain-bionic-11 main' | sudo tee -a /etc/apt/sources.list


@kunaltyagi can/should these two bionic references be extrapolated to an outer scope/file now that we're merging with master?

Replace the name with lsb_release -cs and install lsb-release via apt

bindings/python/README.md

aPonza · 2020-08-30T18:10:48Z

bindings/python/scripts/generate.py

+    Class containing functions for generating bindings from AST info.
+
+    How to use:
+        - to_be_filled_after_updation


you mean after this merge or even later?

bindings/python/scripts/parse.py

Co-authored-by: aPonza <[email protected]>

kunaltyagi · 2020-09-08T19:19:21Z

bindings/python/tests/test_generate.py

+
+
+"""
+TODO


Please use sphinx supported semantics:

https://www.sphinx-doc.org/en/master/usage/extensions/todo.html

Also, add all TODOs as issues on PCL too (1 per area of interest)

kunaltyagi · 2020-09-08T19:20:27Z

bindings/python/tests/test_parse.py

+    assert delete_constructor["kind"] == "CONSTRUCTOR"
+    assert delete_constructor["name"] == "aClass"
+    assert delete_constructor["result_type"] == "void"
+    # no check available for deleted ctor analogous to `is_default_constructor`


TODO:

Add test for template instantiation parsing since that's bread and butter for PCL bindings

kunaltyagi · 2020-09-08T19:21:57Z

bindings/python/scripts/utils.py

+    else:
+        args = None
+
+    args = parser.parse_args()


Use parse_known_args() in order to compose together several parsers

kunaltyagi · 2020-09-08T19:23:56Z

bindings/python/scripts/parse.py

+        output_filepath = utils.get_output_path(
+            source=source,
+            output_dir=utils.join_path(args.json_output_path, "json"),
+            split_from="pcl",


split_from is another user input. This is technically base_path or something similar

kunaltyagi · 2020-09-08T19:24:10Z

bindings/python/scripts/utils.py

+        - args: Parsed command line arguments
+    """
+
+    if script == "parse":


Use add_subparsers feature of argparse

larshg · 2020-09-09T05:17:56Z

bindings/python/README.md


 ### pybind11/* 
- pybind11 cpp o/p generated by `generate_bindings.py`
+- pybind11 C++ outupt generated by `generate_bindings.py`


Kaju-Bubanja · 2020-09-14T09:59:20Z

How is the state of this PR? The whole projects sounds amazing and looking at the commits it's looks quite promising. Based on the proposed timeline it should be close to completion? I'm starting a new project and wanted to use PCL. I thought I have to use C++, but looking at this, this looks like it could be the first Python binding that works.

kunaltyagi · 2020-09-14T12:08:32Z

Glad you like this :)

Regarding usability: This is a WIP. There are some upstream clang issues (clang doesn't parse = delete;, etc.), some integration issues (no end-to-end cmake) and a few chinks to smoothen. If you tinker with this, you might get common module bound, but beyond that might be a bit difficult (since even we haven't experimented till there)

You're welcome to contribute to this to make it better, faster 😄

PS: This is product of an awesome GSoC project by @divmadan.

Kaju-Bubanja · 2020-09-14T13:04:48Z

Thank you for the quick answer. Based on this I think I will have to go ahead with C++. Sadly my knowledge in the area of writing and getting bindings to work is non existent and I don't think I will get time to work on that.

stale · 2020-12-19T08:47:34Z

Marking this as stale due to 30 days of inactivity. Commenting or adding a new commit to the pull request will revert this.

ArkaprabhaChakraborty · 2022-01-12T21:12:46Z

Is this complete? or are there any works left?

shelper · 2022-02-28T18:57:25Z

Is this complete? or are there any works left?

same question here, should this issue be closed by PointCloudLibrary/clang-bind#1 ?
or it is a different one? seems to me they are almost the same

jiapei-nexera · 2022-06-17T16:38:50Z

Is this still ongoing? 2 years already?

diivm added 30 commits May 22, 2020 03:35

[point_types.json] add point_types.json

7981a1c

[point_types.json] update to a part of hpp implementation

cab3031

[point_types.json] add parent structs and friendly operators

c7498c1

[point_types.json] change to parent

ed344c6

[generate_binding.py] start generate_binding.py

066a5ce

[generate_binding.py] ultra rough

65f3d19

[generate_binding.py] format and add global

40eab68

[py_point_types.cpp] add output for point_types

afbc1c7

[py_point_types.cpp] corrected for namespaces and operators

4f37afd

[generate_binding.py] rearrange

b1e3cc9

[generate_binding.py] change namespace to braces form

ff4a689

[generate_binding.py] remove operator handling cause errors

37acd51

[generate_binding.py] fix parameters to just use type

1e19e67

[py_point_types.cpp] after some fixes

b432122

[point_types.json] fix change parameters to parameters_types

4a2a548

[point_types.json] fix add meta for parent

0b2dfdf

[setup.py] add build files. building, but not importing

0994867

[CMakeLists.txt] add build files. building, but not importing

421cf62

[CMakeLists.txt] change module name to fix import error. import worki…

3598b20

…ng now

[Dockerfile] minimum working dockerfile

5bdd5ca

[Makefile] init Makefile

ff92a9e

[parse.cpp] minimum compiling solution

fedded1

[discussion.md] add discussion.md

29eb197

[Dockerfile] updates

a4ab71f

[libclang.py] initial grep example

6f4bf43

[libclang.py] easy print

f2bdfad

refactor folder

b8c1a6b

[union_test.cpp] union_test for pybind11

c915cd1

[python/README.md] init info

8c307e9

[tooling/README.md] init info

5d7b8cf

diivm and others added 12 commits August 26, 2020 18:56

[generate.py] move combine_lines out of the if loop

d074f13

[generate.py] Add option of directly operating on dicts apart from JSONs

17298d2

[test_generate.py] generate_bindings: operate directly on dict

fa2b67b

[generate.py] minor changes

68bfb14

- add type bounds - comments - named args

[generate.py] range to enumerate

933da99

[generate.py] remove self from static method

9a78047

[generate.py] remove blacklist

ae1c9ce

[generate.py] add Exception to return type

f41e9e0

[generate.py] nothing, self doesn't work; use classname in staticmethod

4a1c70b

[generate.py] comments

244d49a

[test_generate.py] rename

f887f04

Merge pull request #41 from divmadan/fix/make_lives_easier-3

3f4ae5e

`generate.py` changes

diivm added the priority: gsoc Reason for prioritization label Aug 29, 2020

[generate.py] fixes encountered during template handling

053c28c

kunaltyagi reviewed Aug 29, 2020

View reviewed changes

[test_generate.py] todo comment

8a78e9f

aPonza reviewed Aug 30, 2020

View reviewed changes

Apply suggestions from code review

276dcda

Co-authored-by: aPonza <[email protected]>

kunaltyagi reviewed Sep 8, 2020

View reviewed changes

larshg reviewed Sep 9, 2020

View reviewed changes

diivm mentioned this pull request Sep 30, 2020

Changes from the development at PCL repo PointCloudLibrary/clang-bind#1

Merged

stale bot added the status: stale label Dec 19, 2020

stale bot removed the status: stale label Jan 12, 2022

		echo 'deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-11 main' \| sudo tee -a /etc/apt/sources.list
		echo 'deb-src http://apt.llvm.org/bionic/ llvm-toolchain-bionic-11 main' \| sudo tee -a /etc/apt/sources.list



		"""
		TODO

Uh oh!

[GSoC 2020] Extending PCL for use with Python: Bindings generation using Pybind11 #4366

Are you sure you want to change the base?

[GSoC 2020] Extending PCL for use with Python: Bindings generation using Pybind11 #4366

Uh oh!

Conversation

diivm commented Aug 29, 2020

Uh oh!

kunaltyagi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kaju-Bubanja commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kunaltyagi commented Sep 14, 2020

Uh oh!

Kaju-Bubanja commented Sep 14, 2020

Uh oh!

stale bot commented Dec 19, 2020

Uh oh!

ArkaprabhaChakraborty commented Jan 12, 2022

Uh oh!

shelper commented Feb 28, 2022

Uh oh!

jiapei-nexera commented Jun 17, 2022

Uh oh!

Uh oh!

Kaju-Bubanja commented Sep 14, 2020 •

edited

Loading