BUG: Fix failing isAffine check due to insuffcient precision in calculations #3339

gdevenyi · 2022-03-25T23:13:21Z

Fixes #3333

Co-authored-by: Vladimir S. FONOV vladimir.fonov@gmail.com

PR Checklist

No API changes were made (or the changes have been approved)
No major design changes were made (or the changes have been approved)
Updated API documentation (or API not changed)
Added license to new files (if any)
Added test
NO Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
NO Added ITK examples for all new major features (if any)

gdevenyi · 2022-03-25T23:15:11Z

This fixed my issues with my high resolution human brains and rat brains I've been working with.

Modules/IO/NIFTI/src/itkNiftiImageIO.cxx

N-Dekker

@gdevenyi Thanks for addressing my commits, Gabriel! 👍

You might still consider adjusting the subject line of the commit text to get it at at most 72 chars, for example:

BUG: Fix failure isAffine due to insufficient precision in calculations

Because currently GitHub breaks the last word of the subject line in two: "caluc……lations" at
abc6b33

But it's not a big deal to me, of course! Approved anyway!

Modules/IO/NIFTI/src/itkNiftiImageIO.cxx

Leengit · 2022-03-28T12:44:57Z

Depending upon where I look in ITK, I see different usage for converting from float to double for a new local variable:

auto   xd0 = static_cast<double>(xf * xf);
auto   xd1 = double{ xf * xf };
double xd2 = xf * xf;

Which is preferred?

hjmjohnson · 2022-03-28T13:01:20Z

I prefer static_cast( ). It is verbose but very explicit about the desired behavior.

N-Dekker · 2022-03-28T13:04:54Z

Depending upon where I look in ITK, I see different usage for converting from float to double for a new local variable:
auto   xd0 = static_cast<double>(xf * xf);
auto   xd1 = double{ xf * xf };
double xd2 = xf * xf;
Which is preferred?

Good question 🤔 I would prefer not to use static_cast<double>(xf * xf) in that particular case, as it might silently convert from integer to floating point. static_cast<double> is too "brute force", when you just want to convert one floating point type to another.

Leengit · 2022-03-28T13:11:09Z

Does the answer change for potentially lossy casts such as between signed and unsigned or from double to int, etc? (N.B. Although the compiler may not know it, hopefully we know that these losses won't happen given the context, or are acceptable.)

N-Dekker · 2022-03-28T13:14:52Z

Does the answer change for potentially lossy casts such as between signed and unsigned or from double to int, etc? (N.B. Although the compiler may not know it, hopefully we know that these losses won't happen given the context, or are acceptable.)

Sure! static_cast is typically for potentially lossy casts. On the other hand, double{ x } is lossless (as checked by the compiler).

hjmjohnson · 2022-03-28T13:26:24Z

Does the answer change for potentially lossy casts such as between signed and unsigned or from double to int, etc? (N.B. Although the compiler may not know it, hopefully we know that these losses won't happen given the context, or are acceptable.)

Sure! static_cast is typically for potentially lossy casts. On the other hand, double{ x } is lossless (as checked by the compiler).

I answered giving my preference under the condition of converting from float to double, and assuming that the conversion was required. I prefer explicit documentation that the developer understands that the conversion is necessary and desired. I work with novice developers, and the explicit documentation of the desired behavior is welcome, even when it is redundnant.

Leengit · 2022-03-28T13:26:33Z

And by its omission, am I to assume that you would not use the third form in either the lossy or lossless scenarios?

hjmjohnson · 2022-03-28T13:41:54Z

And by its omission, am I to assume that you would not use the third form in either the lossy or lossless scenarios?

Personally, I would not. I know that it is a lossless conversion, but novice developers benefit from having the conversion made painfully obvious.

N-Dekker · 2022-03-28T13:42:04Z

And by its omission, am I to assume that you would not use the third form in either the lossy or lossless scenarios?

In this particular case, conversion from float to double:

ITK/Modules/IO/NIFTI/src/itkNiftiImageIO.cxx

Lines 1756 to 1760 in abc6b33

    
           vnl_matrix_fixed<double, 4, 4> mat; 
        
           for (int i = 0; i < 4; ++i) 
        
             for (int j = 0; j < 4; ++j) 
        
               mat[i][j] = double{ nifti_mat.m[i][j] };

In this case, implicit conversion mat[i][j] = nifti_mat.m[i][j]; would be OK to me as well. Implicit conversion is quite safe, in C++.

N-Dekker · 2022-03-28T13:45:36Z

I know that it is a lossless conversion, but novice developers benefit from having the conversion made painfully obvious.

Still, novice developers need to learn about double{ x } conversion, as well as about implicit conversion. I'm sorry to say so, but C++ has become quite an advanced and complicated language. We cannot hide that away from novice developers.

N-Dekker · 2022-03-28T14:19:03Z

While I have already approved this PR, I think it would be worth adding a minimal unit test, that checks the fix. Something like this?

From @vfonov at #3333 (comment)

0.300000012, 0, 0, -81.6654968
0, 0.300000012, 0, -79.5957031
0, 0, 0.300000012, -26.6077003
0, 0, 0, 1

Would that be the most simplified (minimal) test case?

Update: I see now, the IsAffine function cannot be unit-tested directly, because it is an internal helper function inside the unnamed namespace of itkNiftiImageIO.cxx. Which is OK, because it just isn't part of the public ITK API. So sorry for the noise!

ITK/Modules/IO/NIFTI/src/itkNiftiImageIO.cxx

Line 1754 in e00bad0

IsAffine(const mat44 & nifti_mat)

Leengit · 2022-03-28T14:22:41Z

How about the following two; which do you prefer when xf is a float?

auto   xd1 = double{ xf * xf };
double xd3{ xf * xf };

vfonov · 2022-03-28T15:04:11Z

I made a simple test (also showing the limits of the original code from http://callumhay.blogspot.com/2010/10/decomposing-affine-transforms.html:


template<typename T> static bool
IsAffine(float _mat[4][4])
{
  vnl_matrix_fixed<T, 4, 4> mat;

  for(int i=0;i<4;i++)
  for(int j=0;j<4;j++)
    mat[i][j]=static_cast<T>(_mat[i][j]);

  // First make sure the bottom row meets the condition that it is (0, 0, 0, 1)
  {
    T bottom_row_error = std::fabs(mat[3][3] - 1.0);
    for (int i = 0; i < 3; ++i)
    {
      bottom_row_error += fabs(mat[3][i]);
    }
    if (bottom_row_error > std::numeric_limits<T>::epsilon())
    {
      return false;
    }
  }

  const T condition = vnl_matrix_inverse<T>(mat.as_matrix()).well_condition();
  // Check matrix is invertible by testing condition number of inverse
  if (!(condition > std::numeric_limits<T>::epsilon()))
  {
    return false;
  }

  // Calculate the inverse and separate the inverse translation component
  // and the top 3x3 part of the inverse matrix
  const vnl_matrix_fixed<T, 4, 4> inv4x4Matrix = vnl_matrix_inverse<T>(mat.as_matrix()).as_matrix();
  const vnl_vector_fixed<T, 3>    inv4x4Translation(inv4x4Matrix[0][3], inv4x4Matrix[1][3], inv4x4Matrix[2][3]);
  const vnl_matrix_fixed<T, 3, 3> inv4x4Top3x3 = inv4x4Matrix.extract(3, 3, 0, 0);

  // Grab just the top 3x3 matrix
  const vnl_matrix_fixed<T, 3, 3> top3x3Matrix = mat.extract(3, 3, 0, 0);
  const vnl_matrix_fixed<T, 3, 3> invTop3x3Matrix = vnl_matrix_inverse<T>(top3x3Matrix.as_matrix()).as_matrix();
  const vnl_vector_fixed<T, 3>    inv3x3Translation = -(invTop3x3Matrix * mat.get_column(3).extract(3));

  // Make sure we adhere to the conditions of a 4x4 invertible affine transform matrix
  const T     diff_matrix_array_one_norm = (inv4x4Top3x3 - invTop3x3Matrix).array_one_norm();
  const T     diff_vector_translation_one_norm = (inv4x4Translation - inv3x3Translation).one_norm();
  constexpr T normed_tolerance_matrix_close = 1e-2;
  return !((diff_matrix_array_one_norm > normed_tolerance_matrix_close) ||
           (diff_vector_translation_one_norm > normed_tolerance_matrix_close));
}

testing results:

  float case0[4][4]={
    0.300000012, 0, 0, -8.16654968,
    0, 0.300000012, 0, -7.95957031,
    0, 0, 0.300000012, -2.66077003,
    0, 0, 0, 1
  };

  float case1[4][4]={
    0.300000012, 0, 0, -81.6654968,
    0, 0.300000012, 0, -79.5957031,
    0, 0, 0.300000012, -26.6077003,
    0, 0, 0, 1
  };

  float case2[4][4]={
    0.003000012, 0, 0, -8166549.68,
    0, 0.003000012, 0, -7959570.31,
    0, 0, 0.003000012, -2660770.03,
    0, 0, 0, 1
  };

  std::cerr<<"Case0:"<<std::endl;
  std::cerr<<"float:"<<IsAffine<float>(case0)<<std::endl;
  std::cerr<<"double:"<<IsAffine<double>(case0)<<std::endl;

  std::cerr<<"Case1:"<<std::endl;
  std::cerr<<"float:"<<IsAffine<float>(case1)<<std::endl;
  std::cerr<<"double:"<<IsAffine<double>(case1)<<std::endl;

  std::cerr<<"Case2:"<<std::endl;
  std::cerr<<"float:"<<IsAffine<float>(case2)<<std::endl;
  std::cerr<<"double:"<<IsAffine<double>(case2)<<std::endl;

Outputs:

Case0:
float:1
double:1
Case1:
float:0
double:1
Case2:
float:0
double:0

N-Dekker · 2022-03-28T15:31:12Z

@vfonov Thanks for the test code!

So I see, case2 (below here) is considered non-affine by both IsAffine<float> and IsAffine<double>. Is that mathematically correct? Or is it just a limitation of the method?

  float case2[4][4]={
    0.003000012, 0, 0, -8166549.68,
    0, 0.003000012, 0, -7959570.31,
    0, 0, 0.003000012, -2660770.03,
    0, 0, 0, 1
  };

Just curious. Obviously switching from float to double does improve the precision of IsAffine, so I'm fine with the proposed code change.

hjmjohnson · 2022-03-28T15:35:44Z

How about the following two; which do you prefer when xf is a float?
auto   xd1 = double{ xf * xf };
double xd3{ xf * xf };

Very minor preference for the second option.

hjmjohnson · 2022-03-28T15:39:45Z

I know that it is a lossless conversion, but novice developers benefit from having the conversion made painfully obvious.

Still, novice developers need to learn about double{ x } conversion, as well as about implicit conversion. I'm sorry to say so, but C++ has become quite an advanced and complicated language. We cannot hide that away from novice developers.

I don't disagree that C++ programmers need to learn and be skilled. I would just like to point out that there is a large ITK community that are primarily physicians (mathemeticians/physicists/ect), secondarily medical imaging researchers, and have a tertiary skill set of being C++ programmers. When possible, assisting with their skillset development has benefits to the ITK community.

N-Dekker · 2022-03-28T16:08:37Z

Just recently, I did a few contributions that should make conversions in ITK easier to read: by a new conversion function, itk::ConvertNumberToString(val) (PR #3326), as well as by declaring several converting constructors explicit (PR #3255, #3237, #3238). I hope that novice developers will benefit from them as well 😃

gdevenyi · 2022-05-09T03:52:48Z

Hopefully sorted, I had the absolute test data path instead of the relative one from the NIFTI IO directory.

N-Dekker · 2022-05-09T12:50:52Z

Modules/IO/NIFTI/test/itkNiftiImageIOTest13.cxx

+    using ReaderType = itk::ImageFileReader<ImageType>;
+    ReaderType::Pointer reader = ReaderType::New();
+    reader->SetFileName(argv[1]);
+    reader->Update();
+    ImageType::Pointer image = reader->GetOutput();


Please consider using itk::ReadImage instead. Just one line of code should so it 😃

Maybe some extra checks here to see that the read image is as expected...? Or at least, check that it is a valid image?

I do not have any C++ object-oriented black magic to anything beyond copying the ITK official example, https://examples.itk.org/src/io/imagebase/readanimage/documentation

This test is to explicitly see if the NiftiIO isAffine check fails on the random image I had in my collection which was causing issues

This test is to explicitly see if the NiftiIO isAffine check fails on the random image I had in my collection which was causing issues

I guess you could still add some checks that the read image is valid, for example:

if (image == nullptr) { std::cerr << "Error: The read image is null!\n"; return EXIT_FAILURE; } if (image->GetBufferPointer() == nullptr) { std::cerr << "Error: the buffer of the read image is null!\n"; return EXIT_FAILURE; } if (image->GetBufferedRegion().GetNumberOfPixels() == 0) { std::cerr << "Error: The read image has no pixels!\n"; return EXIT_FAILURE; }

Right?

@gdevenyi Did you still consider adding the extra checks that I suggested here? In order to ensure that the reading does actually yield a valid image?

if (image == nullptr) { std::cerr << "Error: The read image is null!\n"; return EXIT_FAILURE; } if (image->GetBufferPointer() == nullptr) { std::cerr << "Error: the buffer of the read image is null!\n"; return EXIT_FAILURE; } if (image->GetBufferedRegion().GetNumberOfPixels() == 0) { std::cerr << "Error: The read image has no pixels!\n"; return EXIT_FAILURE; }

This can be condiserably abbreviated via itk::ReadImage and ITK_TRY_EXPECT_NO_EXCEPTION. We can do that in a follow-up PR. Let's merge this, as it has dragged on for quite a long time.

Both done in #3448.

Modules/IO/NIFTI/test/itkNiftiImageIOTest13.cxx

N-Dekker · 2022-05-09T15:33:13Z

Modules/IO/NIFTI/test/CMakeLists.txt

+itk_add_test(NAME itkNiftiSmallVoxelsAffinePrecisionTest
+COMMAND ITKIONIFTITestDriver itkNiftiImageIOTest13
+      DATA{Input/SmallVoxels_AffinePrecision.nii.gz})


What about using itkNiftiImageIOTest2 instead, like this:

itk_add_test(NAME itkNiftiSmallVoxelsAffinePrecisionTest COMMAND ITKIONIFTITestDriver itkNiftiImageIOTest2 ${ITK_TEST_OUTPUT_DIR} itkNiftiIOShouldSucceed true DATA{Input/SmallVoxels_AffinePrecision.nii.gz})

Would that also work?

That test with the original code does not fail.

That test with the original code does not fail.

Why not? Originally you added your SmallVoxels_AffinePrecision.nii.gz to an existing itk_add_test. My suggestion (above here) would be to do a new itk_add_test, referring to the old itkNiftiImageIOTest2 cxx file and your SmallVoxels_AffinePrecision.nii.gz file. But that would not work either?

I don't get why itkNiftiImageIOTest2 doesn't fail, because the test file SmallVoxels_AffinePrecision.nii.gz definitely causes an exception for me (running ANTs PrintHeader built against recent ITK).

Reverting the change, and running both tests.

$ ctest -R AffinePrecision --output-on-failure Test project /home/gdevenyi/projects/itk-build Start 1157: itkNiftiSmallVoxelsAffinePrecisionTest 1/2 Test #1157: itkNiftiSmallVoxelsAffinePrecisionTest ............................***Failed 0.04 sec ExceptionObject caught ! itk::ExceptionObject (0x563a086a0fa0) Location: "unknown" File: /home/gdevenyi/projects/ITK/Modules/IO/NIFTI/src/itkNiftiImageIO.cxx Line: 1988 Description: ITK ERROR: ITK only supports orthonormal direction cosines. No orthonormal definition found! Start 1158: itkNiftiSmallVoxelsAffinePrecisionTest_fromitkNiftiImageIOTest2 2/2 Test #1158: itkNiftiSmallVoxelsAffinePrecisionTest_fromitkNiftiImageIOTest2 ... Passed 0.03 sec 50% tests passed, 1 tests failed out of 2 Label Time Summary: ITKIONIFTI = 0.07 sec*proc (2 tests) Total Test time (real) = 0.22 sec The following tests FAILED: 1157 - itkNiftiSmallVoxelsAffinePrecisionTest (Failed) Errors while running CTest

So itkNiftiImageIOTest2 just cannot be used for SmallVoxels_AffinePrecision.nii.gz, for some unknown reason, right?

So itkNiftiImageIOTest2 just cannot be used for SmallVoxels_AffinePrecision.nii.gz, for some unknown reason, right?

Correct.

The difference between the two sets of code appears to be

itk::NiftiImageIO::Pointer io = itk::NiftiImageIO::New(); auto imageReader = ImageReaderType::New(); imageReader->SetImageIO(io);

Where the NiftiimageIO ImageIO is explicitly set in itkNiftiImageIOTest2

@gdevenyi Indeed, itkNiftiImageIOTest2 does explicitly set ImageIO, but it does also do a "read" without explicitly setting ImageIO. By calling itk::IOTestHelper::ReadImage:

ITK/Modules/IO/NIFTI/test/itkNiftiImageIOTest2.cxx

Line 57 in 8c2c654

input = itk::IOTestHelper::ReadImage<ImageType>(std::string(arg2));

Anyway, it seems to me that the CMake code of itkNiftiIOShouldSucceed was already incorrect. It says:

itk_add_test(NAME itkNiftiIOShouldSucceed COMMAND ITKIONIFTITestDriver itkNiftiImageIOTest2 ${ITK_TEST_OUTPUT_DIR} itkNiftiIOShouldSucceed true DATA{${ITK_DATA_ROOT}/Input/LittleEndian.hdr,LittleEndian.mhd,LittleEndian.img})

ITK/Modules/IO/NIFTI/test/CMakeLists.txt

Lines 55 to 57 in 6fc6164

itk_add_test(NAME itkNiftiIOShouldSucceed

COMMAND ITKIONIFTITestDriver itkNiftiImageIOTest2

${ITK_TEST_OUTPUT_DIR} itkNiftiIOShouldSucceed true DATA{${ITK_DATA_ROOT}/Input/LittleEndian.hdr,LittleEndian.mhd,LittleEndian.img})

The arguments of itk_add_test don't seem to match correctly with arg1, arg2, and prefix at

ITK/Modules/IO/NIFTI/test/itkNiftiImageIOTest2.cxx

Lines 32 to 42 in 8c2c654

if (argc != 4)

return EXIT_FAILURE;

char * arg1 = argv[1];

char * arg2 = argv[2];

char * prefix = argv[3];

int test_success = 0;

using ImageType = itk::Image<short, 3>;

using ImagePointer = ImageType::Pointer;

if ((strcmp(arg1, "true") == 0) && WriteNiftiTestFiles(prefix) == -1)

When I run it locally, arg1 == "itkNiftiIOShouldSucceed", arg2 == "true", and prefix == "C:/X/bin/I/ITK/ExternalData/Testing/Data/Input/LittleEndian.hdr". While arg1 should have been the boolean ("true" or "false"), and arg2 should have been the input file name. Right?

I think itkNiftiSmallVoxelsAffinePrecisionTest should be more like the following, if you would still like to use the existing itkNiftiImageIOTest2:

itk_add_test(NAME itkNiftiSmallVoxelsAffinePrecisionTest COMMAND ITKIONIFTITestDriver itkNiftiImageIOTest2 ${ITK_TEST_OUTPUT_DIR} true DATA{${ITK_DATA_ROOT}/Input/SmallVoxels_AffinePrecision.nii.gz} SomeSpecificPrefix)

I don't know what exactly the prefix ("SomeSpecificPrefix") should be. But it should be the last argument to itk_add_test.

My two cents, Niels

N-Dekker · 2022-05-12T20:58:28Z

@gdevenyi Would a smaller test image file be possible? I just noticed that the file you added for this test, SmallVoxels_AffinePrecision.nii.gz is more than 1 MB! https://data.kitware.com/#item/627539e24acac99f42c8929a

If I understand correctly, the only part of the image that is relevant to this test is the 4x4 matrix at m_NiftiImage->sto_xyz, right? That's just 64 bytes.

gdevenyi · 2022-05-13T16:59:05Z

@gdevenyi Would a smaller test image file be possible? I just noticed that the file you added for this test, SmallVoxels_AffinePrecision.nii.gz is more than 1 MB! https://data.kitware.com/#item/627539e24acac99f42c8929a

If I understand correctly, the only part of the image that is relevant to this test is the 4x4 matrix at m_NiftiImage->sto_xyz, right? That's just 64 bytes.

I took the problematic naturalistic image I had, and zeroed all the voxels to make it maximally compressible. I don't know the NIFTI format well enough to successfully modify the file any further without accidentally breaking the important properties.

cookpa · 2022-05-13T17:14:13Z

Does this work? I just copied the header to a smaller array using RNifti

smallVoxels_affinePrecision_reduced.nii.gz

issakomi · 2022-05-14T09:37:55Z

Modules/IO/NIFTI/src/itkNiftiImageIO.cxx

  // First make sure the bottom row meets the condition that it is (0, 0, 0, 1)
  {
-    float bottom_row_error = itk::Math::abs(mat[3][3] - 1.0f);
+    double bottom_row_error = itk::Math::abs(mat[3][3] - 1.0);


is float epsilon several lines below (line 1769) intentional?

if (bottom_row_error > std::numeric_limits<float>::epsilon())

I thought that it might have been forgotten.

is float epsilon several lines below (line 1769) intentional?

@issakomi Good question. But I wonder, isn't std::numeric_limits<T>::epsilon() rather arbitrary anyway? I just don't know, but if mat[3][i] must be close to zero (for i = 0, 1, 2), then those matrix elements might as well be compared against std::numeric_limits<T>::min(), which if much smaller than std::numeric_limits<T>::epsilon().

Honestly I'm just wondering what is intended as well. I have no suggestion here.

I think the bottom row should be precisely (0,0,0,1) - but I guess it's possible there might be some epsilon error when the inverse is calculated

I think it was just forgotten.

there might be some epsilon error when the inverse is calculated

OK, but std::numeric_limits<T>::epsilon() isn't just some epsilon error. It is specifically the difference between 1 and the least value greater than 1 that is representable by floating point type T. https://www.cplusplus.com/reference/cfloat/ That's why I wonder if this specific epsilon should also be used for values that should be close to zero (rather than close to one). But I just don't know, honestly.

One further idea here, currently the test is made with the sum as each entry added. Should we instead test the individual entries?

Changing to ::min(), or testing the individual entries will be substantially stricter than the existing implementation as well.

N-Dekker · 2022-05-14T12:45:56Z

Does this work? I just copied the header to a smaller array using RNifti

smallVoxels_affinePrecision_reduced.nii.gz

@cookpa Thanks Philip. Your reduced nii.gz does trigger the "ITK ERROR: ITK only supports orthonormal direction cosines" when I try to read it by itk::ReadImage, using an ITK version without Gabriel's (@gdevenyi) proposed fix. (ITK v5.3rc03, VS2019, Windows.) And it's just 129 bytes! 😃

Fixes InsightSoftwareConsortium#3333 Co-authored-by: Vladimir S. FONOV <vladimir.fonov@gmail.com>

gdevenyi · 2022-05-16T17:36:33Z

Thanks @cookpa and @N-Dekker. I've updated the test file.

N-Dekker · 2022-05-16T18:28:30Z

I've updated the test file.

@gdevenyi That's really great, Gabriel, thanks! Can you possibly also remove the large (1.042 MB) version of SmallVoxels_AffinePrecision.nii.gz from https://data.kitware.com/#item/627539e24acac99f42c8929a ?

gdevenyi · 2022-05-16T18:33:04Z

@gdevenyi That's really great, Gabriel, thanks! Can you possibly also remove the large (1.042 MB) version of SmallVoxels_AffinePrecision.nii.gz from https://data.kitware.com/#item/627539e24acac99f42c8929a ?

As far as I understood, I did. The PR only includes the HASH of the small replacement file.

N-Dekker · 2022-05-16T18:48:47Z

@gdevenyi That's really great, Gabriel, thanks! Can you possibly also remove the large (1.042 MB) version of SmallVoxels_AffinePrecision.nii.gz from https://data.kitware.com/#item/627539e24acac99f42c8929a ?

As far as I understood, I did.

But it's still there at https://data.kitware.com/#item/627539e24acac99f42c8929a ! Right? Isn't it possible to remove the file from https://data.kitware.com/#item/627539e24acac99f42c8929a ?

gdevenyi · 2022-05-16T19:00:34Z

But it's still there at https://data.kitware.com/#item/627539e24acac99f42c8929a ! Right? Isn't it possible to remove the file from https://data.kitware.com/#item/627539e24acac99f42c8929a ?

data.kitware.com is a bit silly. Following your link there's no option for deletion, but if I navigate to my files via my personal account I can delete it. Its now gone.

It would've never been fetched anyways.

gdevenyi · 2022-05-18T13:46:58Z

@hjmjohnson I believe we're just waiting on you here. Can you please re-review?

gdevenyi · 2022-05-24T14:15:58Z

Following up again here, this is ready to go.

dzenanz · 2022-05-25T12:50:22Z

Modules/IO/NIFTI/test/itkNiftiImageIOTest13.cxx

+    using ReaderType = itk::ImageFileReader<ImageType>;
+    ReaderType::Pointer reader = ReaderType::New();
+    reader->SetFileName(argv[1]);
+    reader->Update();
+    ImageType::Pointer image = reader->GetOutput();


This can be condiserably abbreviated via itk::ReadImage and ITK_TRY_EXPECT_NO_EXCEPTION. We can do that in a follow-up PR. Let's merge this, as it has dragged on for quite a long time.

addressed

github-actions bot added area:IO Issues affecting the IO module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances labels Mar 25, 2022

N-Dekker reviewed Mar 26, 2022

View reviewed changes