-
Notifications
You must be signed in to change notification settings - Fork 50
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
Radiomics extension CLI fails on mac 4.8.1 #43
Comments
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. |
Some interesting points concerning this issue are raised in this topic on slicer discourse. Apparently due to Apple SIP policy, slicer |
A simple, only-slightly-hacky solution would be to set a var like |
@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. |
Few approaches: (1) You could improve the launcher so that it also set variables like (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 |
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 Unfortunately process control is hard to do correctly in a cross-platform way in C++ (you can't use |
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). |
Remove use .bat and .sh wrapper script, and instead define the CLI entry point as a python script. This makes use of the new Slicer functionality described in Slicer/Slicer#894 and implemented in r27143-r27145. This fixes #43. Additionally, check if a `--label <label>` argument is present, and if so, update it to the new-style: `--setting=label:<label>`. Parameter `--label` is deprecated by PR AIM-Harvard/pyradiomics#347.
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.
The text was updated successfully, but these errors were encountered: