WIP: ENH: Increase `itk::DCMTKFileReader` class coverage

[jhlegarreta]

Add a new test to the IODCMTK module to increase the coverage of the itk::DCMTKFileReader::GetElement* methods.

PR Checklist

• [X] No API changes were made (or the changes have been approved)
• [X] No major design changes were made (or the changes have been approved)
• [X] Added test (or behavior not changed)
• [X] Added license to new files (if any)

[jhlegarreta]

A few notes:

• I know the module gets less use than GDCM, but if it is in the toolkit, it should get some more testing.
• I could not compile the DCTMK test module:

Build started...
1>------ Build started: Project: ITKIODCMTKTestDriver, Configuration: Debug x64 ------
1>itkDCMTKGetDicomTagsTest.cxx
1>C:\SDKs\ITK\itk-head\ITK\Modules\IO\DCMTK\include\itkDCMTKFileReader.h(27,10): fatal error C1083:
Cannot open include file: 'dcmtk/dcmdata/dcdict.h': No such file or directory
1>Done building project "ITKIODCMTKTestDriver.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Using the built-in DCMTK (i.e. not a system DCMTK) I cannot see where the error stems from. I do see the file on my build tree in \itk-head-Bin\Modules\ThirdParty\DCMTK\ITKDCMTK_ExtProject\dcmdata\include\dcmtk\dcmdata\dcdict.h

I submitted an experimental build to see if that helps other folks to try to reproduce the issue: https://open.cdash.org/build/7503588

Thus, I could not compile and run the test to see its errors.

Not sure if the CIs have the module flag set to On so they may come up green.

• Not sure if the test name is the most appropriate one. Suggestions are welcome.
• I did not found non-empty tags in the used DICOM data to exercise other methods like, GetElementCSorOB, etc. Adding another test that uses another set of DICOM data could allow querying such elements/exercising those methods. Best would be to make the test quantitative for that.
• @thewtex In order to make the test quantitative, I'm wondering whether we have CMake-enabled ways to compare a potential txt output file with the read tags to a baseline with the expected tags. If not, do you have some suggestion?

[jhlegarreta]

Making the test quantitative would be good, but that can be a follow-up commit. This is good as-is.

Sounds reasonable. Note that I have not been able to compile the module, and hence nor I have tested it, and I would not merge this until it gets tested.

The CI builds did not set the module to ON, and thus, even if they have come green, it might well be the case that the test does not compile/pass:

2021-10-10T01:29:38.0772539Z 595: //Request building ITKIODCMTK (non-remote)
2021-10-10T01:29:38.0773102Z 595: Module_ITKIODCMTK:BOOL=OFF

[dzenanz]

I will test it locally and report back.

[dzenanz]

It fails to compile for me too:

7>itkDCMTKGetDicomTagsTest.cxx
7>C:\Dev\ITK-git\Modules\IO\DCMTK\include\itkDCMTKFileReader.h(27,10): fatal error C1083: Cannot open include file: 'dcmtk/dcmdata/dcdict.h': No such file or directory

[dzenanz]

The offending file is located in C:\Dev\ITK-2019\Modules\ThirdParty\DCMTK\ITKDCMTK_ExtProject\dcmdata\include\dcmtk\dcmdata\dcdict.h, and the only mention of DCMTK in additional include directories is C:\Dev\ITK-git\Modules\IO\DCMTK\include.

[jhlegarreta]

and the only mention of DCMTK in additional include directories is C:\Dev\ITK-git\Modules\IO\DCMTK\include

Meaning that including the directory is missing in the CMakeLists.txt file?

I did try to add it manually to my MSVC properties, but it did not solve the problem. But maybe I did not do it appropriately.

Also, Brad's AWS build tests the module, so it would be weird that this should not happen on Linux: https://open.cdash.org/build/7505424/notes

[dzenanz]

ITKDCMTK_INCLUDE_DIRS seems to have appropriate dirs, being C:/Dev/ITK-2019/Modules/ThirdParty/DCMTK/ITKDCMTK_ExtProject/dcmdata/include;.... Manually adding the first entry to Visual Studio project ITKIODCMTKTestDriver, changes the error to:

5>------ Build started: Project: ITKIODCMTKTestDriver, Configuration: RelWithDebInfo x64 ------
5>itkDCMTKGetDicomTagsTest.cxx
5>C:\Dev\ITK-2019\Modules\ThirdParty\DCMTK\ITKDCMTK_ExtProject\dcmdata\include\dcmtk/dcmdata/dcdict.h(26,10): fatal error C1083: Cannot open include file: 'dcmtk/config/osconfig.h': No such file or directory
5>itkDCMTKImageIOTest.cxx
5>itkDCMTKImageIONoPreambleTest.cxx
5>Generating Code...
5>Done building project "ITKIODCMTKTestDriver.vcxproj" -- FAILED.

Adding the entire content of ITKDCMTK_INCLUDE_DIRS to VS project gets it "building":

2>------ Build started: Project: ITKIODCMTKTestDriver, Configuration: RelWithDebInfo x64 ------
2>ITKIODCMTKTestDriver.cxx
2>itkDCMTKGetDicomTagsTest.cxx
2>C:\Dev\ITK-2019\Modules\ThirdParty\DCMTK\ITKDCMTK_ExtProject-build\config\include\dcmtk/config/arith.h(1,1): warning C4335: Mac file format detected: please convert the source file to either DOS or UNIX format
2>C:\Dev\ITK-git\Modules\IO\DCMTK\include\itkDCMTKFileReader.h(307,1): error C2440: 'static_cast': cannot convert from 'T' to 'TType'
2>        with
2>        [
2>            T=Float64
2>        ]
2>        and
2>        [
2>            TType=std::string
2>        ]
2>C:\Dev\ITK-git\Modules\IO\DCMTK\include\itkDCMTKFileReader.h(307,1): message : No constructor could take the source type, or constructor overload resolution was ambiguous
2>C:\Dev\ITK-git\Modules\IO\DCMTK\test\itkDCMTKGetDicomTagsTest.cxx(92): message : see reference to function template instantiation 'int itk::DCMTKFileReader::GetElementDS<std::string>(const unsigned short,const unsigned short,unsigned short,TType *,const bool) const' being compiled
2>        with
2>        [
2>            TType=std::string
2>        ]
2>itkDCMTKImageIOTest.cxx
2>itkDCMTKRGBImageIOTest.cxx
2>itkDCMTKSeriesReadImageWrite.cxx
2>itkDCMTKSeriesStreamReadImageWrite.cxx
2>itkDCMTKImageIOOrthoDirTest.cxx
2>itkDCMTKMultiFrame4DTest.cxx
2>itkDCMTKLoggerTest.cxx
2>itkDCMTKImageIOSlopeInterceptTest.cxx
2>itkDCMTKImageIOMultiFrameImageTest.cxx
2>itkDCMTKImageIONoPreambleTest.cxx
2>Generating Code...
2>Done building project "ITKIODCMTKTestDriver.vcxproj" -- FAILED.

I don't know where the fix needs to be applied, but it is almost certainly somewhere in ITK's DCMTK-related CMake code.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[jhlegarreta]

This is still relevant stale bot. I am willing to take up this as soon as #2817 gets resolved.

[jhlegarreta]

Making the test quantitative would be good, but that can be a follow-up commit. This is good as-is.

I may give a go at this. If the module is turned on by the CIs, they'll tell about the tag contents. I'd think of two ways to make this quantitative:

• Write the expected tag values to a text file, provide it as the input to the test, read each line and compare each value with the value read from the DICOM files.
• Write the values read from the DICOM files to a text file, provide a text file with the expected tag values as a baseline to CMake, and let it do the comparison job.

If the latter is possible, it involves less code, and allows for improvements (e.g. reading/comparing more tags) without minimal changes to the test code.

@dzenanz Wondering whether the CMake compare is able to compare two text files (i.e. without using the content links).

[issakomi]

DCMTK has own tests, BTW, there are some macros too e.g. OFCHECK_EQUAL https://github.com/DCMTK/dcmtk/tree/master/dcmdata/tests

[jhlegarreta]

DCMTK has own tests

:+1: Here we are trying to make sure that ITK's DCMTK IO object reads what it is supposed to read.

BTW, there are some macros too e.g. OFCHECK_EQUAL https://github.com/DCMTK/dcmtk/tree/master/dcmdata/tests

:+1: Thanks Mihail.

[dzenanz]

CMake can compare files (syntax), but only binary comparison. It should work in this case, and it is less work to set up than read the file and compare ourselves. The downside is that error message will be "file are different", rather than more meaningful messages we can craft in the custom code. If you have time, you can go the custom comparison route. Otherwise let CMake compare the outputs.

P.S. I don't know how easy or hard it is to turn CMake -E compare_files into a test.

[dzenanz]

Slicer should be able to read DICOM tags.

[jhlegarreta]

P.S. I don't know how easy or hard it is to turn CMake -E compare_files into a test.

Yes, that one I had found yesterday, but did not find out whether our compare argument does also accept text files.

[issakomi]

FYI, some parts are too bad for testing and coverage, e.g. this function. It takes Bits Allocated (0x0028, 0x0100) with wrong name SamplesPerPixel and assumes that if Bits Allocated is 8 or 16 it means that the image is scalar, otherwise RGB or VECTOR, it is wrong. RGB image has of course Bits Allocated 8 or 16 too and Samples per Pixel (0x0028,0x0002) 3. And e.g. 32 bits dose (RT) image is not a vector. Also Photometric Interpretation should be checked. There were some very very old ACR-NEMA images with bitsallocated 24 for RGB, extremely rare.

IOPixelEnum
DCMTKFileReader::GetImagePixelType() const
{
  unsigned short SamplesPerPixel;
  if (this->GetElementUS(0x0028, 0x0100, SamplesPerPixel, false) != EXIT_SUCCESS)
  {
    return IOPixelEnum::UNKNOWNPIXELTYPE;
  }
  IOPixelEnum pixelType;
  switch (SamplesPerPixel)
  {
    case 8:
    case 16:
      pixelType = IOPixelEnum::SCALAR;
      break;
    case 24:
      pixelType = IOPixelEnum::RGB;
      break;
    default:
      pixelType = IOPixelEnum::VECTOR;
  }
  return pixelType;
}

Edit: this function is unused, pixel type is processed in ReadImageInformation (also with issues, but much better)

[jhlegarreta]

@issakomi Thanks for the investigation. I'd be more than happy to add more test cases to test the relevant methods if the appropriate DICOM data can be shared.

[issakomi]

@issakomi Thanks for the investigation. I'd be more than happy to add more test cases to test the relevant methods if the appropriate DICOM data can be shared.

I think i have mentioned a bug, better say forgotten garbage in DCMTKFileReader class. There are many test files for GDCM tests. But i think they should be better tested manually first to get some knowledge of the class, e.g. we don't know does PALETTE_COLOR works or not.

The test is here, it is complete minimal standalone test. Not intended to be included in test suite.

#include "itkImage.h"
#include "itkImageFileReader.h"
#include "itkDCMTKImageIO.h"
#include "itkDCMTKFileReader.h"
#include <iostream>

int main(int, char **)
{
  auto imageIO = itk::DCMTKImageIO::New();
  imageIO->SetFileName("../data/raw-RGB.dcm");
  try
  {
    imageIO->ReadImageInformation();
  }
  catch (const itk::ExceptionObject & ex)
  {
    std::cout << ex.GetDescription() << std::endl;
  }
  itk::IOPixelEnum pixel_type1 = imageIO->GetPixelType();
  std::cout << "DCMTKImageIO reported Pixel Type  "
    << imageIO->GetPixelTypeAsString(pixel_type1)
    << std::endl;

  // GetImagePixelType() is unused garbage
  itk::DCMTKFileReader dcmtkFileReader;
  dcmtkFileReader.SetFileName("../data/raw-RGB.dcm");
  dcmtkFileReader.LoadFile();
  itk::IOPixelEnum pixel_type2 = dcmtkFileReader.GetImagePixelType();
  std::cout << "DCMTKFileReader reported Pixel Type  "
    << imageIO->GetPixelTypeAsString(pixel_type2)
    << std::endl;
  return 0;
}

DCMTKImageIO reported Pixel Type  rgb
DCMTKFileReader reported Pixel Type  scalar

Sorry, but i am out and will not do any more work on DCMTK in the near future.