-
Notifications
You must be signed in to change notification settings - Fork 62
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
Python wrapping #377
Comments
One option would be to provide an itk remote module. I have been looking into this for other code, but it might also be good for dcmqi. |
Probably the ease of installation
Since the dcmqi CLIs are standalone executables. A first step could be to simply provide the CLIs within a python package like we do for cmake and ninja.
Doing so would allow to leverage the available ITK build tree when doing CI. Regarding the API, unless that code is refactored to provide ITK interfaces/classes, I am not sure it makes yet. |
Interesting, so it's those two: But there are already binaries for cmake, for example, for all platforms - what is the real advantage of I didn't even realize you have those. Can you tell, how big would be the effort to make something like this for dcmqi, given its organization?
I agree, I had similar concerns. |
@pieper says this could be the cleanest approach: https://cppyy.readthedocs.io/. Related pointers:
|
This was an important step to streamline the integration with the python ecosystem in the context of python binary packages. These are building blocks for scikit-build Downloading and extracting an archive was tedious for both python developer and CI maintenainers. |
Thinking about this more, I do think the approach used by cmake and ninja makes the most sense for dcmqi. I don't think we would need to expose the full C++ API for most python use cases. |
We would want a helper script though to expose a pythonic api. |
@jcfr thanks for the clarification. What would be the best place to start to do something like you did for cmake for dcmqi? |
After speaking with @piiq at the Project Week I will try to take a look at SWIG (or some of the other tools he suggested) to bind the CPP DCQMI implementation to python - and then possibly package everything. I'll report on the progress as soon as I have something. I have never tried this, but it could be fun. Worst case we could start from a dumb python wrapper using *edit: just saw @piiq already forked DCMQI and is probably working on this. I'd be glad to tag along, so let's see how this goes. |
There are several mature packages for wrapping C++ code for use in python and it would be good to survey them for use with dcmqi. In my experience cppyy is worth exploring as a modern take on the problem. Here is a colab notebook with some examples. |
Yes, I was also mentioning cppyy to @denbonte |
Overall, I am worried that python wrapping will become a rabbit hole, and also I do not know how much expertise would be needed to deal with compilers and CI of those wrapping tools on various platforms. But if someone wants to contribute this functionality - I am definitely very much for it (I would definitely want to fix mac OS CI first though to at least make sure it does not cause issues on mac).
This is where I would start as well! |
We decided to proceed with the following plan towards simplified access to dcmqi from python, which @LennyN95 will be working on:
|
It would go into this repo: https://github.com/ImagingDataCommons/dcmqi-python-distributions |
Potentially, it might be interesting to have a python package that wraps the converter interfaces. It is difficult to assess the effort and benefits of such package, since (due to my limited understanding of how things work):
For the reference, since I spent a little bit of time on this, here are some related pointers:
If anyone has an opinion on this topic, or is interested to contribute this functionality, please add comments to this issue!
The text was updated successfully, but these errors were encountered: