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] Add mask argument to t2smap, tedana #108

Merged
merged 6 commits into from
Aug 8, 2018
Merged

Conversation

emdupre
Copy link
Member

@emdupre emdupre commented Aug 6, 2018

Closes #96.

Changes proposed in this pull request:

  • Allow for the input of a binary, spatially aligned mask in tedana and t2smap workflows

@codecov
Copy link

codecov bot commented Aug 6, 2018

Codecov Report

Merging #108 into master will increase coverage by 0.07%.
The diff coverage is 64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   45.36%   45.44%   +0.07%     
==========================================
  Files          28       28              
  Lines        1587     1602      +15     
==========================================
+ Hits          720      728       +8     
- Misses        867      874       +7
Impacted Files Coverage Δ
tedana/tests/test_utils.py 100% <100%> (ø) ⬆️
tedana/workflows/tedana.py 11.57% <16.66%> (-0.3%) ⬇️
tedana/workflows/t2smap.py 70% <60%> (-1.65%) ⬇️
tedana/utils/utils.py 83.33% <83.33%> (-0.52%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0ee1cc...907ad45. Read the comment docs.

@emdupre emdupre changed the title [WIP, ENH] Add mask argument to t2smap, tedana [ENH] Add mask argument to t2smap, tedana Aug 7, 2018
@emdupre emdupre requested a review from tsalo August 7, 2018 17:03
@@ -169,6 +169,9 @@ def make_adaptive_mask(data, minimum=True, getsum=False):
data : (S x E x T) array_like
Multi-echo data array, where `S` is samples, `E` is echos, and `T` is
time
mask : :obj:`str`, optional
Binary mask for voxels to consider in TE Dependent ANAlysis. Default is
to generate mask from data with good signal across echoes
Copy link
Member

Choose a reason for hiding this comment

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

This can be a string (filename), a numpy array, or an image, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now I'm only accepting a file name, since it's user-specified. Can you explain how it could be an image or numpy array ?

else:
# if the user has supplied a binary mask
mask = load_image(mask).astype(bool)
masksum = masksum * mask
Copy link
Member

Choose a reason for hiding this comment

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

Should there be shape/affine checks or would the ValueError numpy will raise when dimensions are incompatible be sufficient?

@emdupre
Copy link
Member Author

emdupre commented Aug 8, 2018

OK, merging ! 🎉

@emdupre emdupre merged commit 7ff7ae4 into ME-ICA:master Aug 8, 2018
@emdupre emdupre deleted the add_mask branch August 20, 2018 19:22
@jbteves jbteves added breaking change WIll make a non-trivial change to outputs and removed output-change labels Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change WIll make a non-trivial change to outputs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for an inputted mask
3 participants