-
Notifications
You must be signed in to change notification settings - Fork 26
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
Generate CIFTI and TSV versions of coverage, timeseries, and correlation files #785
Conversation
Should play nicely with PennLINC#782.
I needed to squeze the atlas_arr from 2D to 1D.
Codecov ReportBase: 71.24% // Head: 71.42% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #785 +/- ##
==========================================
+ Coverage 71.24% 71.42% +0.18%
==========================================
Files 73 74 +1
Lines 6544 6562 +18
==========================================
+ Hits 4662 4687 +25
+ Misses 1882 1875 -7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@mattcieslak I think this is finally ready to review. |
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.
a massive undertaking, looks good to me. Just minor comments/questions
) | ||
n_voxels_in_masked_parcels = sum_masker_masked.fit_transform(atlas_img_bin) | ||
n_voxels_in_parcels = sum_masker_unmasked.fit_transform(atlas_img_bin) | ||
parcel_coverage = np.squeeze(n_voxels_in_masked_parcels / n_voxels_in_parcels) | ||
coverage_thresholded = parcel_coverage < coverage_threshold | ||
|
||
n_nodes = coverage_thresholded.size | ||
n_nodes = len(node_labels) | ||
n_found_nodes = coverage_thresholded.size |
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.
I've never seen .size used this way before.
Looks like there's one parcel in the 1000-parcel atlas with ~0.5 coverage. Is getting NaNed out in the timeseries file, but checking the value in the coverage file resulted in a mismatch.
Going to merge. The file changes will be a drop in the bucket of the git history. |
Closes #555 and closes #777. Supercedes #559 and #783.
Changes proposed in this pull request
--cifti
flag, generate and write out tsv versions of the coverage, timeseries, and correlation files for each atlas.CiftiPrepareForParcellation
andCiftiCorrelation
interfaces.Documentation that should be reviewed