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

Add python wrapper and usage example with Jupyter notebooks #70

Open
grzanka opened this issue Apr 15, 2024 · 6 comments
Open

Add python wrapper and usage example with Jupyter notebooks #70

grzanka opened this issue Apr 15, 2024 · 6 comments

Comments

@grzanka
Copy link
Contributor

grzanka commented Apr 15, 2024

Use PyBind11 or some wrapper around it. pyproject.toml could be used to package the project. Uploading the package to pypi would be nice

@grzanka
Copy link
Contributor Author

grzanka commented Apr 16, 2024

@michal367 I was thinking if our build system allows easily to make a python wrapper.
On one hand we don't have that many header + source files, so probably we can leave compilation of the library to some python tools, instead of relying on our CMake.

On the other hand it seems that GDCM is obligatory requirement which would make it a pretty heavy target for Python packaging. Can we build a yagit library without DICOM support with current CMake toolset ?

@michal367
Copy link
Collaborator

I want to avoid a situation where the C++ library is buried under the Python library and is inaccessible on its own. Ideally, there should be two separate libraries, with the C++ library completely independent from the Python one. Also, because of this, in the case of the C++ library alone, CMake shouldn't be replaced with a Python tool. In the case of the Python library, CMake can be executed within setup.py.

For example, GDCM uses SWIG for Python (and other languages) binding. It builds this binding only if the CMake option GDCM_WRAP_PYTHON is set to ON. This is done in the main repository, but the Python package itself is in another unofficial repository python-gdcm.

OpenCV does this in a similar way - it uses its custom Python binding in the same repo, but Python package is in another repo opencv-python.

So, in summary, Python binding will be probably in this repository, but Python packaging needs to be in another one for less mess. The second repository can be named yagit-python, python-yagit, pyyagit, pyagit, or something else.

As a binding tool, I suggest using nanobind. Pybind11 and nanobind are very similar and are both developed by the same author. Nanobind offers fewer features, but it is faster.

When it comes to the GDCM dependency in YAGIT, I plan to add a CMake option WITH_GDCM to enable or disable this dependency. Naturally, functions reliant on this dependency, such as DataReader::readRTDoseDicom, will not work if it's disabled.

@grzanka
Copy link
Contributor Author

grzanka commented Apr 21, 2024

I want to avoid a situation where the C++ library is buried under the Python library and is inaccessible on its own. Ideally, there should be two separate libraries, with the C++ library completely independent from the Python one. Also, because of this, in the case of the C++ library alone, CMake shouldn't be replaced with a Python tool. In the case of the Python library, CMake can be executed within setup.py.

For example, GDCM uses SWIG for Python (and other languages) binding. It builds this binding only if the CMake option GDCM_WRAP_PYTHON is set to ON. This is done in the main repository, but the Python package itself is in another unofficial repository python-gdcm.

OpenCV does this in a similar way - it uses its custom Python binding in the same repo, but Python package is in another repo opencv-python.

So, in summary, Python binding will be probably in this repository, but Python packaging needs to be in another one for less mess. The second repository can be named yagit-python, python-yagit, pyyagit, pyagit, or something else.

I think its a good idea to separate python packaging to another repository and simply link to this one (which contains C++ code) using submodules. pyyagit looks like a good name.

The only thing which I am puzzled about is where to put all the python binding code. @michal367 what do you think of having a second CMake in the pyyagit repository which takes the C++ code from submodule and builds a python binding libraries ? This way we wouldn't "poison" base C++ repo with any python code.

As a binding tool, I suggest using nanobind. Pybind11 and nanobind are very similar and are both developed by the same author. Nanobind offers fewer features, but it is faster.

nanobind seems a good choice. According to https://nanobind.readthedocs.io/en/latest/packaging.html there may be chance to get rid of old-school setup.py and rely on the new-style pyproject.toml.
Its worth to follow https://github.com/wjakob projects :)

When it comes to the GDCM dependency in YAGIT, I plan to add a CMake option WITH_GDCM to enable or disable this dependency. Naturally, functions reliant on this dependency, such as DataReader::readRTDoseDicom, will not work if it's disabled.

That seems like prelimiary task before trying to work on python binding. Python example may simply use pydicom to read and write dicom files and handle data as numpy arrays.

Anyway python code would be a major advantage for the end users. Many more of them would like to prefer simple python notebook than starting to adopt the code from an example written in C++ (https://github.com/DataMedSci/yagit/tree/master/examples)

@michal367
Copy link
Collaborator

I think its a good idea to separate python packaging to another repository and simply link to this one (which contains C++ code) using submodules. pyyagit looks like a good name.

Instead of using git submodules, I suggest using the CMake module FetchContent because git submodules are very problematic.

The only thing which I am puzzled about is where to put all the python binding code. @michal367 what do you think of having a second CMake in the pyyagit repository which takes the C++ code from submodule and builds a python binding libraries ? This way we wouldn't "poison" base C++ repo with any python code.

That's fine with me.

@grzanka
Copy link
Contributor Author

grzanka commented Apr 23, 2024

I think its a good idea to separate python packaging to another repository and simply link to this one (which contains C++ code) using submodules. pyyagit looks like a good name.

Instead of using git submodules, I suggest using the CMake module FetchContent because git submodules are very problematic.

Will that FetchContent work with C++ source and header files stored in some other repository ?

@michal367
Copy link
Collaborator

Will that FetchContent work with C++ source and header files stored in some other repository ?

Yes. It can be used to download and build an external CMake project, but also to only download the source code.

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

No branches or pull requests

2 participants