Make NIfTI a required dependency (or at least ON by default)
schuhschuh opened this issue · 3 comments
The IRTK tools to be of practical use require the NIfTI library because this is the (possibly only) real image format that we support.
I considered anything that has conditional inclusion in the code, i.e which has a respective HAS macro as an optional dependency with a corresponding WITH option.
FYI:
grep -nr HAS_NIFTI Modules/
Modules/Image/src/irtkFileToImage.cc:60:#ifdef HAS_NIFTI
Modules/Image/src/irtkImageToFile.cc:85:#ifdef HAS_NIFTI
Modules/Image/src/irtkBaseImage.cc:180:#ifdef HAS_NIFTI
Modules/Image/src/irtkImageToFileNIFTI.cc:17:#ifdef HAS_NIFTI
Modules/Image/src/irtkFileNIFTIToImage.cc:17:#ifdef HAS_NIFTI
Modules/Image/include/irtkFileNIFTIToImage.h:21:#ifdef HAS_NIFTI
Modules/Image/include/irtkImageToFileNIFTI.h:21:#ifdef HAS_NIFTI
Modules/Image/include/irtkImageToFile.h:98:#ifdef HAS_NIFTI
Modules/Image/include/irtkFileToImage.h:125:#ifdef HAS_NIFTI
Modules/Image/include/irtkNIFTI.h:21:#ifdef HAS_NIFTI
Modules/Registration/src/irtkProbabilisticImageSimilarity.cc:396:#ifdef HAS_NIFTI
Modules/Registration/src/irtkNormalizedIntensityCrossCorrelation.cc:1168:#ifdef HAS_NIFTI
So either we get rid of all the conditionals, and make NifTI a requirement or we keep the WITH option, either opt-in or opt-out. I am happy to make it opt-out if that fixes your issue.
I disagree. Just because code is prepared to be optional for advanced or future purposes doesn't mean that the build configuration must be too. And when, then just set the option to ON be default and make it advanced. Right now, only NIFTI is supported of all medical image formats. But who says this may not change in the future. Then the conditional statements might be useful.
So I don't think it's necessarily an either or question. But if you prefer to get rid of the conditionals in the code, go ahead.