NIFTI-Imaging / nifti_clib

C libraries for NIFTI support
Other
35 stars 25 forks source link

nifti_findimgname loads incorrect file #129

Open vanossj opened 3 years ago

vanossj commented 3 years ago

https://github.com/NIFTI-Imaging/nifti_clib/blob/7934f742f435c3d6628a1099cd2dba14d6cd2216/niftilib/nifti1_io.c#L2841

nifti_findimgname doesn't load the passed filepath. It searches through extensions and loads the first matching filename+extension.

If A.nii and A.nii.gz are in a directory, the library will load A.nii even if it was told to load A.nii.gz

Issue originally raised in https://github.com/InsightSoftwareConsortium/ITK/issues/2243 and https://github.com/Slicer/Slicer/issues/5378

afni-rickr commented 3 years ago

@vanossj While I basically agree that priority should be given to the original name, it seems like a faulty situation to begin with. It is begging for trouble to have FILE.nii and FILE.nii.gz in the same location, as well as a FILE.hdr/img pair. One could argue against that point, but then the library should not even allow/search for any file but what is provided. If a change for this is made, the library should then also whine about the existence of the multiple "matching" files. If it does not do that, it should not even look for other files. Do you agree that the library should whine about this situation, basically suggesting it is an error?

vanossj commented 3 years ago

Is there a use case for search for matching files instead of always loading the requested file?

~If the user asks for an invalid file and the fuction returns a valid file is that desired behavior? That seems like asking for just as much trouble.~ Actually this particular case this is checked for. The function returns NULL instead of checking for a matching file

Options are:

  1. Keep current behavior, but whine about returning a different file than was asked for.
  2. Only load the file the user asked for, if its valid.

My vote is for option 2