Add SlicerRadiomicsCLI

[JoostJM]

Add a SlicerExecutionModel CLI for PyRadiomics. This will run the PyRadiomics extraction in a separate process and return the results. This will allow to perform large extractions without freezing the main UI thread and also allow for progress reporting.

TODO:

• [x] Integrate with SlicerRadiomics module
• [ ] Add progress reporting
• [ ] Intercept logging messages?
• [ ] Make CLI module hidden for users?

[pieper]

LGTM

[pieper]

I didn't try a build - I'll see if it works as is on mac or linux.

[fedorov]

Great progress! (and thanks for adding the license!)

I will review later this week.

[pieper]

As Jc suspected it doesn't build because the macro is looking for a C++ program to build and install.

I guess that instead of a bat file we could write a small C++ executable that proxies to the python code. That seems just wrong. Still, it would work and would bypass the need to refactor any cmake macros.

[pieper]

I think this alternative will work on all platforms:

https://github.com/Radiomics/SlicerRadiomics/compare/add-cli-in-scripted?expand=1

Instead of making a new build of a CLI it puts the executables in with the scripted module. Tested on mac and it looks good.

[jcfr]

((since you have push access to SlicerRadiomics repo, you should be able to push onto the branch of @JoostJM directly))

That will indeed work and allow the module to be discovered given the current implementation.

👍

While this doesn't prevent the integration, few things to be aware:

• Associating the CLIs as a scripted resources (MODULE_PYTHON_RESOURCES) could easily break if management of python script resources or scripts is updated.

• It would also break the contract by:

  • misleading the Slicer users (the scripted directory wouldn't contain only scripted modules)
  • and (given the current implementation) Slicer options like these ones would "fail" ...

  --disable-cli-modules                         Disable the loading of any Command Line Modules.
  --disable-builtin-cli-modules                 Disable the loading of builtin Command Line Modules.
  [...]
  --disable-scripted-loadable-modules           Disable the loading of any Scripted Loadable Modules.
  --disable-builtin-scripted-loadable-modules   Disable the loading of builtinScripted Loadable Modules.

[pieper]

@jcfr Agreed, I wondered if we might come up with a better solution, so I made the other branch as an alternative in case we came up with something else to build on this one. (But I suspect we won't come up with anything easier).

Regarding the contract, yes the directory names become a bit problematic (a 'cli' in the 'scripted' directory). But we already have some mixing of concepts because adding module paths on both the Slicer command line and the preferences don't make a distinction of module type.

Also I don't see that we have a problem with the flags you mentioned - if I disable CLI module and use verbose module discovery I see that qSlicerCLIExecutableModuleFactory and qSlicerCLILoadableModuleFactory aren't being tested, so the flags are independent of the module directory names.

[jcfr]

If doing the refactoring of the macro is too much work, the proposed solution is reasonable 👍

But we already have some mixing of concepts because adding module paths on both the Slicer command line and the preferences don't make a distinction of module type.

I would say this is a convenience for the experimenting locally where the convention is implicitly known.

I don't see that we have a problem with the flags you mentioned ... so the flags are independent of the module directory names.

Great. No problem then 😄

[pieper]

Fair enough - I took a look at the SEMMacroBuildCLI macro but it looked like a big job to generalize it (with potential to break things...).

[jcfr]

I took a look at the SEMMacroBuildCLI macro

Great

A simpler approach could be to simply add code like this one into the CLI CMakeLists.txt:

• custom target:

add_custom_target(Copy${PROJECT_NAME}ToBuildDirectory ALL
  COMMAND ${CMAKE_COMMAND} -E copy_if_different
       SlicerRadiomicsCLI
       ${SlicerExecutionModel_DEFAULT_CLI_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/SlicerRadiomicsCLI
  COMMAND ${CMAKE_COMMAND} -E copy_if_different
       SlicerRadiomicsCLI.bat
       ${SlicerExecutionModel_DEFAULT_CLI_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/SlicerRadiomicsCLI.bat
  COMMAND ${CMAKE_COMMAND} -E copy_if_different
       SlicerRadiomicsCLI.xml
       ${SlicerExecutionModel_DEFAULT_CLI_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/SlicerRadiomicsCLI.xml
  COMMENT "Copying ${PROJECT_NAME} files into build directory"
  )

• install rules:

    # Install the XML file
    install(FILES
       SlicerRadiomicsCLI
       SlicerRadiomicsCLI.bat
       SlicerRadiomicsCLI.xml
      DESTINATION ${SlicerExecutionModel_DEFAULT_CLI_INSTALL_RUNTIME_DESTINATION}
      COMPONENT RuntimeLibraries
      )

Hth Jc

[jcfr]

Will push an update shortly

[jcfr]

PTAL

[pieper]

Nice work @jcfr! That looks like it will do the trick.

[jcfr]

@JoostJM Good catch. Unless you want to address the remaining todos now, it should be good for final review and integration.

[JoostJM]

@jcfr Thanks, I already thought the code could be simplified, but I'm not proficient enough with CMAKE (yet).

LGTM