Skip to content

ENH: Add regression test for reading legacy multi-frame DICOM#315

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:masterfrom
thewtex:dicom-multiframe-legacy-test
Dec 18, 2018
Merged

ENH: Add regression test for reading legacy multi-frame DICOM#315
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:masterfrom
thewtex:dicom-multiframe-legacy-test

Conversation

@thewtex
Copy link
Copy Markdown
Member

@thewtex thewtex commented Dec 16, 2018

Ensure that origin and spacing and identified correctly (the image
comparison checks this metadata).

Test image from David Clunie.

See also the discussion:

https://discourse.slicer.org/t/dicom-multiframe-support/4806/9

@fedorov
Copy link
Copy Markdown
Member

fedorov commented Dec 16, 2018

@thewtex can you remind me how I can access the image files used in testing?

@thewtex
Copy link
Copy Markdown
Member Author

thewtex commented Dec 16, 2018

@fedorov this particular file is here:

https://data.kitware.com/#item/5c16a3388d777f072bd41f38

In general, you can check out the branch, and run make ITKData in the build tree to fetch the files.

Additional information can be found in ITK/Documentation/Data.md.

)

itk_add_test(NAME itkGDCMLegacyMultiFrameTest
COMMAND ITKIOGDCMTestDriver
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be quite helpful to make this more accessible to broader group of users if direct download pointers for the datasets could be added in the comments here. It takes quite a lot of time and space to checkout/configure/download ITK source and data. I also think there should be a link to the original MR series that was converted into the legacy enhanced dataset, since the ultimate test is whether the geometry (and slice order) of the image volume is the same while loading from the original non-enhanced series and the legacy enhanced one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the link to the original non-enhanced MR series: https://www.dropbox.com/s/8m7ugu4cmw83fvd/dicoms-anon.zip?dl=0

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be quite helpful to make this more accessible to broader group of users if direct download pointers for the datasets could be added in the comments here. It takes quite a lot of time and space to checkout/configure/download ITK source and data.

Yes -- after merged and the nightly data upload occurs, the data will be available here.

I also think there should be a link to the original MR series that was converted into the legacy enhanced dataset, since the ultimate test is whether the geometry (and slice order) of the image volume is the same while loading from the original non-enhanced series and the legacy enhanced one.

Here's the link to the original non-enhanced MR series: https://www.dropbox.com/s/8m7ugu4cmw83fvd/dicoms-anon.zip?dl=0

Thanks for the link, I will add a reference to this in a comment.

@fedorov
Copy link
Copy Markdown
Member

fedorov commented Dec 16, 2018

Compared with loading the original non-enhanced dataset (linked above) the order of slices is reversed (reconstruction is the same, but orientation is LR vs RL). Is this the expected behavior?

Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good.

@thewtex
Copy link
Copy Markdown
Member Author

thewtex commented Dec 17, 2018

Compared with loading the original non-enhanced dataset (linked above) the order of slices is reversed (reconstruction is the same, but orientation is LR vs RL). Is this the expected behavior?

This sounds like an issue in how the non-enhanced dataset was converted to the multiframe file? We are only testing how ITK is interpreting the resulting multiframe file here.

Ensure that origin and spacing and identified correctly (the image
comparison checks this metadata).

Test image from David Clunie.

See also the discussion:

  https://discourse.slicer.org/t/dicom-multiframe-support/4806/9
@thewtex thewtex force-pushed the dicom-multiframe-legacy-test branch from f05fcff to 3638fe7 Compare December 17, 2018 21:32
@hjmjohnson hjmjohnson merged commit 12148e9 into InsightSoftwareConsortium:master Dec 18, 2018
#include "itkImageFileReader.h"
#include "itkImageFileWriter.h"

// This test verifies that we obtain the correct origin and spacing for a
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I understand the actual goal of this test. I do not see a comparison being done against the original (legacy) MR Image Storage Series. I guess someine validated it both are the same, but it would be good to mention that in the test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is checking the mf.dcm's origin and spacing are read and interpreted correctly -- they previously were not. The conversion of the original (legacy) MR Image Storage Series is not addressed here. The discussion is provided for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants