Nifti image conflicted names #9

hjmjohnson · 2025-01-23T20:02:40Z

If there are conflicted names, prefer to give a un-ambigous error stating the problem rather than guessing what to do.

Related to: InsightSoftwareConsortium/ITK#2243

Inspired by: NIFTI-Imaging#138 @neurolabusc

emulate FSL when multiple ambiguous filenames exist (.nii/.nii.gz), Configure 'run "FSL=1 make"' and 'cmake -DFSLSTYLE=ON .' Make names consistent nifti_strdup() is too small to include an extension

emulate AFNI pigz support Fixed conversion error when PIGZ is defined. Configure 'run "FSL=1 make"' and 'cmake -DFSLSTYLE=ON .'

Reject complex data like FSL does Configure 'run "FSL=1 make"' and 'cmake -DFSLSTYLE=ON .'

hjmjohnson · 2025-01-23T20:08:19Z

@neurolabusc Could you please review this update to your suggestions posed several years ago to NIFTI-Imaging/nifti_clib? These changes may provide a fix for an issue reported to ITK.

Thanks,
Hans

hjmjohnson · 2025-01-23T20:12:17Z

@gdevenyi @lassoan Would you mind commenting on this upstream change as a solution to InsightSoftwareConsortium/ITK#2243? We could have ITK build with the nifti libraries with FSLSTYLE_NAME_CONFLICTS:BOOL=ON.

gdevenyi · 2025-01-24T03:45:43Z

Well, its 3 commits, which is what I suggested in the original PR :)

Erroring out on some of the more...interesting choices of how the reader works seems the most sensible thing to do, mirroring how FSL does it already (and re-using the environment variables) is good too.

I'm agnostic on pigz, its a band-aid to improve things on a file format lacking good internal compression.

seanm

IMHO this was merged prematurely...

seanm · 2025-01-29T18:57:38Z

nifti2/nifti2_io.c

+   #ifdef REJECT_COMPLEX
+   if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) {
+      fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname);
+      exit(13);


This was a few years ago, perhaps that was the output of fsl tools at the time. Feel free to change this to 134 as that is what I get with the current version of fslmaths (the FSL error is odd, the tools work fine with float 32, so the error really refers to the enumeration for DT_COMPLEX64 which is 32.

$ fslmaths complex.nii.gz -add 0 sfsf
Image Exception : #8 :: Fslread: DT 32 not supported
libc++abi: terminating due to uncaught exception of type std::runtime_error: Fslread: DT 32 not supported
/Users/chris/fsl/share/fsl/bin/fslmaths: line 2: 19588 Abort trap: 6 /Users/chris/fsl/bin/fslmaths "$@"
$ echo $?
134

And you actually need that exact error code? As I said in another comment, it's not ideal IMHO to call exit() in a library...

seanm · 2025-01-29T18:57:49Z

niftilib/nifti1_io.c

+   #ifdef REJECT_COMPLEX
+   if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) {
+      fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname);
+      exit(13);


niftilib/nifti1_io.c

nifti2/nifti2_io.c

CMakeLists.txt

seanm · 2025-01-29T19:04:40Z

nifti2/nifti2_io.c

@@ -7831,6 +7831,73 @@ znzFile nifti_image_write_hdr_img2(nifti_image *nim, int write_opts,
     if( imgfile ) *imgfile = fp;                                       \
     return 1 ; } while(0)

+#ifdef PIGZ
+#ifdef HAVE_ZLIB
+int doPigz2(nifti_image *nim, struct nifti_2_header nhdr, const nifti_brick_list * NBL) {


struct nifti_2_header is a pretty big struct, might be better to pass by reference, not value, no?

nifti2/nifti2_io.c

seanm · 2025-01-29T19:12:26Z

nifti2/Makefile

@@ -9,6 +9,12 @@ CC		= gcc
 IFLAGS          = -I. -I../niftilib -I../znzlib
 CFLAGS          = -Wall -std=gnu99 -pedantic $(USEZLIB) $(IFLAGS)

+
+#run "FSLSTYLE=1 make" for JPEGLS build


I don't see the JPEGLS connection...

seanm · 2025-01-29T19:16:51Z

nifti2/nifti2_io.c

+      strcpy(gzname, hdrname);
+      strcat(gzname,extzip);


strcpy and strcat should not be used in new code

seanm · 2025-01-29T19:20:40Z

nifti2/nifti2_io.c

+      if (nifti_fileexists(gzname)) {
+         fprintf(stderr,"Image Exception : Multiple possible filenames detected for basename (*.nii, *.nii.gz): %s\n", basename);
+         free(gzname);
+         exit(134);


Why 134?

Also, it doesn't seem appropriate for library code to call exit(), nowhere else in this file calls exit(). This should probably return NULL.

The compile directive FSLSTYLE emulates the beahavior of fsl tools, these terminate with these exit codes for these failures:

$fslmaths a add -0 b Image Exception : #61 :: Multiple possible filenames detected for basename: a libc++abi: terminating due to uncaught exception of type std::runtime_error: Multiple possible filenames detected for basename: a /Users/chris/fsl/share/fsl/bin/fslmaths: line 2: 20227 Abort trap: 6 /Users/chris/fsl/bin/fslmaths "$@" $ echo $? 134

So that's a SIGABRT. If we really need to emulate that, better to call abort().

But what would happen if we just return NULL? Could you try this: #11

hjmjohnson added 3 commits January 23, 2025 14:04

ENH: Address ambiguous situation when both .nii and .nii.gz files exist

04b1efb

emulate FSL when multiple ambiguous filenames exist (.nii/.nii.gz), Configure 'run "FSL=1 make"' and 'cmake -DFSLSTYLE=ON .' Make names consistent nifti_strdup() is too small to include an extension

ENH: Match FSL behaviors when PIGZ is defined

a6678ba

emulate AFNI pigz support Fixed conversion error when PIGZ is defined. Configure 'run "FSL=1 make"' and 'cmake -DFSLSTYLE=ON .'

ENH: Match FSL behavior of reject complex data

8047aee

Reject complex data like FSL does Configure 'run "FSL=1 make"' and 'cmake -DFSLSTYLE=ON .'

hjmjohnson force-pushed the nifti_image_conflicted_names branch from bc14651 to 8047aee Compare January 23, 2025 20:04

hjmjohnson self-assigned this Jan 23, 2025

hjmjohnson added the enhancement New feature or request label Jan 23, 2025

hjmjohnson mentioned this pull request Jan 23, 2025

Corrupted nifti image loading if same filename is used with different extensions in the same folder InsightSoftwareConsortium/ITK#2243

Open

hjmjohnson merged commit 9a0b246 into master Jan 25, 2025
1 check passed

hjmjohnson deleted the nifti_image_conflicted_names branch January 25, 2025 18:08

seanm reviewed Jan 29, 2025

View reviewed changes

seanm mentioned this pull request Feb 4, 2025

Optional file behavior NIFTI-Imaging/nifti_clib#138

Open

Nifti image conflicted names #9

Nifti image conflicted names #9

Uh oh!

Conversation

hjmjohnson commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hjmjohnson commented Jan 23, 2025

Uh oh!

hjmjohnson commented Jan 23, 2025

Uh oh!

gdevenyi commented Jan 24, 2025

Uh oh!

Uh oh!

seanm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanm Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hjmjohnson commented Jan 23, 2025 •

edited

Loading

seanm Feb 4, 2025 •

edited

Loading