Skip to content
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

[ENH] Use images in MovingAveragePCA instead of arrays #32

Merged
merged 2 commits into from
Feb 24, 2021

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Feb 23, 2021

Closes #29. I do want to switch to using nilearn for masking and unmasking, but have come across a number of roadblocks with that. I'm still working on it in my reorg branch, but it's slow going.

Changes proposed:

  • Use images instead of arrays in MovingAveragePCA.fit(), .transform(), and .inverse_transform().
  • Check input data shapes with nilearn._utils.check_niimg_[3d|4d].
  • Return images instead of arrays.
  • Add new .u_nii_ attribute with img version of component maps.
  • Rename .u attribute to .u_ to follow convention.

@tsalo tsalo requested a review from eurunuela February 23, 2021 23:46
Copy link
Collaborator

@eurunuela eurunuela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @tsalo !

@eurunuela eurunuela added the enhancement New feature or request label Feb 24, 2021
@eurunuela eurunuela merged commit c098329 into ME-ICA:main Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make class accept images instead of arrays
2 participants