Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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] Update Figure Generation Code #236
[ENH] Update Figure Generation Code #236
Changes from 30 commits
4094323
dbd2fdb
52e98a4
6b1c71a
295ed20
95967e6
3c686e7
a0a9c4d
0111fb2
53f54dc
60cf82c
a2a8570
73e49f1
56021ed
1cb29be
ba5de7e
b75ed3a
91c3a73
1268ba4
a273301
d41ab96
20054a5
dd73665
484771b
969187e
7274be7
2a4d9ac
49b3153
01734d8
625a3ab
539a73e
a48c9a6
039e0e3
1dfb5bf
fe05678
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Should we use "variance explained" or "normalized variance explained" here and elsewhere?
@emdupre Thoughts?
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.
Thinking more on this, both of these sum up to be close to 100, even in cases where the PCs entered into in the ICA step explain <100% of the variance, right, because ICs are not orthogonal? I'm looking at an older run of the meica tedana, where the comp_table says 76.7% of the variance was explained, but summing the Variance and Variance norm columns gives me 100.03 and 96.78% respectively.
Means that
100 - sum(variance_expl)
that calculates the unexplained variance is never going to work quite right.Nor would calculating the variance explained (as is done in io.py, around line 202) and adding that in give the right number - because then the pie chart would be relative to the total (ie ~100 variance from ICA + unexplained = some_number >100). Perhaps some scaling would be needed for the values, prior to giving them to that part of figure creation.
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.
👍 for postponing the discussion until #223 !