[ENH] Proposition for adni_fmap renaming function

[AliceJoubert]

Refactor of adni_fmap.py following issue #1174 .

It mainly tackles the renaming of the files after using dcm2nix to convert the files. I chose the easy way ie relying on what dcm2nix outputs to rename the files but that can be discussed.

[NicolasGensollen]

We can discuss that next week, but I have some concerns with this approach.
I haven't had time to look closely at the PR, but looking at this: https://github.com/rordenlab/dcm2niix/blob/master/BidsGuess/README.md?plain=1, it seems like BidsGuess is quite recent and still experimental.

If I understand correctly, this means that users with a dcm2niix older than v1.0.20230731 (quite recent) will get an error at this point.

Also, I was a bit concerned by sections of the README linked above like "Therefore, it is unable to resolve fieldmap...". Might be outdated, but still a bit concerning...

I'm not saying that we shouldn't use this option, but I think we should have a backup way in case things go wrong or the user's version of dcm2niix is not recent enough.

WDYT ?

[AliceJoubert]

I only use 'BidsGuess' to determine what type of image I have (phase diff, magnitude, echo), without having to make the checks myself and from what I have read and tested it seems to work well !

However, I agree there should be another option if the field 'BidsGuess' does not appear in the .json files anyway. I will work on this next week, based on what was proposed by @tharpm95 and https://bids-specification.readthedocs.io/en/stable/modality-specific-files/magnetic-resonance-imaging-data.html#types-of-fieldmaps.

This way, both options will be available. What do you think ?

[likeajumprope]

Hi all, apologies for just jumping in - I have run the dev branch with the new fieldmaps conversion feature over three subjects. We had a short discussion about the "BidsGuess" in my team as well, but eventually we are planning to use the output of the converter for fmriprep and we would need an "IntendedFor" key. Would it be possible to add that?

[likeajumprope]

Second, I had to make some (small) adjustments to get the fieldmap conversion to run (some fieldmap file extensions were not included) - I might open a small PR --> #1177

[likeajumprope]

Third, when running the new dev branch over my three tests subjects, I am left with a tmp folder (clinica, terminates, no errors):

any clue what that means?

I am also happy to investigate, but just wanted to let you know.

[tharpm95]

Commenting here to confirm I've also had the tmp_dcm issue. I had the code to remove those automatically which I think we decided to remove during the merge. It could likely be reintegrated into the more robust for loop to remove these files, or added somewhere else in adni_utils.py

[NicolasGensollen]

Hi all,

The issue with the tmp_dcm folder which is not removed is caused by the function create_file (in adni_utils.py) returning early.

More precisely, the flag nifti_exists is very often set to False because the converted image does not have the expected name. This causes the function to warn and return early:

clinica/clinica/iotools/converters/adni_to_bids/adni_utils.py

Lines 1551 to 1556 in a4984bc

	if not nifti_exists or not dwi_bvec_and_bval_exist:
	cprint(
	msg=f"Conversion with dcm2niix failed for subject {subject} and session {session}",
	lvl="warning",
	)
	return np.nan

Such that the following line is not executed:

clinica/clinica/iotools/converters/adni_to_bids/adni_utils.py

Lines 1602 to 1603 in a4984bc

	# Check if there is still the folder tmp_dcm_folder and remove it
	remove_tmp_dmc_folder(bids_dir, image_id)

For example, the expected file

"sub-ADNI941S6471/ses-M024/fmap/sub-ADNI941S6471_ses-M024_fmap.nii.gz"

does not exist, but

"sub-ADNI941S6471/ses-M024/fmap/sub-ADNI941S6471_ses-M024_fmap_e2.nii.gz"

does.

A quick and dirty way around this, would be to modify the nifti_exists flag to take fmap specificities into account. Something like this maybe:

if modality == "fmap":
    nifti_exists = (
        nifti_exists
        or file_without_extension.with_name(f"{file_without_extension.name}_e2.nii.gz").is_file()
        or file_without_extension.with_name(f"{file_without_extension.name}_e2_ph.nii.gz").is_file()
    )

@likeajumprope the converter explicitly delete images that have "real" and "imaginary" suffixes added by dcm2niix:

clinica/clinica/iotools/converters/adni_to_bids/adni_utils.py

Lines 1517 to 1521 in a4984bc

	# Removing images with unsupported suffixes if generated by dcm2niix
	for suffix in ("ADC", "real", "imaginary"):
	file_with_bad_suffix = output_path / f"{output_filename}_{suffix}"
	if any(
	[

I believe those are not BIDS compliant, and will definitely break any pipeline that you'd run with clinica. Is there a reason you'd need those ? (my knowledge of field maps is quite limited, so I might be missing something here).