nltools icon indicating copy to clipboard operation
nltools copied to clipboard

Refactor (and document) MNI template preferences?

Open ejolly opened this issue 7 years ago • 4 comments

@ljchang I'm realizing now that there might be a simpler way to handle different MNI templates, and one that allows different Brain_Data instances to use different spaces at the same time with clearer info about what's happening without the need for a prefs module.

The current usage requires importing a MNI_Template object (which is technically a dictionary) and then changing a key within, that affects all future Brain_Data instances that get created. This is useful for changing the package-wise default during a session, but is a bit clunky. I copied this style from how psychopy handles it's defaults (e.g. use a specific audio-device set at the stop of an experiment script).

The alternative is just to have the class take an argument as sketched below. Both approaches have pros and cons.

Current (package-wise approach):

  • Pro:
    • Nice because it's a "set it and forget it" approach that the user can do at the top of analysis script and be sure everything occurs in a specific space.
  • Con:
    • Not flexible if users want to work with different spaces within the same analysis session though; users will have to manually switch the preferences before a new instance is created and switching back-and-forth can be confusing.

New style:

  • Pro:
    • Each Brain_Data instance has it's own space attribute, which makes working in different spaces simultaneously seamless (though I'm not sure how often that will actually happen)
  • Cons:
    • Requires the user to remember to initialize each new Brain_Data instance with a non-default template argument if they are working in something other than 2mm which can get annoying when dealing with multiple Brain_Data images (the whole point of why it's implement as it is now)
    • Will probably need to make sure all class methods that check about different sampling resolutions (e.g. append, similarity) notify the user if the sampling resolution changes.

Sketch:

  1. Edit the Brain_Data __init__ method to take a template argument (which can default to 2mm).
  2. Have methods internally can refer to that attribute (e.g. plotting, etc)
  3. Add a .resample() method to the class that can convert between spaces. All it has to do is update the internal attribute and return a brand new Brain_Data instance, as the class constructor already handles resampling during the __init__

For example:

class Brain_Data(object):

def __init__(self,  
             data=None, 
             Y=None, 
             X=None,
             mask=None, 
             output_file=None, 
             template='2mm',
             **kwargs):
        
       ### Something like this gets added then referenced everywhere else ###
        self.template = template 
        
        if mask is not None:
            if not isinstance(mask, nib.Nifti1Image):
                if isinstance(mask, six.string_types):
                    if os.path.isfile(mask):
                        mask = nib.load(mask)
                else:
                    raise ValueError("mask is not a nibabel instance or a "
                                     "valid file name")
            self.mask = mask
        else:
            ### We still use the resolve_mni_path function ####
            ### but now with the class attribute ###
            
        self.mask = nib.load(resolve_mni_path(self.template)['mask'])
        self.nifti_masker = NiftiMasker(mask_img=self.mask)
.
.
.

# Here we can easily switch between spaces
# By initializing a new instance with all the previous
# attributes, except the new space we want
# The class _init_ will handle all the resampling internally

def resample(self,space):
        return Brain_Data(data=self.data,
                          Y=self.Y,
                          X=self.X,
                          mask=self.mask,
                          output_file=self.output_file,
                          template=space)

ejolly avatar Dec 18 '17 19:12 ejolly

this seems like it could be a good idea. I like the idea of keeping preferences self contained. Is there a way to check template size when loading file on init and then we can switch what the default anatomical is? This might create more problems then help.

ljchang avatar Feb 16 '18 02:02 ljchang

That's essentially what we're doing now. Right now each Brain_Data init grabs the current value of the nltools package-wide MNI_Template dictionary. It's convenient for setting it for a current working session of nltools, but not flexible in case you want each Brain_Data instance to have it's own template.

ejolly avatar Feb 16 '18 02:02 ejolly

We would also need to make sure that you can't merge different templates without resampling.

ljchang avatar Feb 16 '18 02:02 ljchang

@ejolly Need to write some docs for this

ejolly avatar Nov 01 '18 21:11 ejolly

Addressed in this commit: https://github.com/cosanlab/nltools/commit/ff353ca599e4b2a2964df378148d6c147305ec56

ejolly avatar Dec 15 '22 23:12 ejolly