Slicer icon indicating copy to clipboard operation
Slicer copied to clipboard

Support use of WebServer module without DICOM support

Open jcfr opened this issue 8 months ago • 5 comments

Is your feature request related to a problem? Please describe.

When launching a Slicer (or Slicer-based) application built with Slicer_BUILD_DICOM_SUPPORT set to OFF, the application fails to load the WebServer module due to a missing dependency:

Traceback (most recent call last):
  ...
  File "/work/salt/build_59/Slicer-build/lib/SlicerSALT-5.9/qt-scripted-modules/WebServer.py", line 18, in <module>
    from WebServerLib.BaseRequestHandler import BaseRequestHandler, BaseRequestLoggingFunction
  File "/work/salt/build_59/Slicer-build/lib/SlicerSALT-5.9/qt-scripted-modules/WebServerLib/__init__.py", line 2, in <module>
    from .DICOMRequestHandler import DICOMRequestHandler
  File "/work/salt/build_59/Slicer-build/lib/SlicerSALT-5.9/qt-scripted-modules/WebServerLib/DICOMRequestHandler.py", line 2, in <module>
    import pydicom
ModuleNotFoundError: No module named 'pydicom'
loadSourceAsModule - Failed to load file "/work/salt/build_59/Slicer-build/lib/SlicerSALT-5.9/qt-scripted-modules/WebServer.py"  as module "WebServer" !
Fail to instantiate module  "WebServer"
The following modules failed to be instantiated:
   WebServer

This happens because WebServerLib/DICOMRequestHandler.py unconditionally imports pydicom, even when DICOM support is explicitly disabled at build time.

Describe the solution you'd like

Update the DICOMRequestHandler.py to gracefully handle the case when pydicom module is not available.

Update DICOMRequestHandler.py to gracefully handle the absence of the pydicom module. For example, by conditionally importing pydicom only when needed, or using a try/except block to fallback or warn appropriately.

Describe alternatives you've considered

Disable the WebServer module entirely when Slicer_BUILD_DICOM_SUPPORT is OFF

Additional context

  • https://github.com/Kitware/SlicerSALT/pull/339

jcfr avatar Apr 08 '25 18:04 jcfr

The proposed solution is certainly acceptable. However, if the implementation is not more complex then it would be nicer to somehow check Slicer_BUILD_DICOM_SUPPORT flag in Python directly. It would allow us to make decisions directly, instead of relying on indirect indicators and assumptions, such as "if pydicom is not available then it is probably because Slicer_BUILD_DICOM_SUPPORT=OFF".

lassoan avatar Apr 09 '25 15:04 lassoan

+1 for exposing cmake build options and other build status cleanly available at runtime. I think there are probably many situations where we'd want to know things like which flags were used in a superbuild library for example. We could also provide more specific feedback to users, like a tooltip on a disabled checkbox saying "DICOMweb not available because Slicer was compiled with DICOM support turned off"

pieper avatar Apr 11 '25 14:04 pieper

For reference, CPython exposes its build options via the sysconfig^1 module. We could adopt a similar approach to expose Slicer’s full set of CMake build options at runtime, enabling Python-based decisions accordingly.

However, this would introduce additional maintenance overhead, which I'd prefer to avoid—especially for Python-based Slicer extensions.

Instead, I suggest we introduce a higher-level abstraction to check for feature availability. In this specific case, two alternatives are:

  • Treat the DICOM module as an optional dependency and register the DICOMRequestHandler only if the module is available.
  • Move the DICOMRequestHandler registration directly into the DICOM module and register it there.

jcfr avatar Apr 11 '25 15:04 jcfr

We already have this logic: https://github.com/Slicer/Slicer/blob/c0d0b9ee9dc6a65e8ca4f4e6cf7d4d248632f727/Modules/Scripted/WebServer/WebServer.py#L230-L231

Which I believe could be added here in the __init__ too, since DICOM should be loaded before WebServer (or we could enforce that).

https://github.com/Slicer/Slicer/blob/c0d0b9ee9dc6a65e8ca4f4e6cf7d4d248632f727/Modules/Scripted/WebServer/WebServerLib/init.py#L2

We could also add that logic here, with a warning if dicom is not available but enableDICOM is true:

https://github.com/Slicer/Slicer/blob/c0d0b9ee9dc6a65e8ca4f4e6cf7d4d248632f727/Modules/Scripted/WebServer/WebServer.py#L617-L620

pieper avatar Apr 11 '25 15:04 pieper

What do you think about adding all CMake options that we need ito access in Python to https://github.com/Slicer/Slicer/blob/main/Base/Python/slicer/kits.py.in?

lassoan avatar Apr 11 '25 18:04 lassoan