Skip to content

ENH: Add mris_expand interface #1893

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

Merged
merged 6 commits into from
Mar 23, 2017
Merged

ENH: Add mris_expand interface #1893

merged 6 commits into from
Mar 23, 2017

Conversation

effigies
Copy link
Member

This utility is the Fischl-endorsed way to get a midthickness/graymid surface.

========
>>> from nipype.interfaces.freesurfer import MRIsExpand
>>> mris_expand = MRIsExpand(thickness=True, distance=0.5)
>>> mris_expand.inputs.in_file = 'lh.white'
Copy link
Member Author

Choose a reason for hiding this comment

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

Travis issue is exists=True. Not really clear why this should be causing an error while the Apas2Aseg (immediately above) interface is fine.

Copy link
Member

Choose a reason for hiding this comment

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

there is no lh.white dummy file in the testing directory, the above interface uses lh.pial

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, should have thought of that. I thought there was some voodoo in the test setup.

Copy link
Member

Choose a reason for hiding this comment

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

The only VooDoo is in our correlations.

@effigies effigies force-pushed the mris_expand branch 3 times, most recently from 4f1dc62 to 77f94a6 Compare March 21, 2017 13:44
@effigies
Copy link
Member Author

Was having issues with actually running until I realized it was dropping the output in the subject's surf directory by default. Updated out_file functionality to be more contained, and added all of the options.

There's one that went in post-6.0 release that I added in its own commit. Not sure how to set min_ver for a dev version. Maybe just leave it as something to add to args until it's in a numbered release?

@satra
Copy link
Member

satra commented Mar 21, 2017

@effigies - we need to augment min_ver and max_ver to allow for ><= combinations

regarding things leaving it in the surf directory, if that's the default behavior of the underlying interface, then that should be retained at the interface level. however the node(interface) should always leave things inside the node's working directory. generally we use copyfile to achieve this, but some interfaces also require recreating directory structures locally.

@effigies
Copy link
Member Author

Augmenting min_ver andmax_ver is a bit beyond the scope here; I'll just comment that out for now, and we can revisit once *_ver are modified.

The directory structure isn't needed, but in this case, if we copy the surface file, we also need to copy the associated ?h.thickness and ?h.pial files. I think I'll need to override _get_filecopy_info.

@effigies effigies force-pushed the mris_expand branch 2 times, most recently from 70d8c2a to 608278f Compare March 21, 2017 15:17
@effigies
Copy link
Member Author

@satra Would appreciate a look at the latest commit. The goal is to support file names (or specifiers, really) as mris_expand takes them, which required a little wrangling that might be seen as abuse of the input spec.

Mimics the FreeSurfer MRIsBuildFileName to select appropriate files
If copying files, defines pial and thickness_name (as needed) to ensure
copy.

Adds a non-argument sphere trait that MUST NOT be changed. mris_expand
will not run in a node if there is not a `?h.sphere` associated with the
in_file.
@satra satra merged commit 8c4e602 into nipy:master Mar 23, 2017
@effigies effigies deleted the mris_expand branch March 23, 2017 13:44
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.

3 participants