highdicom
highdicom copied to clipboard
ImagingDataCommons
SR templates do not support Image Library
Highdicom doesn't have support for TID 1604 (Image Library). This would be useful for encoding imaging attributes (imaging orientation, slice thickness, pixel spacing) for clients interpreting Comprehensive 2D SRs before the source image SOPInstances were retrieved.
@seandoyle that's already underway with #69 (see highdicom.sr.ImageLibraryEntry).
Note that I did not implement the modality-specific templates, but instead enable the caller to add the content items via the descriptors parameter. I would appreciate if you could test the implementaiton and let me know what you think.
One thing I found confusing about the API is the structure of the array of ImageLibraryEntry objects. It's an array of an array of ImageLibraryEntry. Initially I had specified an array of the following sort:
[[ImageLibraryEntry, ImageLibraryEntry, ...]] for 91 entries. This worked fine in the code - but the pixelmed validator generated the following error:
Error: Template 1602 ImageLibraryEntryDescriptors/[Row 1] CODE (121139,DCM,"Modality"): within 1.7.1: /CONTAINER (126000,DCM,"Imaging Measurement Report")/CONTAINER (111028,DCM,"Image Library")/CONTAINER (126200,DCM,"Image Library Group"): Incorrect content item value multiplicity - found 91 content items but expected 1
But - if I generated an array of the following sort:
[[ImageLibraryEntry], [ImageLibraryEntry], ... ]
then it worked just fine.
My issues are
- ImageLibrary template accepted both formats but only one was accepted by the validator.
- From looking at the DICOM spec I couldn't figure out which should be correct.
If the second approach is correct I should add some code to throw an error if it encounters input from the first approach.
If the second approach is correct I should add some code to throw an error if it encounters input from the first approach.
I think I have implemented the template incorrectly. TID 1600 Image Library contains 1-n "Image Library Group" content items of value type CONTAINER, each containing 1-n content items of value type IMAGE (TID 1601 Image Library Entry) or multiple content items of different value types (TID 1602 Image Library Entry Descriptors).
The ImageLibraryEntry class actually implements TID 1602 rather than TID 1601, which is the source of the problem. I have fixed this and added further tests via c7227224a7bb9cac911940eb9900ae322f0b62d5. Could you kindly try again?
@seandoyle we could now implement subclasses of ImageLibraryEntryDescriptors for specific imaging modalities (TID 160{3-7}).
I am not sure whether we should implement TID 1602 Image Library Entry or whether we should omit this for now and only support descriptions of groups of images. The row is optional so I don't see any problems from a standard perspective. @seandoyle @CPBridge @fedorov @dclunie @pieper what are your thoughts on this?
I thought I understood it in the past, but I don't anymore, as I was looking at the TIDs. It appears that it would be valid to have Image library container that has descriptors only, and no references to the actual images, which I am not sure is particularly useful.
To answer your question Markus - yes, it appears to be valid to only implement TID 1602 Image Library Entry Descriptors, but I am afraid that would not be very useful (in the previous post, did you mean to TID 1602, or "TID 1601 Image Library Entry"?)
Here's how dcmtk/dcmqi encode Image library:
@hackermd - Yes - this works in the sense that it passes the pixelmed validator. Thanks!
@fedorov - I agree that it looks like TID 1601 isn't required and that it doesn't make sense to exclude it. This is the only way to get the SOPInstances matched up with the descriptors from TID 1602.
Ok. Thanks @fedorov and @seandoyle. If I understand it correctly, the group-level descriptors are intended to provide attributes that are common to the collection of image instances and TID 1601 Image Library Entry would provide the image-level descriptors that are unique to individual image instances.
@seandoyle do you want to take a shot at implementing ImageLibraryEntry?
The constructor of ImageLibrary could then accept Sequence[Tuple[ImageLibraryEntryDescriptors, Sequence[ImageLibraryEntry]]], where each group item may contain 1 ImageLibraryEntryDescriptors instance (for the collection or group of images) and 1-n ImageLibraryEntry instances (for each image). However, that type annotation looks scary. Therefore, I would suggest to abstract all of that and just pass Sequence[pydicom.dataset.Dataset] to the ImageLibrary constructor and construct the ImageLibraryEntry and ImageLibraryEntryDescriptors instances under the hood.
What do you think?
If I understand it correctly, the group-level descriptors are intended to provide attributes that are common to the collection of image instances and TID 1601 Image Library Entry would provide the image-level descriptors that are unique to individual image instances.
Correct, but there is another major difference between 1601 and 1602: the former requires IMAGE to be present, the latter does not include IMAGE even as optional. With 1602 you describe the descriptors, but are allowed to omit the images those correspond to.
@hackermd Sure - I'll give it a try tonight.
I'm finding this very odd - the reason I want to use the image library descriptors is so I can store specific image attributes (slice thickness &etc) in the SR. The axial and sagittal views have different attributes but share the same FrameOfReferenceUID. Without sending the SOPInstanceUIDs it's going to have limited usefulness.
The constructor of
ImageLibrarycould then acceptSequence[Tuple[ImageLibraryEntryDescriptors, Sequence[ImageLibraryEntry]]], where each group item may contain 1ImageLibraryEntryDescriptorsinstance (for the collection or group of images) and 1-nImageLibraryEntryinstances (for each image). However, that type annotation looks scary. Therefore, I would suggest to abstract all of that and just passSequence[pydicom.dataset.Dataset]to theImageLibraryconstructor and construct theImageLibraryEntryandImageLibraryEntryDescriptorsinstances under the hood.
This would certainly simplify the code that is using highdicom. To avoid a lot of conditional logic (and standardize the SR creation) should we assume that all of the optional entries in 1602, 1603, 1604 are included? Should I include 1605-1607 for this issue? I'd like to leave them for the next iteration and see how this looks first.
This would certainly simplify the code that is using highdicom. To avoid a lot of conditional logic (and standardize the SR creation) should we assume that all of the optional entries in 1602, 1603, 1604 are included? Should I include 1605-1607 for this issue?
Yeah, I thought we could either include all optional content items or an opinionated subject for each of the provided images.