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] Improve manual component selection #263

Merged
merged 3 commits into from
Apr 26, 2019
Merged

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Apr 20, 2019

Closes None. This was originally part of #247, but has been split off because it is self-contained and should be merged separately.

Changes proposed in this pull request:

  • Allow users to re-run tedana on the same directory without raising copyfile errors.
  • Retain original classification and rationale when manually overriding classification.
  • Allow manacc to be a list to make it easier to run within Python.
  • Clean up logic around checking mixm, ctab, and manacc arguments.

tsalo added 2 commits April 20, 2019 12:06
- Allow users to re-run tedana on the same directory without raising
copyfile errors.
- Retain original classification and rationale when manually overriding
classification.
- Allow manacc to be a list to make it easier to run within Python.
- Clean up logic around checking mixm, ctab, and manacc arguments.
@codecov
Copy link

codecov bot commented Apr 20, 2019

Codecov Report

Merging #263 into master will increase coverage by 0.09%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #263      +/-   ##
=========================================
+ Coverage   46.11%   46.2%   +0.09%     
=========================================
  Files          33      35       +2     
  Lines        2045    2058      +13     
=========================================
+ Hits          943     951       +8     
- Misses       1102    1107       +5
Impacted Files Coverage Δ
tedana/workflows/tedana.py 13.14% <0%> (-1.51%) ⬇️
tedana/selection/select_comps.py 4.82% <0%> (-0.21%) ⬇️
tedana/combine.py 88% <0%> (-0.24%) ⬇️
tedana/decomposition/__init__.py 100% <0%> (ø) ⬆️
tedana/selection/__init__.py 100% <0%> (ø) ⬆️
tedana/decomposition/eigendecomp.py
tedana/decomposition/pca.py 15.49% <0%> (ø)
tedana/decomposition/ica.py 26.92% <0%> (ø)
tedana/selection/tedpca.py 11.11% <0%> (ø)

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 c64ad6f...0a6889f. Read the comment docs.

@tsalo
Copy link
Member Author

tsalo commented Apr 20, 2019

I have tested the various combinations of ctab, manacc, and mix locally, and everything seems to work fine. I think this is ready for review.

emdupre
emdupre previously approved these changes Apr 26, 2019
Copy link
Member

@emdupre emdupre left a comment

Choose a reason for hiding this comment

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

This LGTM ! Wondering if we should generalize the I001 rationale to just "Manually classified" so the manually accepted components also have an accompanying rationale. But that's probably outside of the scope of this PR.

@tsalo
Copy link
Member Author

tsalo commented Apr 26, 2019

I think that's a good idea. I just made a commit with that change to the documentation, although the actual change to the manual classification will have to go in #266.

@tsalo tsalo merged commit a567410 into ME-ICA:master Apr 26, 2019
@tsalo tsalo deleted the improve-manacc branch April 27, 2019 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants