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

Write out CIFTI time series and correlations as TSV files #559

Closed
wants to merge 65 commits into from

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Oct 4, 2022

Closes #555.

To do:

  • Update the file patterns to support the new files.
  • [ ] Determine what, if any, metadata we want to store in sidecar files. I'll probably deal with this in the provenance PR.
  • [ ] Remove header and index from conmat files, and move them to a _nodelabels file. Probably at the top level of the derivatives. Everything's easier with a header and index, so I'll leave it for now.
  • Update the outputs documentation.

Changes proposed in this pull request

  • Create a new function, xcp_d.utils.utils.extract_ptseries, that uses nibabel and pandas to extract the parcellated time series.
  • Create a new function, xcp_d.utils.fcon.compute_functional_connectivity, that uses pandas to perform the pair-wise Pearson correlations on the time series TSV file. This is used by both CIFTI and NIFTI workflows now.
  • Remove the now-unused CiftiCorrelation interface.
  • Change the time series and correlation outputs in the CIFTI workflow to be TSVs.
  • Split up the steps in the NiftiConnect interface, with the masking/time series extraction done in one function Node and the correlation done in another. Remove the now unused NiftiConnect interface.
  • Ensure that nifti time series and conmat files are structured the same way as cifti ones. I.e., currently time series have column headers and conmats have both column headers and index columns.

Documentation that should be reviewed

@tsalo
Copy link
Member Author

tsalo commented Oct 4, 2022

The TSV files will need to be modified a bit to be BIDS compliant. Especially the conmat file.

@tsalo tsalo added enhancement New feature or request breaking-change PRs that change results or interfaces. labels Oct 4, 2022
@tsalo
Copy link
Member Author

tsalo commented Oct 7, 2022

@kahinimehta WDYT about the conmat structure? Currently, I've got the index column of node names and the rest of the column names are node names as well. Based on BEP017, it seems like that's not preferred.

  • Do you think I should make a _nodelabels.tsv file?
  • If so, what metadata should go in the _nodelabels.json file?
  • If not, should I use the NodeLabels metadata field in the _conmat.json file?

docs/output.rst Outdated Show resolved Hide resolved
@kahinimehta
Copy link
Collaborator

kahinimehta commented Oct 7, 2022

Conmat structure looks good... I'd honestly be okay leaving the header and index in and having the atlas info kinda in a similar structure as it is now, but if we wanted to go by the BEPS, this is probably the BEP we are looking at for nodelabels.tsv and nodelabels.json https://docs.google.com/document/d/1RxW4cARr3-EiBEcXjLpSIVidvnUSHE7yJCUY91i5TfM/edit#heading=h.4k1noo90gelw . We could have the node labels and index under the nodelabels.tsv and then in the json, the source atlases and description of the .tsv if needed. What do you think?

@tsalo
Copy link
Member Author

tsalo commented Oct 7, 2022

I like that idea. The atlas BEP seems to have more detailed, and useful, recommendations for atlases than I thought.

Unfortunately, I needed to get the NIFTI-based timeseries and conmats into the same structure to get everything working. NIFTIs, of course, don't have the associated node names within the files, so I need to refactor things a bit to carry along that info throughout the process, which really requires standardized atlases. Basically, finishing up this PR will need to wait until we have the atlases standardized.

@tsalo tsalo added this to the 0.3.0 milestone Oct 19, 2022
@tsalo tsalo removed this from the 0.3.0 milestone Dec 19, 2022
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Base: 68.83% // Head: 69.74% // Increases project coverage by +0.91% 🎉

Coverage data is based on head (28d02c0) compared to base (4db3a25).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #559      +/-   ##
==========================================
+ Coverage   68.83%   69.74%   +0.91%     
==========================================
  Files          71       73       +2     
  Lines        6430     6552     +122     
==========================================
+ Hits         4426     4570     +144     
+ Misses       2004     1982      -22     
Impacted Files Coverage Δ
xcp_d/interfaces/connectivity.py 100.00% <ø> (ø)
xcp_d/interfaces/workbench.py 91.17% <ø> (-0.33%) ⬇️
xcp_d/tests/conftest.py 100.00% <ø> (ø)
xcp_d/tests/test_interfaces_prepostcleaning.py 69.09% <ø> (ø)
xcp_d/tests/test_utils_atlas.py 100.00% <ø> (ø)
xcp_d/tests/test_utils_fcon.py 100.00% <ø> (ø)
xcp_d/tests/test_workflow_connectivity.py 100.00% <ø> (ø)
xcp_d/utils/atlas.py 100.00% <ø> (+90.00%) ⬆️
xcp_d/utils/concatenation.py 86.73% <ø> (+2.26%) ⬆️
xcp_d/utils/fcon.py 92.45% <ø> (-7.55%) ⬇️
... and 16 more

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tsalo tsalo modified the milestones: 0.4.0, manuscript Jan 24, 2023
@tsalo
Copy link
Member Author

tsalo commented Jan 30, 2023

I will write out both TSVs and ptseries/pconn files, which means I'll need to redo a lot of the code here.

@tsalo
Copy link
Member Author

tsalo commented Feb 3, 2023

Closing in favor of #785.

@tsalo tsalo closed this Feb 3, 2023
@tsalo tsalo deleted the cifti-tsvs branch June 21, 2023 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change PRs that change results or interfaces. enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supplement parcellated time series and connectivity CIFTI files with TSVs
2 participants