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.

Sign up for GitHub

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

Jump to bottom

[ENH] Add PETsurfer to nipype #3437

Merged
merged 114 commits into from
Apr 21, 2022
Merged

[ENH] Add PETsurfer to nipype #3437

merged 114 commits into from
Apr 21, 2022

Conversation

mnoergaard
Copy link
Contributor

@mnoergaard mnoergaard commented Mar 11, 2022 •
edited
Loading

Summary

This PR adds PETsurfer to nipype (Issue #3422) by providing support for two FreeSurfer functions, namely gtmseg and mri_gtmpvc. It also provides support for pharmacokinetic modeling, which is supported by mri_glmfit. A tutorial for running these commands can be found here (https://surfer.nmr.mgh.harvard.edu/fswiki/PetSurfer).
All the necessary code has been added to petsurfer.py, including minor edits in the freesurfer interface init file and model.py.

  • Added support for PETsurfer via petsurfer.py
  • Updated init.py to import the classes GTMSeg and GTMPVC
  • Updated model.py to include flags for mrtm1, mrtm2 and logan in the GLMFitInputSpec

Acknowledgment

I acknowledge that this contribution will be available under the Apache 2 license.

mnoergaard and others added 30 commits August 10, 2021 13:45
@mnoergaard
Add mri_gtmseg to freesurfer preprocess module
bd0a2f8
@mnoergaard
Update mri_glmfit to include pharmacokinetic models
204ba98
@mnoergaard
Add petsurfer module
dbbe6dc
@mnoergaard
Add arguments to petsurfer.py
0d06893
@mnoergaard
Update petsurfer.py
4df2bf6
@mnoergaard
Update petsurfer.py
552d6b9
@mnoergaard
Update petsurfer.py
7fc4887
@mnoergaard
Update petsurfer.py
bf37e28
@mnoergaard
Update __init__.py
f6e58ed
@mnoergaard
Update petsurfer.py
8c9b00e
@mnoergaard
Update petsurfer.py
32ce6ec
@mnoergaard
Update petsurfer.py
566c4c6
@mnoergaard
Update petsurfer.py
06d2279
@mnoergaard
minor updates to petsurfer.py
0e16755 
CHANGES:
1. '-seg' changed to '--seg' in the input specs for GTMPVC
2. removed xor from registration flag
@mnoergaard
Update docstring for mri_gtmpvc
4374348
@mnoergaard
Add output specs for mri_gtmpvc
2c56ae7
@mnoergaard
Update logan function to correct output (bp)
f046d55
@mnoergaard
Update petsurfer.py
cd0afae
@mnoergaard
Update petsurfer.py
d52ac5c
@avneetreen
update petsurfer.py
0755948
@avneetreen
update petsurfer.py
03656b3
@mnoergaard
Merge pull request #1 from avneet14027/add_pet_freesurfer
08f9c7c 
merge avneets branch into martin's branch (update add_pet_freesurfer)
@mnoergaard
Add save-yhat flag to MRTM2 class
5be8307
@mnoergaard
Remove gtmseg part from preprocess.py
d8b90eb
@mnoergaard
Update mrtm2 and logan spec
e957416
@mnoergaard
Add k2prime output to MRTM class
603c3ef
@mnoergaard
Add documentation for LoganRef class
51fafbf
@mnoergaard
Update documentation LoganRef class
ea8e89d
os.join --> os.path.join
2c0e384
@mnoergaard
Merge branch 'add_pet_freesurfer' into add_pet_freesurfer
b9039d6
effigies
effigies reviewed Apr 19, 2022
View reviewed changes
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Okay, I think I've now gone over everything closely. Some final comments and suggestions. As always, feel free to push back or ask questions.

nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
mnoergaard and others added 15 commits April 18, 2022 19:00
@mnoergaard @effigies
FIX: rename ctab to colortable
2630b76 
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@mnoergaard
FIX: remove gca_color_table from _ctab_inputs
0aa1801
@mnoergaard
FIX: rename segpvres to tissue_fraction_resolution
30a893e
@mnoergaard @effigies
FIX: update description for rbv to include region-based voxelwise PVC…
4f2f60b 
… correction

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@mnoergaard @effigies
FIX: update gm flag to include correct input parameters
425469e 
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@mnoergaard @effigies
FIX: rename ss to steady_state_params
1f4e941 
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@mnoergaard @effigies
FIX: change name and trait for opt to enumeration
51e5a79 
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@mnoergaard
FIX: update _format_arg to accommodate optimization string and output…
5ecbbec 
… integer
@mnoergaard
FIX: fix tests for GTMPVC
0c0157f
@mnoergaard
FIX: fix spelling in mg trait GTMPVC
7fd8ade
@effigies
STY: black [ignore-rev]
b9716eb
@effigies
FIX: Update mapped values for optimization_schema
ce20aa7
@effigies
TEST/FIX: Implement argstr handling for mri_gtmpvc --mg
b91153f
@effigies
STY: Customary to mark all options in a mandatory mutex group as mand…
acea195 
…atory
@effigies
TEST: make specs
26a5349
@effigies
Copy link
Member

@mnoergaard Did you have any further people you wanted to look over this?

@pwighton @mgxd Would you like time to review before merge?

@mnoergaard
Copy link
Contributor Author

Tagging @melanieganz just to be sure, and also to potentially refer others. But otherwise everything LGTM. Thanks for all your help on this @effigies.

mgxd
mgxd approved these changes Apr 19, 2022
View reviewed changes
Copy link
Member

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

LGTM, one little thing

nipype/interfaces/freesurfer/petsurfer.py Outdated Show resolved Hide resolved
@mnoergaard @mgxd
FIX: update opt_params path
06e1193 
Co-authored-by: Mathias Goncalves <goncalves.mathias@gmail.com>
@mnoergaard
Copy link
Contributor Author

Thank you for your corrections/reviewing @mgxd!

@ghisvail
Copy link
Contributor

That would help us for our PET pipelines in Clinica 👍

Is there a plan for a point release of nipype soon after this is merged? Just to know whether we should wait for the update or vendor this interface within Clinica in the meantime.

@effigies
Copy link
Member

@ghisvail I almost always vendor new interfaces in projects I run so that I'm not dependent on the nipype release process. Once they're in and I've tested the latest release, I can transition away.

That said, I'm happy to merge this now, and we can plan to make a release soon. I want to give other contributors a chance to polish off PRs if they're ready to do so, so I expect it will be next week.

@effigies effigies merged commit 1a476bd into nipy:master Apr 21, 2022
@ghisvail
Copy link
Contributor

@ghisvail I almost always vendor new interfaces in projects I run so that I'm not dependent on the nipype release process. Once they're in and I've tested the latest release, I can transition away.

That's what we did for the recent mrtrix3 merge actually :-)

That said, I'm happy to merge this now, and we can plan to make a release soon. I want to give other contributors a chance to polish off PRs if they're ready to do so, so I expect it will be next week.

A week sounds good to us. Thanks for your quick reply.

@effigies effigies mentioned this pull request Apr 27, 2022
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgxd mgxd mgxd approved these changes

@effigies effigies effigies approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.8.0
Development

Successfully merging this pull request may close these issues.
6 participants
@mnoergaard @effigies @pwighton @ghisvail @mgxd @avneetreen