ME-ICA / mapca

A Python implementation of the moving average principal components analysis methods from GIFT
GNU General Public License v2.0
6 stars 8 forks source link

[BUG] Dimensions of masked data not considered for fit_transform #16

Closed eurunuela closed 3 years ago

eurunuela commented 3 years ago

While working on the integration test, I saw there is a little bug in the code that probably comes from losing the context we had in tedana.

Basically, we are not considering that the data has been masked when giving shape3d to fit_transform(). We're currently giving the original dimensions when it should be the masked ones.

Edit: The error comes from the fact that apply_mask from nilearn only keeps the values inside the mask, while mapca in tedana keeps everything.

tsalo commented 3 years ago

Is this just an issue with the documentation? I noticed that I totally forgot to document the shape_3d and mask_vec parameters in the docstrings. Of course, we could re-evaluate those two arguments completely. They're hopefully just there until we solve #5, but if we can't get rid of the spatial elements then I think we can just pass around images (basically getting rid of the function and making the class behave that way).

eurunuela commented 3 years ago

I believe it's not a documentation issue. I'll open a draft PR so you can see what I mean.

eurunuela commented 3 years ago

This was fixed by changes to the core code in #19