ITK icon indicating copy to clipboard operation
ITK copied to clipboard

Parse strings as multi-element data structures in tests

Open jhlegarreta opened this issue 5 years ago • 3 comments

Description

Some tests require a number of arguments that are part of the same data structures (e.g. the image size, or resolution). Having a utility function in a single place to parse the input parameter strings as a multi-element data structure (e.g. a vector) would be desirable.

Expected behavior

To be able to parse a string as a vector or whatever multi-element data structure.

Actual behavior

Developer's are forced to parse the string on each test that requires such an input.

jhlegarreta avatar Feb 16 '19 23:02 jhlegarreta

Inspiration for this could be found somewhere in https://github.com/Slicer/SlicerExecutionModel.

dzenanz avatar Jun 02 '22 07:06 dzenanz

@jhlegarreta adding a few example tests where this could be applied would make it a "good first issue".

dzenanz avatar Jun 02 '22 07:06 dzenanz

@jhlegarreta adding a few example tests where this could be applied would make it a "good first issue".

I should have done this when I opened the issue, I am really sorry. I had a hard time finding examples of these cases, either - because I am having a hard time remembering what I meant 🤦‍♂️,

  • because the naming chosen for parsing these elements in the tests is not intuitive (I can take the blame),
  • because the words I used for the search were not accurate enough,
  • because since these common methods do not exist, I have systematically avoided to add such parameters to tests/re-write parsers on a case-by-case basis,
  • because the classes/tests were deprecated/moved to a remote.

But I think these cases illustrate what I meant (when git blaming the files, the commits that added the parser methods turn out to be mine, so probably this issue partially arose from writing them 🙃): https://github.com/InsightSoftwareConsortium/ITK/blob/23da3dfa79152648f3d38050ee147df47f41ce0a/Modules/Core/ImageFunction/test/itkBSplineDecompositionImageFilterTest.cxx#L37 https://github.com/InsightSoftwareConsortium/ITK/blob/23da3dfa79152648f3d38050ee147df47f41ce0a/Modules/Filtering/Denoising/test/itkPatchBasedDenoisingImageFilterTest.cxx#L37

This one might also be relevant: https://github.com/InsightSoftwareConsortium/ITK/blob/23da3dfa79152648f3d38050ee147df47f41ce0a/Modules/Video/IO/test/itkFileListVideoIOTest.cxx#L305 This one is an example file, not a test (but the same principle may apply): https://github.com/InsightSoftwareConsortium/ITK/blob/23da3dfa79152648f3d38050ee147df47f41ce0a/Examples/Filtering/DigitallyReconstructedRadiograph1.cxx#L126

So (I think that) the idea was/is that instead of specifying e.g. the image size test parameter as imageSizeX imageSizeY imageSizeZ we could specify it as imageSize (in the test usage), specifying the argument in the CMakeLists.txt as e.g. "10 20 30" (instead of 10 20 30), and have a method to parse the string and fill the image size array. Now this assumes that the e.g. image dimension is either inferred from the test parameters or is added as an explicit parameter (the latter may make things easier, but gives room for incurring into more inconsistencies I think). This makes the test less explicit, since we do not know the image dimensionality by just reading the test (we need to go to the CMakeLists.txt file), but gives way to potentially increase the number of tests done without duplicating the code.

As an example, the following is a test where the spacing is required as 3 separate parameters: https://github.com/InsightSoftwareConsortium/ITK/blob/23da3dfa79152648f3d38050ee147df47f41ce0a/Modules/IO/GDCM/test/itkGDCMSeriesStreamReadImageWriteTest.cxx#L54

In the case of other lists that do not influence the dimensionality of the template parameters, like in https://github.com/InsightSoftwareConsortium/ITK/blob/23da3dfa79152648f3d38050ee147df47f41ce0a/Modules/Core/ImageFunction/test/itkBSplineDecompositionImageFilterTest.cxx#L37, the need for the parser is clear I think. These would also apply to e.g. some rigid/affine transformation matrices that one would be willing to test (or these could be stored in e.g. txt files, but the latter should also get parsed, and such parsers be common/re-used).

jhlegarreta avatar Jun 03 '22 00:06 jhlegarreta