Add load confounds

[harveyaa]

Hi Dan :)

Sorry for not being in touch for a minute, I implemented what we discussed in the issue this is a summary of where things are at:

• Integrated load_confounds
• Major changes:
  • Regressors are now numpy array not pandas dataframe
  • README.md examples use denoising_strategy Params6 instead of regressor_names
• To do:
  • Add denoising_strategy vs regressor_names explanation to README.md
  • Fix labels for plots
    • No longer provided by regressors dataframe
    • Replaced w/ numerical labels for now
  • Reports not working (even on master branch) need to investigate
  • Discuss changing behaviour in case where regressor_files specified and neither denoising_strategy nor regressor_names specified
    • Currently all confounds in file specified used
    • Would like to change to error message or make default strategy load_confounds Params6
      • If data is not fmri_prep preprocessed, load_confounds will throw an error

[danjgale]

Thanks for the update!

RE: Reports not working (even on master branch) need to investigate:
I'll look into this when I get a chance in the coming days. I've been using a local branch with some changes I'll merge after merging your PR, and the fix may already be in there.

[danjgale]

Okay, been doing some thinking.

We might be doing ourselves a disservice by juggling regressor_names and denoising_strategy. I've pulled your PR branch locally and tested out the idea of having everything under one parameter/flag: regressors. With regressors, you can enter either a) a list of column names in the regessor_files (exactly like regressor_names), b) a predefined strategy from load_confounds, or c) a list of flexible strategies compatible with load_confounds. So this puts everything in one spot rather than trying to split it up. If nothing is provided, then all regressors are used*

* this might be weird for fmriprep, given just how many confound regressors there are in the files. But people using custom preprocessing pipelines might have only the regressors they need in the regressor files. This is often the case in our lab when we aren't using fmriprep

[danjgale]

I can share my changes to your branch on here. Mind you, because of regressors, I ended up cutting it down quite a bit to integrate it nicely with the existing code. I also made some fixes that prevent downstream bugs in the reports (i.e. the regressors are always dataframes, even if determined by load_confounds, so that the column names can be use for the report).

I think this approach minimizes the changes we have to make to the codebase

[danjgale]

Okay so I made changes to cli.py and niimasker.py. See the commit comments for more details. Let me know what you think -- although they're commits we can always update or revert if needed. Seems to work nicely for my local version, but probably need to figure out how to test these changes. And, admittedly, the testing right now for niimasker is not very good (but I have lots of local examples that demonstrate that niimasker does what it intends to do haha).

[danjgale]

@harveyaa could you pull the current commits and then add load_confounds to the dependencies in setup.py? That seems to be where the CI is breaking

[danjgale]

Got to the bottom of the CI issues, see #27. @harveyaa could you pull in the latest version of master to your add_load_confounds branch? You might have to resolve a merge conflict with the load_confounds dependency (commit d3564b0)

[danjgale]

Excellent! Didn't know that I could resolve conflicts directly on here. So, nevermind my previous comment.

[danjgale]

Updated the README.md and example config file. All LGTM, will merge.