Skip to content
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

Expected behavior for studies that contain a single SR should be refined #2348

Closed
fedorov opened this issue Apr 1, 2021 · 12 comments
Closed
Assignees
Labels
IDC:priority Items that the Imaging Data Commons wants to help sponsor

Comments

@fedorov
Copy link
Member

fedorov commented Apr 1, 2021

This study contains a single SR series, but the SR series does not show up in the list on the left.

https://idc-sandbox-000.firebaseapp.com/projects/idc-dev-etl/locations/us-central1/datasets/idc/dicomStores/v2/study/1.3.6.1.4.1.5962.1.1.0.0.0.1563690660.7173.1309

image

@fedorov fedorov added Community: Report 🐛 IDC:priority Items that the Imaging Data Commons wants to help sponsor labels Apr 1, 2021
@igoroctaviano
Copy link
Contributor

igoroctaviano commented Apr 8, 2021

@fedorov Currently, the sorting series function had only two supported SR sop classes ((Enhanced SR Storage and Grayscale Softcopy Presentation State Storage) in its array:

@fedorov
Copy link
Member Author

fedorov commented Apr 8, 2021

Currently, the sorting series function had only two supported SR sop classes ((Enhanced SR Storage and Grayscale Softcopy Presentation State Storage) in its array

I think that should be revised. TID 1500 instances can be saved as Comprehensive SR storage, so that is not a good assumption to have.

This package can currently only interpret DICOM SR TID 1500.

I would also think that display of the SR content as text should be supported for any SR, independent of the template. Do you agree? I think the current constraints are too limiting.

@igoroctaviano
Copy link
Contributor

igoroctaviano commented Apr 13, 2021

Currently, the sorting series function had only two supported SR sop classes ((Enhanced SR Storage and Grayscale Softcopy Presentation State Storage) in its array

I think that should be revised. TID 1500 instances can be saved as Comprehensive SR storage, so that is not a good assumption to have.

This package can currently only interpret DICOM SR TID 1500.

I would also think that display of the SR content as text should be supported for any SR, independent of the template. Do you agree? I think the current constraints are too limiting.

@fedorov Just to clarify, this dataset contains ContentTemplateSequence.TemplateIdentifier === "8101" and the dcmjs cornerstone adapter can currently only interpret DICOM SR TID 1500. We should update dcmjs cornerstone adapter to interpret 8101?

@fedorov
Copy link
Member Author

fedorov commented Apr 14, 2021

I think the dcmjs adapter should be updated to read the SR content tree, independent of the template encoded, to produce SR format similar to what DCMTK dsr2html does, as illustrated here: https://peerj.com/articles/2057/#fig-7, or by the Pixelmed tools, as illustrated below (image credit: https://github.com/dcmjs-org/data/releases/download/DICOMSR_AIM_Test/DICOMSR_Test.zip). Identification of tree patterns specific to a given template is a higher order operation, which should not be needed for basic rendering of the SR content in a human-readable form.

image

@pieper
Copy link
Member

pieper commented Apr 14, 2021

Agreed, yes, a nicely formatted SR would be a good addition.

@igoroctaviano
Copy link
Contributor

PR Updated. #2352
@Punzo can you take a look/test?

@igoroctaviano
Copy link
Contributor

https://peerj.com/articles/2057/#fig-7

@fedorov I have updated the PR to allow the rendering of this SOP in the formatted HTML/SR extension. It should render this SR as an HTML page in the viewport like we currently see in OHIF for other SRs. But now I'm wondering if you want to improve that extension to render differently as well? I mean, with collapsible nodes like in your example?

@fedorov
Copy link
Member Author

fedorov commented Apr 20, 2021

Yes, it would be nice, but not urgent. I think implementation of a more user-friendly rendering should be done both for the DICOM Tag view (to support collapsible sequences) and SR view in a similar way. Let's add a separate issue for improved rendering?

@igoroctaviano
Copy link
Contributor

Yes, it would be nice, but not urgent. I think implementation of a more user-friendly rendering should be done both for the DICOM Tag view (to support collapsible sequences) and SR view in a similar way. Let's add a separate issue for improved rendering?

That makes sense, issue created: #2372.

@fedorov
Copy link
Member Author

fedorov commented May 20, 2021

When I open the URL in the initial post of the issue, I get the following at the bottom of the SR view - this must be wrong, no?

image

@igoroctaviano
Copy link
Contributor

When I open the URL in the initial post of the issue, I get the following at the bottom of the SR view - this must be wrong, no?

image

Maybe there's something wrong in dcmjs or in HTML extension, this task didn't make any changes to the parsing logic, it only included more sop classes. I'll investigate and let you know.

@igoroctaviano
Copy link
Contributor

@fedorov fixed here: #2419

These tags are dicom meta info which was not naturalized before they were sent to the parsing function in dicom-html extension. Now I have added a new section to better display this info.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDC:priority Items that the Imaging Data Commons wants to help sponsor
Projects
None yet
Development

No branches or pull requests

4 participants