[axel-grc]

Confidence Score: 5/5

The PR appears safe to merge, with the existing non-blocking issue for interpreter paths containing spaces.

Findings

1.  Spaces prevent commands from starting ▶

Summary

Adds a script to build and install RTK's Python files beside an existing ITK package.

• The installer builds RTK's Python wrapping into an ITK package folder.
• The selected Python environment can import RTK and run its apps.

SimonRit dismissed the earlier Python 3.8 concern because ITK no longer supports it; Greptile withdrew that finding.

_{Reviews (7) · Last reviewed commit: "ENH: Add RTK Python package install scri..." · Reviewed by Greptile}

[SimonRit]

Nice, just two small questions below

[axel-grc]

I made some tests, the script does not do a full rebuild each time a cxx files is changed, so ready to be merged I think

[SimonRit]

I'd change the script to compile ITK with CudaCommon and RTK ON instead of compiling RTK separately. What do you think?

[SimonRit]

I'd remove this line, CudaCommon is in ITK as a remote module so it's always findable. CUDA might not be findable but that's beyond the scope of this script.

[SimonRit]

If I understand correctly, you must compile ITK first. Why not provide a script that jointly compiles an editable ITK and RTK?

[SimonRit]

Replace

A convenience script at  `utilities/build_rtk_python.sh`

by

The convenience script [build_rtk_python.sh](utilities/build_rtk_python.sh)

please.

[axel-grc]

I'd change the script to compile ITK with CudaCommon and RTK ON instead of compiling RTK separately. What do you think?

I'm not sure the joint/editable build is the solution. We can't pip install an ITK source tree, it has no pip build system.
So we can use ITKPythonPackages to build the ITK (and RTK) python packages from a prebuilt ITK. And even then, RTK's hand-written Python (rtkargumentparser.py, init_rtk, rtkExtras, rtkConfig, group/app scripts) has to be copied into the itk python package manually, otherwise the rtk*` applications aren't importable.

[SimonRit]

I'd change the script to compile ITK with CudaCommon and RTK ON instead of compiling RTK separately. What do you think?

I'm not sure the joint/editable build is the solution. We can't pip install an ITK source tree, it has no pip build system. So we can use ITKPythonPackages to build the ITK (and RTK) python packages from a prebuilt ITK. And even then, RTK's hand-written Python (rtkargumentparser.py, init_rtk, rtkExtras, rtkConfig, group/app scripts) has to be copied into the itk python package manually, otherwise the rtk*` applications aren't importable.

Ok, I didn't know one couldn't pip install an ITK source tree. So with this script, one needs to

• pip install itk (and itk-cudacommon if using CUDA ?)
• build ITK with Python wrapping
• build RTK according to the script?

It's currently not clear that one needs to pip install itk first, I would clarify this if this is the proposed procedure.

[axel-grc]

• build RTK ac

We don't need to pip install itk, we just need to build and install itk with python wrapping (and optionally CudaCommon) and give the build path to the script

[SimonRit]

• build RTK ac

We don't need to pip install itk, we just need to build and install itk with python wrapping (and optionally CudaCommon) and give the build path to the script

I tried to test this (fresh Python env, built ITK, run the script). Here is the result:

(localcomp312) srit@srit-Precision-5480:~/src/rtk/rtk$ ITK_DIR=/home/srit/src/itk/lin64-PythonWrapping-install RTK_PIP_BUILD_DIR=/home/srit/src/rtk/lin64-pip ./utilities/build_rtk_python.sh 
+ RTK_PIP_BUILD_DIR=/home/srit/src/rtk/lin64-pip
+ BUILD_TYPE=Release
+ RTK_USE_CUDA=OFF
+ '[' -z /home/srit/src/itk/lin64-PythonWrapping-install ']'
++ python -c 'import itk; import os; print(os.path.dirname(itk.__file__))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import itk; import os; print(os.path.dirname(itk.__file__))
    ^^^^^^^^^^
ModuleNotFoundError: No module named 'itk'
+ ITK_PYTHON_DIR=

So pip install itk is required?

[SimonRit]

My bad, the link doesn't work. I guess it only works for the .md files copied for readthedocs. Is there a proper way to link to files in the GitHub repository?

[SimonRit]

• build RTK ac

We don't need to pip install itk, we just need to build and install itk with python wrapping (and optionally CudaCommon) and give the build path to the script

I tried to test this (fresh Python env, built ITK, run the script). Here is the result:

(localcomp312) srit@srit-Precision-5480:~/src/rtk/rtk$ ITK_DIR=/home/srit/src/itk/lin64-PythonWrapping-install RTK_PIP_BUILD_DIR=/home/srit/src/rtk/lin64-pip ./utilities/build_rtk_python.sh 
+ RTK_PIP_BUILD_DIR=/home/srit/src/rtk/lin64-pip
+ BUILD_TYPE=Release
+ RTK_USE_CUDA=OFF
+ '[' -z /home/srit/src/itk/lin64-PythonWrapping-install ']'
++ python -c 'import itk; import os; print(os.path.dirname(itk.__file__))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import itk; import os; print(os.path.dirname(itk.__file__))
    ^^^^^^^^^^
ModuleNotFoundError: No module named 'itk'
+ ITK_PYTHON_DIR=

So pip install itk is required?

So I progressed by adding the ITK bindings to the PYTHONPATH, as I usually do with my local ITK compilation. Then I had to install scikit_build_core. It's now compiling, I'll let you know if it works...

[SimonRit]

Ok, it worked with the additional patch I have pushed:
c8c305b
We need to clarify a bit the doc. Should we consider testing the independent compilation of itk-cuda-common?

[axel-grc]

I confirmed that the editable mod of pip does not work, so there is no reason to use pip install, we can install rtk with cmake and create symlinks in the script like I did.
This way, the bindings can be used across all environments just by adding it to pythonpath or using a .pth

[greptile-apps]

Spaces prevent commands from starting

If the selected Python environment lives in a directory containing spaces, #!$PYTHON_BIN produces an invalid interpreter line on Linux. The generated commands fail before importing the application, even though installation succeeds. Generate a shell launcher that quotes the interpreter path, as Python package installers do.