ITK
ITK copied to clipboard
Parse strings as multi-element data structures in tests
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.
Inspiration for this could be found somewhere in https://github.com/Slicer/SlicerExecutionModel.
@jhlegarreta adding a few example tests where this could be applied would make it a "good first issue".
@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).