Radiomics extension CLI fails on mac 4.8.1

[pieper]

Tested Slicer 4.8.1 and it has an old version. Once we are comfortable with the CLI we should update the version of SlicerRadiomics for the stable release.

Currently the mac nightly is not available due to the qt5 transition, so there's no version for mac at the moment.

RadiomicsCLI standard error:

Traceback (most recent call last):
  File "/Applications/Slicer-4.8.1.app/Contents/Extensions-26813/SlicerRadiomics/lib/Slicer-4.8/cli-modules/SlicerRadiomicsCLIScript", line 6, in <module>
    from radiomics.scripts import commandline
  File "/Applications/Slicer-4.8.1.app/Contents/Extensions-26813/SlicerRadiomics/lib/python2.7/site-packages/radiomics/__init__.py", line 4, in <module>
    import collections  # noqa: F401
  File "/Applications/Slicer-4.8.1.app/Contents/lib/Python/lib/python2.7/collections.py", line 20, in <module>
    from _collections import deque, defaultdict
ImportError: No module named _collections

RadiomicsCLI completed with errors

[JoostJM]

Related to #41.

With the exception of the build error on Mac, the extensions seems to be working very nicely. The windows test fails, but this is due to the fact that it can't find the CLI. If you test manually on windows the test passes normally.

[JoostJM]

Some interesting points concerning this issue are raised in this topic on slicer discourse. Apparently due to Apple SIP policy, slicer DYLD_LIBRARY_PATH is not exported, meaning that the SlicerRadiomicsCLI script is unable to find the correct python libraries.

[ihnorton]

A simple, only-slightly-hacky solution would be to set a var like __MY_DYLD_LIBRARY_PATH to contain the same paths as DYLD_LIBRARY_PATH in the calling environment. Then check for that in the wrapper script, and if it exists, export the proper var in the subshell.

[JoostJM]

@ihnorton Thanks for tracking this down! We'll look into fixing this, hopefully implementing some not-really hacking solution soon, but going for a slightly more hacky solution if needed.

[jcfr]

Few approaches:

(1) You could improve the launcher so that it also set variables like APPLAUNCHER_DYLD_LIBRARY_PATH, APPLAUNCHER_LD_LIBRARY_PATH and APPLAUNCHER_PATH. Once the fix implemented, we could easily release new launcher binaries by tagging a new version.

(2) Otherwise, an easier approach could be to simply use the AppLauncher CMake function in the pyradiomics CMakeLists.txt to configure a launcher per CLI. For example, see here

Let me know if you have questions

[ihnorton]

Note that the SIP restriction only applies to system-provided binaries (/bin/sh, /usr/bin/python, etc.). That's why the executable C++ based CLIs work fine. So, clean and simple solution would be to make RadiomicsCLI a "real" CLI in C++, but one which just does the same thing as the current script (find python-real, launch it with pass-through arguments).

Unfortunately process control is hard to do correctly in a cross-platform way in C++ (you can't use std::system because it just shells-out, so the same restriction applies). If you go that route, I would suggest to use kwsys because I think you already have the ITK dependency (which -> SystemTools::FindProgram, and you can look at the process creation steps in e.g. Slicer CLI execution code).

[ihnorton]

I had another idea for a potential solution which could avoid adding any environment variables, and avoid additional layers of cmake/launchers (plus would allow removing the .bat and .sh wrapper scripts).

https://github.com/Slicer/Slicer/pull/894