-
Notifications
You must be signed in to change notification settings - Fork 95
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
[FIX] Ignore non-significant kappa elbow when no non-significant kappa values exist #760
Conversation
Codecov Report
@@ Coverage Diff @@
## main #760 +/- ##
==========================================
+ Coverage 92.83% 92.91% +0.08%
==========================================
Files 27 27
Lines 2192 2203 +11
==========================================
+ Hits 2035 2047 +12
+ Misses 157 156 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @tsalo ! Thanks in particular for adding tests as well so that this doesn't regress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning message in the getelbow functions seems a bit more dramatic than necessary, but otherwise fine.
If you feel like changing it, I'd change to "This error happens when a getelbow function is incorrectly called on no components. If you see this message, please open an issue at https://github.com/ME-ICA/tedana/issues with the full traceback and any data necessary to reproduce this error, so that we create additional data checks to prevent this from happening."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closes #752. In the ICA decision tree, there is a step where two elbows are calculated: one from all kappa values and one from non-significant kappa values. When there are no non-significant kappa values, this step fails.
Changes proposed in this pull request:
getelbow
orgetelbow_cons
fail, raise a more informative exception and include in that message a request to open an issue here.getelbow
andgetelbow_cons
.