[ENH] Add copy_header option to N4BiasFieldCorrection #2034
Conversation
This N4 algorithm does not propagate the nifti headers after run. This PR allows setting the option ``copy_header=True`` and that will overwrite the N4 output with the corrected data and the original headers.
| nii = nb.load(out_file) | ||
| hdr.set_data_dtype(nii.header.get_data_dtype()) | ||
| nb.Nifti1Image(nii.get_data(), refnii.affine, hdr).to_filename( | ||
| out_file) |
There was a problem hiding this comment.
Does N4BiasFieldCorrection always use NIfTI images, or are we making this assumption? To be more general, this could be:
if self.inputs.copy_header and runtime.returncode in correct_return_codes:
import nibabel as nb
in_img = nb.load(self.inputs.input_image)
out_file = self._gen_filename('output_image')
out_img = nb.load(out_file, mmap=False)
new_img = out_img.__class__(out_img.get_data(), in_img.affine, in_img.header)
new_img.set_data_dtype(out_img.get_data_dtype())
new_img.to_filename(out_file)A couple additional notes:
- Should make sure we ran successfully before reading/overwriting the output
mmap=Falseonnb.load()- See Feature Request: Detect when writing to open mmap nibabel#492- No need to copy header (that's done in image creation)
- I prefer to do
get/set_data_dtypeon the image itself, rather than the header, even though they're equivalent. Since an image contains a reference to the header and not the other way around, if some image class needs (in a future nibabel version) to modify the data portion of the image during this call, it can. Your call on this one.
There was a problem hiding this comment.
N4BiasFieldCorrection virtually accepts any of the file formats supported by ITK. So I guess it is worth checking the image type (Analyze or Nifti) before copying the header. @satra any thoughts?
|
I imagine we should do the same for |
Codecov Report
@@ Coverage Diff @@
## master #2034 +/- ##
==========================================
- Coverage 72.19% 72.18% -0.02%
==========================================
Files 1132 1132
Lines 57032 57044 +12
Branches 8167 8168 +1
==========================================
+ Hits 41175 41177 +2
- Misses 14572 14582 +10
Partials 1285 1285
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #2034 +/- ##
==========================================
- Coverage 72.19% 72.18% -0.02%
==========================================
Files 1132 1132
Lines 57032 57044 +12
Branches 8167 8168 +1
==========================================
+ Hits 41175 41177 +2
- Misses 14572 14582 +10
Partials 1285 1285
Continue to review full report at Codecov.
|
RF: Factor _copy_header, update bias image
|
@oesteban - what aspects of the header does it not copy? as in, is the output file an invalid Nifti file? and is this an issue that should be fixed in ANTs? |
|
@satra we started here nipreps/fmriprep#497 . I don't know exactly what are the problems, I could go deeper into it. EDIT: but the output files are valid nifti. @effigies that would make sense for consistency |
|
@oesteban - i looked at the issue, the voxel size bit seems related to floating point representation and is fine. the shape mismatch seems weird. so two questions
i'm worried that we shouldn't be patching bugs in other software, if they are indeed bugs. has this been reported on ants? and if we patch, we need to be very clear to un-patch it when it's fixed. @kaczmarj, @mgxd - could we create a test across different versions of ANTs for N4 and test this issue? |
|
@oesteban - any thoughts on my comment? |
|
We can go ahead with this patch in our settings (fmriprep and Co.) and close this PR if you prefer to wait for ANTs to solve the issue. On the other hand, they don't release very often, so this patch could be an incentive for using nipype in the meantime. Since the major problem is the change of the qform code, I think nipype could very well use this patch. EDIT: but overall yes, I agree we shouldn't patch problems that we identify on the underlying interfaces and rather report them to the appropriate places 👍 |
|
Update: since the qform issue derives from ITK, it is not likely to change (and we probably should address this in all ITK-related interfaces, to keep workflows consistent). Then the type-casting is the user's responsibility. I can see nipype helping on that. |
This N4 algorithm does not propagate the nifti headers after run.
This PR allows setting the option
copy_header=Trueand that willoverwrite the N4 output with the corrected data and the original headers.
Default value of
copy_headerisFalse