diff --git a/cortex/segment.py b/cortex/segment.py index 0981d313..f274b6ab 100644 --- a/cortex/segment.py +++ b/cortex/segment.py @@ -1,7 +1,6 @@ -"""Controls functions for segmentation of white/gray matter and other things in the brain. -""" +"""Controls functions for segmentation of white/gray matter and other things in the brain.""" + import os -import time import shlex import warnings import numpy as np @@ -18,19 +17,19 @@ from .freesurfer import autorecon as run_freesurfer_recon from .freesurfer import import_subj as import_freesurfer_subject -default_blender_path = options.config.get('dependency_paths', 'blender') -default_slim_path = options.config.get('dependency_paths', 'slim') +default_blender_path = options.config.get("dependency_paths", "blender") +default_slim_path = options.config.get("dependency_paths", "slim") def init_subject(subject, filenames, do_import_subject=False, **kwargs): """Run the first initial segmentation for a subject's anatomy (in Freesurfer). - This function creates a Freesurfer subject and runs autorecon-all, + This function creates a Freesurfer subject and runs autorecon-all, then (optionally) imports the subject into the pycortex database. - NOTE: This function requires a functional Freesurfer install! + NOTE: This function requires a functional Freesurfer install! Also, still can't handle T2 weighted anatomical volume input. Please use - Freesurfer directly (and then import) for advanced recon-all input + Freesurfer directly (and then import) for advanced recon-all input options; this is just a convenience function. Parameters @@ -44,18 +43,20 @@ def init_subject(subject, filenames, do_import_subject=False, **kwargs): file, etc. do_import_subject : bool Whether to import the Freesurfer-processed subject (without further) - editing) into pycortex. False by default, since we recommend editing - (or at least inspecting) the brain mask and white matter segmentations + editing) into pycortex. False by default, since we recommend editing + (or at least inspecting) the brain mask and white matter segmentations prior to importing into pycortex. kwargs : keyword arguments passed to cortex.freesurfer.autorecon() useful ones: parallel=True, n_cores=4 (or more, if you have them) """ - if 'run_all' in kwargs: - warnings.warn('`run_all` is deprecated - please use do_import_subject keyword arg instead!') - do_import_subject = kwargs.pop('run_all') + if "run_all" in kwargs: + warnings.warn( + "`run_all` is deprecated - please use do_import_subject keyword arg instead!" + ) + do_import_subject = kwargs.pop("run_all") if not isinstance(filenames, (list, tuple)): filenames = [filenames] - filenames = ' '.join(['-i %s'%f for f in filenames]) + filenames = " ".join(["-i %s" % f for f in filenames]) cmd = "recon-all {fname} -s {subj}".format(subj=subject, fname=filenames) print("Calling:\n%{}".format(cmd)) sp.call(shlex.split(cmd)) @@ -64,10 +65,12 @@ def init_subject(subject, filenames, do_import_subject=False, **kwargs): import_freesurfer_subject(subject) -def edit_segmentation(subject, - volumes=('aseg.mgz', 'brainmask.mgz', 'wm.mgz'), - surfaces=('lh.smoothwm', 'rh.smoothwm', 'lh.pial', 'rh.pial'), - freesurfer_subject_dir=None): +def edit_segmentation( + subject, + volumes=("aseg.mgz", "brainmask.mgz", "wm.mgz"), + surfaces=("lh.smoothwm", "rh.smoothwm", "lh.pial", "rh.pial"), + freesurfer_subject_dir=None, +): """Edit automatic segmentation results using freeview Opens an instance of freeview with relevant files loaded. @@ -89,45 +92,58 @@ def edit_segmentation(subject, """ if freesurfer_subject_dir is None: - freesurfer_subject_dir = os.environ['SUBJECTS_DIR'] - cmaps = {'brain': 'grayscale', - 'aseg': 'lut', - 'brainmask': 'gray', - 'wm': 'heat', - 'smoothwm': 'yellow', - 'white': 'green', - 'pial': 'blue' - } - opacity={'brain': 1.0, - 'aseg': 0.4, - 'brainmask': 1.0, - 'wm': 0.4, - } + freesurfer_subject_dir = os.environ["SUBJECTS_DIR"] + cmaps = { + "brain": "grayscale", + "aseg": "lut", + "brainmask": "gray", + "wm": "heat", + "smoothwm": "yellow", + "white": "green", + "pial": "blue", + } + opacity = { + "brain": 1.0, + "aseg": 0.4, + "brainmask": 1.0, + "wm": 0.4, + } vols = [] for v in volumes: - vpath = os.path.join(freesurfer_subject_dir, subject, 'mri', v) + vpath = os.path.join(freesurfer_subject_dir, subject, "mri", v) vv, _ = os.path.splitext(v) - vextra = ':colormap={cm}:opacity={op:0.2f}'.format(cm=cmaps[vv], op=opacity[vv]) + vextra = ":colormap={cm}:opacity={op:0.2f}".format(cm=cmaps[vv], op=opacity[vv]) vols.append(vpath + vextra) surfs = [] for s in surfaces: - spath = os.path.join(freesurfer_subject_dir, subject, 'surf', s) - _, ss = s.split('.') - sextra = ':edgecolor={col}'.format(col=cmaps[ss]) + spath = os.path.join(freesurfer_subject_dir, subject, "surf", s) + _, ss = s.split(".") + sextra = ":edgecolor={col}".format(col=cmaps[ss]) surfs.append(spath + sextra) - cmd = ["freeview", '-v'] + vols + ['-f'] + surfs - print("Calling: {}".format(' '.join(cmd))) + cmd = ["freeview", "-v"] + vols + ["-f"] + surfs + print("Calling: {}".format(" ".join(cmd))) sp.call(cmd) print("If you have edited the white matter surface, you should run:\n") - print(" `cortex.segment.run_freesurfer_recon('%s', 'wm')`\n"%subject) + print(" `cortex.segment.run_freesurfer_recon('%s', 'wm')`\n" % subject) print("If you have edited the brainmask (pial surface), you should run:\n") - print(" `cortex.segment.run_freesurfer_recon('%s', 'pia')`"%subject) - - -def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, - freesurfer_subject_dir=None, flatten_with='freesurfer', - method=None, do_import_subject=True, blender_path=default_blender_path, - recache=True, auto_overwrite=False, **kwargs): + print(" `cortex.segment.run_freesurfer_recon('%s', 'pia')`" % subject) + + +def cut_surface( + cx_subject, + hemi, + name="flatten", + fs_subject=None, + data=None, + freesurfer_subject_dir=None, + flatten_with="freesurfer", + method=None, + do_import_subject=True, + blender_path=default_blender_path, + recache=True, + auto_overwrite=False, + **kwargs, +): """Initializes an interface to cut the segmented surface for flatmapping. This function creates or opens a blend file in your filestore which allows surfaces to be cut along hand-defined seams. Blender will automatically @@ -136,7 +152,7 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, The surface will be automatically extracted from blender then run through the mris_flatten command in freesurfer. The flatmap will be imported once - that command finishes if `do_import_subject` is True (default value). + that command finishes if `do_import_subject` is True (default value). Parameters ---------- @@ -147,7 +163,7 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, name : str, optional String name of the current flatten attempt. Defaults to "flatten" data : Dataview or List(Dataview) - A data view object or list of data view objects to display on the + A data view object or list of data view objects to display on the surface as a cutting guide. fs_subject : str Name of Freesurfer subject (if different from pycortex subject) @@ -156,19 +172,19 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, Name of Freesurfer subject directory. None defaults to SUBJECTS_DIR environment variable flatten_with : str - One of 'freesurfer','SLIM', or 'blender' - 'freesurfer' (default) uses freesurfer's + One of 'freesurfer','SLIM', or 'blender' - 'freesurfer' (default) uses freesurfer's `mris_flatten` function to flatten the cut surface. 'SLIM' uses the SLIM algorithm, which takes much less time but tends to leave - more distortions in the flatmap. SLIM is an optional dependency, and - must be installed to work; clone the code - (https://github.com/MichaelRabinovich/Scalable-Locally-Injective-Mappings) - to your computer and set the slim dependency path in your pycortex config + more distortions in the flatmap. SLIM is an optional dependency, and + must be installed to work; clone the code + (https://github.com/MichaelRabinovich/Scalable-Locally-Injective-Mappings) + to your computer and set the slim dependency path in your pycortex config file to point to /ReweightedARAP method : str method to use for UV unwrap. When using Blender, it must be present and can be one of 'CONFORMAL', 'ANGLE_BASED', 'MINIMUM_STRETCH'. do_import_subject : bool - set option to automatically import flatmaps when both are completed + set option to automatically import flatmaps when both are completed (if set to false, you must import later with `cortex.freesurfer.import_flat()`) blender_path : str Path to blender executable. If None, defaults to path specified in pycortexconfig file. @@ -186,8 +202,12 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, # Double-check that fiducial and inflated vertex counts match # (these may not match if a subject is initially imported from freesurfer to pycortex, # and then edited further for a better segmentation and not re-imported) - ipt, ipoly, inrm = freesurfer.get_surf(fs_subject, hemi, "inflated") - fpt, fpoly, fnrm = freesurfer.get_surf(fs_subject, hemi, "fiducial") + ipt, ipoly, inrm = freesurfer.get_surf( + fs_subject, hemi, "inflated", freesurfer_subject_dir=freesurfer_subject_dir + ) + fpt, fpoly, fnrm = freesurfer.get_surf( + fs_subject, hemi, "fiducial", freesurfer_subject_dir=freesurfer_subject_dir + ) if ipt.shape[0] != fpt.shape[0]: raise ValueError( "Please re-import subject - fiducial and inflated vertex counts don't match!" @@ -203,19 +223,21 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, if not os.path.exists(fname) or recache: if os.path.exists(fname): os.remove(fname) - print("Initializing blender file %s..."%fname) - blender.fs_cut_init(fname, fs_subject, hemi, freesurfer_subject_dir, blender_path=blender_path) + print("Initializing blender file %s..." % fname) + blender.fs_cut_init( + fname, fs_subject, hemi, freesurfer_subject_dir, blender_path=blender_path + ) # Add localizer data to facilitate cutting if data is not None: data = data if isinstance(data, list) else [data] for d in data: blender.add_cutdata(fname, d, name=d.description, blender_path=blender_path) - + # Open blender for user to manually do the cuts - print("Opening blender file %s..."%fname) + print("Opening blender file %s..." % fname) blender.fs_cut_open(fname, blender_path=blender_path) - + # Generate 3D base freesurfer patch to flatten base_patch_path = freesurfer.get_paths( fs_subject, hemi, freesurfer_subject_dir=freesurfer_subject_dir @@ -223,15 +245,17 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, if not os.path.exists(base_patch_path) or recache: if os.path.exists(base_patch_path): os.remove(base_patch_path) - print("Writing base patch to %s..."%base_patch_path) - blender.write_volume_patch(fname, base_patch_path, hemi, blender_path=blender_path) + print("Writing base patch to %s..." % base_patch_path) + blender.write_volume_patch( + fname, base_patch_path, hemi, blender_path=blender_path + ) """ pts_v, polys_v, _ = freesurfer.get_surf(fs_subject, hemi, "patch", name, freesurfer_subject_dir=freesurfer_subject_dir) pts_f, polys_f, _ = freesurfer.get_surf(fs_subject, hemi, "patch", name+".flat", freesurfer_subject_dir=freesurfer_subject_dir) """ # Flatten - if flatten_with == 'blender': + if flatten_with == "blender": assert method is not None, "method must be provided when using blender" flat_patch_path = freesurfer.get_paths( @@ -240,19 +264,32 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, if os.path.exists(flat_patch_path): os.remove(flat_patch_path) - print("Generating flat patch via blender method %s to %s..."%(method, flat_patch_path)) - done = blender.write_flat_patch(fname, flat_patch_path, hemi, method=method, blender_path=blender_path) + print( + "Generating flat patch via blender method %s to %s..." + % (method, flat_patch_path) + ) + done = blender.write_flat_patch( + fname, flat_patch_path, hemi, method=method, blender_path=blender_path + ) path_type, flat_type = "patch", "blender" - elif flatten_with == 'freesurfer': + elif flatten_with == "freesurfer": print("Generating flat patch via freesurfer...") done = freesurfer.flatten( - fs_subject, hemi, patch=name, freesurfer_subject_dir=freesurfer_subject_dir, **kwargs + fs_subject, + hemi, + patch=name, + freesurfer_subject_dir=freesurfer_subject_dir, + **kwargs, ) path_type, flat_type = "patch", "freesurfer" - elif flatten_with == 'SLIM': + elif flatten_with == "SLIM": print("Generating flat patch via SLIM...") done = flatten_slim( - fs_subject, hemi, patch=name, freesurfer_subject_dir=freesurfer_subject_dir, **kwargs + fs_subject, + hemi, + patch=name, + freesurfer_subject_dir=freesurfer_subject_dir, + **kwargs, ) path_type, flat_type = "slip", "slim" else: @@ -262,7 +299,7 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, # (Do not attempt to import completed flatmaps) if not done: return - + # Import from freesurfer to pycortex DB if do_import_subject: hemi = "lh" if hemi == "rh" else "rh" @@ -281,8 +318,15 @@ def cut_surface(cx_subject, hemi, name='flatten', fs_subject=None, data=None, ) -def flatten_slim(subject, hemi, patch, n_iterations=20, freesurfer_subject_dir=None, - slim_path=default_slim_path, do_flatten=None): +def flatten_slim( + subject, + hemi, + patch, + n_iterations=20, + freesurfer_subject_dir=None, + slim_path=default_slim_path, + do_flatten=None, +): """Flatten brain w/ slim object flattening Parameters @@ -300,29 +344,44 @@ def flatten_slim(subject, hemi, patch, n_iterations=20, freesurfer_subject_dir=N slim_path : str path to SLIM flattening. Defaults to path specified in config file. """ - if slim_path == 'None': - slim_url = 'https://github.com/MichaelRabinovich/Scalable-Locally-Injective-Mappings' - raise ValueError("Please download SLIM ({slim_url}) and set the path to it in the `slim` field\n" - "in the `[dependency_paths]` section of your config file ({usercfg}) \n" - "if you wish to use slim!".format(slim_url=slim_url, usercfg=options.usercfg)) + if slim_path == "None": + slim_url = ( + "https://github.com/MichaelRabinovich/Scalable-Locally-Injective-Mappings" + ) + raise ValueError( + "Please download SLIM ({slim_url}) and set the path to it in the `slim` field\n" + "in the `[dependency_paths]` section of your config file ({usercfg}) \n" + "if you wish to use slim!".format( + slim_url=slim_url, usercfg=options.usercfg + ) + ) if do_flatten is None: - resp = input('Flattening with SLIM will take a few mins. Continue? (type y or n and press return)') - do_flatten = resp.lower() in ('y', 'yes') + resp = input( + "Flattening with SLIM will take a few mins. Continue? (type y or n and press return)" + ) + do_flatten = resp.lower() in ("y", "yes") if not do_flatten: print("Not flattening...") return # File paths if freesurfer_subject_dir is None: - freesurfer_subject_dir = os.environ['SUBJECTS_DIR'] - patchpath = freesurfer.get_paths(subject, hemi, - freesurfer_subject_dir=freesurfer_subject_dir) + freesurfer_subject_dir = os.environ["SUBJECTS_DIR"] + patchpath = freesurfer.get_paths( + subject, hemi, freesurfer_subject_dir=freesurfer_subject_dir + ) patchpath = patchpath.format(name=patch) - obj_in = patchpath.replace('.patch.3d', '.obj') - obj_out = obj_in.replace('.obj', '_slim.obj') + obj_in = patchpath.replace(".patch.3d", ".obj") + obj_out = obj_in.replace(".obj", "_slim.obj") # Load freesurfer surface exported from blender - pts, polys, _ = freesurfer.get_surf(subject, hemi, "patch", patch=patch, freesurfer_subject_dir=freesurfer_subject_dir) + pts, polys, _ = freesurfer.get_surf( + subject, + hemi, + "patch", + patch=patch, + freesurfer_subject_dir=freesurfer_subject_dir, + ) # Cull pts that are not in manifold pi = np.arange(len(pts)) pii = np.in1d(pi, polys.flatten()) @@ -331,14 +390,14 @@ def flatten_slim(subject, hemi, patch, n_iterations=20, freesurfer_subject_dir=N # Match indices in polys to new index for pts polys_new = np.vstack([np.searchsorted(idx, p) for p in polys.T]).T # save out obj file - print("Writing input to SLIM: %s"%obj_in) + print("Writing input to SLIM: %s" % obj_in) formats.write_obj(obj_in, pts_new, polys_new) # Call slim to write new obj file - print('Flattening with SLIM (will take a few minutes)...') + print("Flattening with SLIM (will take a few minutes)...") slim_cmd = [slim_path, obj_in, obj_out, str(n_iterations)] - print('Calling: {}'.format(' '.join(slim_cmd))) + print("Calling: {}".format(" ".join(slim_cmd))) out = sp.check_output(slim_cmd) - print("SLIM code wrote %s"%obj_out) + print("SLIM code wrote %s" % obj_out) # Load resulting obj file _, _, _, uv = formats.read_obj(obj_out, uv=True) uv = np.array(uv) @@ -348,8 +407,8 @@ def flatten_slim(subject, hemi, patch, n_iterations=20, freesurfer_subject_dir=N # fiducial brains. uv -= uv.min(0) uv /= uv.max() - uv -= (uv.max(0) / 2) - infl_scale = np.max(np.abs(pts_new.min(0)-pts_new.max(0))) + uv -= uv.max(0) / 2 + infl_scale = np.max(np.abs(pts_new.min(0) - pts_new.max(0))) # This is a magic number based on the approximate scale of the flatmap # (created by freesurfer) to the inflated map in a couple other subjects. # For two hemispheres in two other subjects, it ranged from 1.37 to 1.5. @@ -359,7 +418,7 @@ def flatten_slim(subject, hemi, patch, n_iterations=20, freesurfer_subject_dir=N # distortions, just the overall scale of the thing. So here we are. # ML 2018.07.05 extra_scale = 1.4 - uv *= (infl_scale * extra_scale) + uv *= infl_scale * extra_scale # put back polys, etc that were missing pts_flat = pts.copy() pts_flat[idx, :2] = uv @@ -369,19 +428,26 @@ def flatten_slim(subject, hemi, patch, n_iterations=20, freesurfer_subject_dir=N nz = pts_flat[:, 2] != 0 pts_flat[nz, 2] -= np.mean(pts_flat[nz, 2]) # Flip X axis for right hem (necessary?) - if hemi=='rh': + if hemi == "rh": # Flip Y axis upside down pts_flat[:, 1] = -pts_flat[:, 1] pts_flat[:, 0] = -pts_flat[:, 0] # Modify output .obj file to reflect flattening - #surfpath = os.path.join(freesurfer_subject_dir, subject, "surf", "flat_{hemi}.gii") - #fname = surfpath.format(hemi=hemi) - #print("Writing %s"%fname) - formats.write_obj(obj_out.replace('_slim','.flat_slim'), pts=pts_flat, polys=polys) + # surfpath = os.path.join(freesurfer_subject_dir, subject, "surf", "flat_{hemi}.gii") + # fname = surfpath.format(hemi=hemi) + # print("Writing %s"%fname) + formats.write_obj(obj_out.replace("_slim", ".flat_slim"), pts=pts_flat, polys=polys) return -def show_surface(subject, hemi, surface_type, patch=None, flatten_step=None, freesurfer_subject_dir=None): +def show_surface( + subject, + hemi, + surface_type, + patch=None, + flatten_step=None, + freesurfer_subject_dir=None, +): """ Parameters ---------- @@ -396,31 +462,36 @@ def show_surface(subject, hemi, surface_type, patch=None, flatten_step=None, fre name of patch, e.g. 'flatten.flat', 'flatten2.flat', etc """ - meshlab_path = options.config.get('dependency_paths', 'meshlab') - if meshlab_path == 'None': + meshlab_path = options.config.get("dependency_paths", "meshlab") + if meshlab_path == "None": try: # exists in system but not available in config - meshlab_path = sp.check_output('command -v meshlab', shell=True).strip() - warnings.warn('Using system meshlab: %s'%meshlab_path) + meshlab_path = sp.check_output("command -v meshlab", shell=True).strip() + warnings.warn("Using system meshlab: %s" % meshlab_path) except sp.CalledProcessError: - raise ValueError('You must have installed meshlab to call this function.') + raise ValueError("You must have installed meshlab to call this function.") if freesurfer_subject_dir is None: - freesurfer_subject_dir = os.environ['SUBJECTS_DIR'] - if surface_type in ('inflated', 'fiducial'): - input_type = 'surf' + freesurfer_subject_dir = os.environ["SUBJECTS_DIR"] + if surface_type in ("inflated", "fiducial"): + input_type = "surf" else: input_type = surface_type - fpath = freesurfer.get_paths(subject, hemi, input_type, - freesurfer_subject_dir=freesurfer_subject_dir) - - if not 'obj' in fpath: - pts, polys, curv = freesurfer.get_surf(subject, hemi, surface_type, - patch=patch, - flatten_step=flatten_step, - freesurfer_subject_dir=freesurfer_subject_dir) + fpath = freesurfer.get_paths( + subject, hemi, input_type, freesurfer_subject_dir=freesurfer_subject_dir + ) + + if "obj" not in fpath: + pts, polys, curv = freesurfer.get_surf( + subject, + hemi, + surface_type, + patch=patch, + flatten_step=flatten_step, + freesurfer_subject_dir=freesurfer_subject_dir, + ) # TODO: use tempfile library here - objf = '/tmp/temp_surf.obj' + objf = "/tmp/temp_surf.obj" formats.write_obj(objf, pts, polys) else: objf = fpath.format(name=patch) @@ -430,6 +501,7 @@ def show_surface(subject, hemi, surface_type, patch=None, flatten_step=None, fre ### DEPRECATED ### + def fix_wm(subject): """Initializes an interface to make white matter edits to the surface. This will open two windows -- a tkmedit window that makes the actual edits, @@ -449,13 +521,19 @@ def fix_wm(subject): subject : str Name of the subject to edit """ - warnings.warn("Deprecated! We recommend using edit_segmentation() and rerun_recon() instead of fix_wm() and fix_pia().") + warnings.warn( + "Deprecated! We recommend using edit_segmentation() and rerun_recon() instead of fix_wm() and fix_pia()." + ) status = _cycle_surf(subject, "smoothwm") - cmd = "tkmedit {subj} wm.mgz lh.smoothwm -aux brainmask.mgz -aux-surface rh.smoothwm" + cmd = ( + "tkmedit {subj} wm.mgz lh.smoothwm -aux brainmask.mgz -aux-surface rh.smoothwm" + ) sp.call(shlex.split(cmd.format(subj=subject))) status.value = 0 - resp = input("1) Run autorecon-wm?\n2) Run autorecon-cp?\n3) Do nothing?\n (Choose 1, 2, or 3)") + resp = input( + "1) Run autorecon-wm?\n2) Run autorecon-cp?\n3) Do nothing?\n (Choose 1, 2, or 3)" + ) if resp == "1": freesurfer.autorecon(subject, "wm") elif resp == "2": @@ -466,6 +544,7 @@ def fix_wm(subject): import_freesurfer_subject(subject) + def fix_pia(subject): """Initializes an interface to make pial surface edits. This function will open two windows -- a tkmedit window that makes the actual edits, @@ -485,13 +564,19 @@ def fix_pia(subject): subject : str Name of the subject to edit """ - warnings.warn("Deprecated! We recommend using edit_segmentation() and rerun_recon() instead of fix_wm() and fix_pia().") + warnings.warn( + "Deprecated! We recommend using edit_segmentation() and rerun_recon() instead of fix_wm() and fix_pia()." + ) status = _cycle_surf(subject, "pial") - cmd = "tkmedit {subj} brainmask.mgz lh.smoothwm -aux T1.mgz -aux-surface rh.smoothwm" + cmd = ( + "tkmedit {subj} brainmask.mgz lh.smoothwm -aux T1.mgz -aux-surface rh.smoothwm" + ) sp.call(shlex.split(cmd.format(subj=subject))) status.value = 0 - resp = input("1) Run autorecon-pia?\n2) Run autorecon-wm?\n3) Do nothing?\n (Choose 1, 2, or 3)") + resp = input( + "1) Run autorecon-pia?\n2) Run autorecon-wm?\n3) Do nothing?\n (Choose 1, 2, or 3)" + ) if resp == "1": freesurfer.autorecon(subject, "pia") elif resp == "2": @@ -503,19 +588,19 @@ def fix_pia(subject): import_freesurfer_subject(subject) - def _cycle_surf(subject, surf): - status = mp.Value('b', 1) + status = mp.Value("b", 1) + def cycle_surf(): - idx, hemis = 0, ['lh', 'rh'] + idx, hemis = 0, ["lh", "rh"] while status.value > 0: - hemi = hemis[idx%len(hemis)] + hemi = hemis[idx % len(hemis)] idx += 1 - #HELLISH CODE FOLLOWS, I heavily apologize for this awful code - #In order for this to work well, mayavi has to block until you close the window - #Unfortunately, with IPython's event hook, mlab.show does not block anymore - #There is no way to force mayavi to block, and hooking directly into backend vtk objects cause it to crash out - #Thus, the only safe way is to call python using subprocess + # HELLISH CODE FOLLOWS, I heavily apologize for this awful code + # In order for this to work well, mayavi has to block until you close the window + # Unfortunately, with IPython's event hook, mlab.show does not block anymore + # There is no way to force mayavi to block, and hooking directly into backend vtk objects cause it to crash out + # Thus, the only safe way is to call python using subprocess cmd = "python -m cortex.freesurfer {subj} {hemi} {surf}" sp.call(shlex.split(cmd.format(subj=subject, hemi=hemi, surf=surf))) diff --git a/cortex/tests/test_segment.py b/cortex/tests/test_segment.py new file mode 100644 index 00000000..7cf1e41e --- /dev/null +++ b/cortex/tests/test_segment.py @@ -0,0 +1,106 @@ +"""Tests for cortex.segment module, specifically for issue #591.""" + +import pytest +from unittest.mock import patch +import numpy as np + +from cortex import segment + + +class TestCutSurfaceFreesurferSubjectDir: + """Test cases for freesurfer_subject_dir parameter handling in cut_surface().""" + + @pytest.fixture + def mock_get_surf_return(self): + """Return value for mocked freesurfer.get_surf calls.""" + # Return dummy points, polygons, and normals with matching vertex counts + pts = np.zeros((100, 3)) + polys = np.zeros((50, 3), dtype=int) + norms = np.zeros((100, 3)) + return pts, polys, norms + + def test_freesurfer_subject_dir_passed_to_get_surf( + self, mock_get_surf_return, monkeypatch + ): + """Regression test for issue #591. + + Ensure freesurfer_subject_dir is passed to get_surf() calls. + Without the fix, this test would fail when SUBJECTS_DIR is not set. + """ + # Remove SUBJECTS_DIR to ensure test doesn't rely on environment + monkeypatch.delenv("SUBJECTS_DIR", raising=False) + + custom_subjects_dir = "/custom/freesurfer/subjects" + + with patch("cortex.segment.freesurfer.get_surf") as mock_get_surf: + mock_get_surf.return_value = mock_get_surf_return + + # Call cut_surface with explicit freesurfer_subject_dir + # We expect it to fail later (e.g., at db.get_paths), but we want to + # verify that get_surf is called correctly first + try: + segment.cut_surface( + cx_subject="test_subject", + hemi="lh", + freesurfer_subject_dir=custom_subjects_dir, + ) + except Exception: + # We expect failures after get_surf (e.g., database access) + pass + + # Verify get_surf was called twice (for inflated and fiducial) + assert mock_get_surf.call_count == 2 + + # Verify freesurfer_subject_dir was passed to both calls + for call in mock_get_surf.call_args_list: + assert call.kwargs.get("freesurfer_subject_dir") == custom_subjects_dir + + def test_raises_keyerror_without_subjects_dir(self, monkeypatch): + """Test that KeyError is raised when SUBJECTS_DIR is not set and no dir provided. + + This tests the expected behavior when neither SUBJECTS_DIR environment + variable nor freesurfer_subject_dir parameter is provided. + """ + # Remove SUBJECTS_DIR environment variable + monkeypatch.delenv("SUBJECTS_DIR", raising=False) + + # Without the freesurfer_subject_dir parameter and without SUBJECTS_DIR, + # the function should eventually raise a KeyError from freesurfer.get_surf + with pytest.raises(KeyError): + segment.cut_surface( + cx_subject="test_subject", + hemi="lh", + # freesurfer_subject_dir not provided + ) + + def test_backward_compatibility_with_subjects_dir_env( + self, mock_get_surf_return, monkeypatch + ): + """Test backward compatibility when SUBJECTS_DIR env var is set. + + When freesurfer_subject_dir is not provided, the function should still + work if SUBJECTS_DIR environment variable is set (None is passed to + get_surf which handles the fallback internally). + """ + # Set SUBJECTS_DIR environment variable + monkeypatch.setenv("SUBJECTS_DIR", "/env/freesurfer/subjects") + + with patch("cortex.segment.freesurfer.get_surf") as mock_get_surf: + mock_get_surf.return_value = mock_get_surf_return + + try: + segment.cut_surface( + cx_subject="test_subject", + hemi="lh", + # freesurfer_subject_dir not provided - should use env var via None + ) + except Exception: + # We expect failures after get_surf (e.g., database access) + pass + + # Verify get_surf was called twice + assert mock_get_surf.call_count == 2 + + # Verify freesurfer_subject_dir=None was passed (internal fallback) + for call in mock_get_surf.call_args_list: + assert call.kwargs.get("freesurfer_subject_dir") is None