-
Notifications
You must be signed in to change notification settings - Fork 9
Add support for extraction of signals using user-supplied mask #38
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
base: main
Are you sure you want to change the base?
Conversation
I added a new function (nifti2arraywmask) that allows the use of the 3-D mask file to extract BOLD signals. I also modified the main data loader function (nifti2timeseries) to support its use - at least for single subject/single session data :)
pydfc/data_loader.py
Outdated
if mask_file is not None and n_rois is not None: | ||
print("Warning: specified mask_file will be used, ignoring n_rois.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if mask_file is not None and n_rois is not None: | |
print("Warning: specified mask_file will be used, ignoring n_rois.") | |
if mask_file is not None and n_rois is not None: | |
n_rois = None | |
warnings.warn("specified mask_file will be used, ignoring n_rois.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and maybe import warnings
pydfc/data_loader.py
Outdated
if mask_file is None and n_rois is None: | ||
print("Either mask_file or n_rois must be defined.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to raise an error here:
if mask_file is None and n_rois is None: | |
print("Either mask_file or n_rois must be defined.") | |
if mask_file is None and n_rois is None: | |
raise ValueError("Either mask_file or n_rois must be defined.") |
@m-miedema Thanks so much for implementing this new functionality! Overall it looks good. I have put a few suggestions. Also, since |
And regarding an example, I think that's a great idea. If we can have a separate example dedicated to use casing of your functionality, based on the script you already have, it'll be super useful for the users. |
@m-miedema , please take a look at the new changes I made to the |
The new additions look good to me! Would you prefer I create a separate demo notebook to add to the future examples directory? I think they will be a good place to highlight considerations for mask creation and to document several potential pitfalls that come with specifying a list of labels I've encountered while testing on my own masks and data. |
Creating a new notebook is a great idea @m-miedema ! I'll create a new directory for examples containing a copy of the current demos and then you can add your new notebook there! |
@m-miedema you can pull the new directory now |
So far this is a quick addition to the data_loader: I added a new function (nifti2arraywmask) that allows the use of the 3-D mask file to extract BOLD signals. I also modified the main data loader function (nifti2timeseries) to support its use - at least for single subject/single session data.
To-dos from here could include adding multi-session support, adding in an example of usage to the demos, and providing more guidance on creating a mask (I have a script I can contribute to that).