[FIX] ADNI-to-BIDS fmap conversion : delete real/imaginary files properly, enable converting to the same folder for fmap

[AliceJoubert]

This PR proposes small changes to allow :

• Removing files that were invalid (real/imaginary suffix after dcm2nix conversion) for all modalities
• Converting to the same folder with FMAP modality without deleting valid files
• Unit tests

[NicolasGensollen]

Thanks @AliceJoubert !
Just two very small suggestions but LGTM otherwise.

[AliceJoubert]

Thanks for the review @NicolasGensollen ! I modified the code as suggested and also added the same kind of logic to delete files with invalid suffixes. Do you see anything else that could be improved ?

[NicolasGensollen]

Thanks @AliceJoubert !
I think the PR looks good but it'd be awesome to add some unit tests covering the file filtering and removal logic. A possible approach that I'm suggesting here is to isolate the related code in one or two helper functions and add unit tests for those.
Let me know what you think !

[NicolasGensollen]

Thanks @AliceJoubert !
This is looking good ! Just a few small suggestions on the organization of these helper functions.

[AliceJoubert]

Thanks for the suggestions @NicolasGensollen ! I modified the code according to them. Do you see anything else ?

[NicolasGensollen]

Thanks @AliceJoubert !

I think this is almost ready. 👍
I have a small suggestion for the API of one of the helper functions (sorry for initially suggesting something different), as well as some minor suggestions to simplify the tests.

[NicolasGensollen]

Sorry, I know I asked for this, but re-looking at this, I think it would be more consistent to have this function return the list of files that were removed. This would offer the same API as _remove_files_with_unsupported_suffixes and the caller code would be pretty much the same.

So it'd be something like:

...
-> Iterable[Path]:
    from clinica.utils.stream import cprint

    if images_to_remove := _get_images_with_suffix(folder, suffixes):
        if mod_to_update:
            cprint(...)
            for path_to_unlink in images_to_remove:
                (path_to_unlink).unlink()
        else:
            cprint(f"There already exist images :...")
    return images_to_remove

[AliceJoubert]

No worries @NicolasGensollen ! Thanks for the comments.
The thing is, I can't use the output of that function by itself if I want to get out of the create_file function since it's totally possible for it to return either an empty or an actual list while we need the process to continue.
If we want to get the images_to_remove list we could call _get_images_with_suffix separately and keep a function returning a flag to know if we continue the process. WDYT ?

[NicolasGensollen]

Indeed, you're totally right !
Let's forget about this comment and keep things like this.
Sorry for the noise.

[NicolasGensollen]

Did you commit the suggestions on the tests ? I see the conversations resolved but not the commits and modifications.

[AliceJoubert]

My bad, I was waiting for your answer before committing the other changes. You should see them now @NicolasGensollen

[NicolasGensollen]

LGTM, thanks @AliceJoubert !