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

[ENH] Improve the crop_nifti #1430

Merged
merged 6 commits into from
Feb 17, 2025

Conversation

NicolasGensollen
Copy link
Member

Following #1426, this PR proposes to improve the crop_nifti function.

Currently the function behaves in a very specific way, by cropping the provided image with a hardcoded bounding box that we defined for a specific use case (T1 images with MNI reference template). Therefore, I think the name is misleading and we could make use of a more generic function.

This PR proposes to make crop_nifti more generic by letting users provide both the bounding box and the reference image. If no bounding box is provided, then no cropping is performed. If no reference image is provided, the input image is used as a reference. In other words crop_nifti(img) with no other argument is the identity function.

The old crop_nifti has been renamed crop_nifti_using_t1_mni_template and is nothing more than a partial function on the new crop_nifti, calling it with the MNI_BBOX and the MNI cropped template.

Implementation details: I introduced two classes called NiftiImage and NiftiImage3D which behave as wrappers around a path to a nifti image. I'm still unsure whether we should keep them or not, but I think it could help us reduce boilerplate code we have in various places to check that a nifti is 3D, or that a nifti file exists, and so on.

@NicolasGensollen NicolasGensollen marked this pull request as ready for review February 14, 2025 13:03
@NicolasGensollen NicolasGensollen self-assigned this Feb 14, 2025
Copy link
Contributor

@AliceJoubert AliceJoubert left a comment

Choose a reason for hiding this comment

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

Thanks for the work @NicolasGensollen ! LGTM, I just have 2 comments about the NiftiImage class

clinica/utils/image.py Outdated Show resolved Hide resolved
clinica/utils/image.py Outdated Show resolved Hide resolved
@NicolasGensollen
Copy link
Member Author

Thanks for the review @AliceJoubert !
Let me know if I shall merge this or not.

@AliceJoubert
Copy link
Contributor

AliceJoubert commented Feb 17, 2025

Thanks for the review @AliceJoubert ! Let me know if I shall merge this or not.

Yes @NicolasGensollen it looks good to me now !

@NicolasGensollen NicolasGensollen merged commit c110adc into aramis-lab:dev Feb 17, 2025
12 checks passed
@NicolasGensollen NicolasGensollen deleted the improve-crop-nifti branch February 17, 2025 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants