Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Properly deleting real and imaginary files for FMAP in adni-to-bids converter. #1188

Merged

Conversation

AliceJoubert
Copy link
Contributor

@AliceJoubert AliceJoubert commented May 22, 2024

The adni-to-bids converter does not remove the "imaginary" and "real" files in case the modality is FMAP because of the way the files are named after dcm2nix conversion. This PR proposes a quick fix and resolves PR #1177 .

For testing, the subject 002_S_1261 from ADNI can be used.

@AliceJoubert AliceJoubert added the fix PR fixing a bug label May 22, 2024
@AliceJoubert AliceJoubert self-assigned this May 22, 2024
@AliceJoubert
Copy link
Contributor Author

AliceJoubert commented May 22, 2024

This might be revamped later : in the case of the FMAP modality if there is at least one wrong image (containing "eX_imaginary" or "eX_real" or ".eX") all the files of the session can be deleted.

WDYT @NicolasGensollen ?

@NicolasGensollen
Copy link
Member

This might be revamped later : in the case of the FMAP modality if there is at least one wrong image (containing "eX_imaginary" or "eX_real" or ".eX") all the files of the session can be deleted.

WDYT @NicolasGensollen ?

I agree, if there is at least one image file (magnitude or phase diff) with a problematic conversion (as the "real" or "imaginary" suffixes would indicate) we need to remove all of them for this specific conversion, and give a warning to the user.

We might want to extract this piece of logic in a separate function and implement some unit tests to make sure file removal is happening as expected. But, let's target this in a different PR.

Copy link
Member

@NicolasGensollen NicolasGensollen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @AliceJoubert !

@NicolasGensollen NicolasGensollen merged commit b77a8af into aramis-lab:dev May 22, 2024
15 of 16 checks passed
@AliceJoubert AliceJoubert deleted the delete_complex_files_adni-utils branch May 22, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix PR fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants