Unable to open SEG-derived SRs

[fedorov]

We still have remaining issues following the merge of #289, unfortunately.

Traceback (most recent call last):
  File "/Applications/Slicer.app/Contents/lib/Slicer-5.9/qt-scripted-modules/DICOMLib/DICOMUtils.py", line 827, in getLoadablesFromFileLists
    loadablesByPlugin[plugin] = plugin.examineForImport(fileLists)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Applications/Slicer.app/Contents/Extensions-33996/QuantitativeReporting/lib/Slicer-5.9/qt-scripted-modules/base/DICOMPluginBase.py", line 53, in examineForImport
    loadablesForFiles = self.examineFiles(files)
                        ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Applications/Slicer.app/Contents/Extensions-33996/QuantitativeReporting/lib/Slicer-5.9/qt-scripted-modules/DICOMTID1500Plugin.py", line 71, in examineFiles
    isDicomTID1500 = self.isDICOMTID1500(dataset)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Applications/Slicer.app/Contents/Extensions-33996/QuantitativeReporting/lib/Slicer-5.9/qt-scripted-modules/DICOMTID1500Plugin.py", line 98, in isDICOMTID1500
    import highdicom as hd
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.12/site-packages/highdicom/__init__.py", line 1, in <module>
    from highdicom import ann
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.12/site-packages/highdicom/ann/__init__.py", line 9, in <module>
    from highdicom.ann.sop import MicroscopyBulkSimpleAnnotations, annread
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.12/site-packages/highdicom/ann/sop.py", line 33, in <module>
    from highdicom.io import _wrapped_dcmread
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.12/site-packages/highdicom/io.py", line 14, in <module>
    from pydicom.encaps import parse_basic_offsets
ImportError: cannot import name 'parse_basic_offsets' from 'pydicom.encaps' (/Applications/Slicer.app/Contents/lib/Python/lib/python3.12/site-packages/pydicom/encaps.py)
DICOM Plugin failed: cannot import name 'parse_basic_offsets' from 'pydicom.encaps' (/Applications/Slicer.app/Contents/lib/Python/lib/python3.12/site-packages/pydicom/encaps.py)

[CPBridge]

This is a dependency management issue. Highdicom requires pydicom>=3.0.1 (parse_basic_offsets was added to pydicom in 3.0.0). But you seem to have pydicom<3.0 installed. Highdicom's pyproject.toml therefore does not seem to be being respected by whatever tool is doing dependency resolution/management

[fedorov]

I think we need to figure out what is that thing that installs pydicom in Slicer right now, whether it has any constraints on the version, and whether pydicom>=3.0.1 would break that thing. @pieper maybe you already know?

[fedorov]

It looks like pydicom 2.4.4 is a core dependency of Slicer: https://github.com/Slicer/Slicer/blob/04a6796e7a3952ae96c764557aa404a027d31c55/SuperBuild/External_python-dicom-requirements.cmake#L44.

@pieper @lassoan @jcfr what do you think about updating it to 3.0.1 in the superbuild? I think the main breaking change in pydicom v3 was the removal of read_file, but I see Slicer is already using dcmread. Of course, this would mean that any extension that relies on read_file would break after the update, but the fix is trival. I think upgrading pydicom in Slicer is the right way forward.

[pieper]

Let's do the 5.10 release with the current version and then update after that. If you want to override the default install in dcmqi there's nothing really stopping you. Might be good to give users an option of upgrading pydicom if they want to read these SRs.

[fedorov]

Sounds good. @deepakri201 let's then check the version and upgrade, same way as you install highdicom!

[CPBridge]

If you want to override the default install in dcmqi there's nothing really stopping you. Might be good to give users an option of upgrading pydicom if they want to read these SRs.

I'd be a bit cautious about this. There were a few breaking changes between pydicom 2.x and 3.0.

I think the main breaking change in pydicom v3 was the removal of read_file, but I see Slicer is already using dcmread

Can't remember specifics but it's more than this

I also recommend not using 3.0.0 but jumping to 3.0.1. 3.0.0 was short lived and had some important bugs that were fixed in 3.0.1 (largely by me...)

[fedorov]

I'd be a bit cautious about this. There were a few breaking changes between pydicom 2.x and 3.0.

I don't know anything more than what I see in https://github.com/pydicom/pydicom/releases!

But then I don't know how we can deliver the functionality to load those planar SR annotations. There are several issues:

1. From what Deepa is telling me, upgrade of pydicom will necessitate Slicer restart. Since upgrade is triggered from DICOM browser, restart will be extremely disruptive, since it will happen in the process of loading a series, and after restart the context will be lost.
2. If pydicom 3.x.x is not stable and it is not recommended to upgrade, we don't know the consequences of upgrading it in Slicer - it is a big problem. The upgrade within this extension will affect all of the components of Slicer outside of QuantitativeReporting, and for an average user, it will be difficult to figure out the cause and downgrade if something goes wrong later.

I am now thinking that we should revert #289 (I guess I can do this by tagging a prior hash and using that for QuantitativeReporting extension definition), to deal with the current regression, and for Slicer 5.10 not add this functionality at all.

@pieper @jcfr @lassoan what is the decision process to upgrade pydicom in the Slicer app - after 5.10?

[CPBridge]

To clarify, 3.0.1 is stable and I would recommend upgrading. Just skip 3.0.0

[deepakri201]

@CPBridge what do you think about downgrading highdicom so that we can use pydicom 2.4.4?