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

DOC: Example that explores BIDS and includes tractography from another pipeline. #577

Merged
merged 8 commits into from
Nov 24, 2020

Conversation

arokem
Copy link
Collaborator

@arokem arokem commented Oct 27, 2020

@jyeatman : this is the example that shows how to use tractography outputs from another pipeline

@arokem
Copy link
Collaborator Author

arokem commented Oct 27, 2020

I think there's something wonky with this trk file. I get:

In [5]: img = nib.load("/Users/arokem/AFQ_data/stanford_hardi/derivatives/vistas
   ...: oft/sub-01/ses-01/dwi/sub-01_ses-01_dwi.nii.gz")

In [6]: load_tractogram("/Users/arokem/AFQ_data/stanford_hardi/derivatives/my_tr
   ...: actography/sub-01/ses-01/dwi/sub-01_ses-01-dwi_tractography.trk", img)
WARNING:root:Dimensions must be above 0.
WARNING:root:Dimensions must be above 0.
WARNING:root:Dimensions must be above 0.
ERROR:root:Affine not equal
ERROR:root:Dimensions not equal
ERROR:root:Voxel_size not equal
ERROR:root:Trk file header does not match the provided reference.
Out[6]: False

@arokem
Copy link
Collaborator Author

arokem commented Oct 28, 2020

Thanks for the comments. I think they're addressed in the last couple of commits, but let me know if you think I should be more specific about anything.

@arokem
Copy link
Collaborator Author

arokem commented Oct 28, 2020

On a bit of further thought, I think that this example could be expanded a bit to explain how/why we use BIDS more generally. Hopefully, I'll find some time to work on that tomorrow. And also to fix the failing test.

# | ├── sub-01_ses-01_dwi.bvals
# | ├── sub-01_ses-01_dwi.bvecs
# | └── sub-01_ses-01_dwi.nii.gz

Copy link
Collaborator

Choose a reason for hiding this comment

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

May want to identify any files that are under bids that cannot be located, do not conform to standards, and do not follow convention. If pybids doesn't support this we may want to open an issue.

In lieu can:

  • run bids-validator
  • run bids_layout.get() and compare to an os.walk
  • run get_entities()

bids_path,
dmriprep='vistasoft',
bundle_info=bundle_info,
custom_tractography_bids_filters={
Copy link
Collaborator

Choose a reason for hiding this comment

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

May want to include primer on how to configure/diagnose bids_filters:

bids_layout = BIDSLayout(BIDS_DIR, derivatives=True)
# find all files
bids_layout.files
# get a specific file; probably want to specify full path
bids_layout.get()
# find out what entities are available for bids filters for file
bids_layout.get().get_entities()

Some issues have been dataset_description.json, file naming convention doesn't uniquely identify file with bids filters, ...

@pep8speaks
Copy link

pep8speaks commented Nov 2, 2020

Hello @arokem! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 1642:80: E501 line too long (80 > 79 characters)

Comment last updated at 2020-11-19 06:22:47 UTC

@arokem arokem changed the title DOC: Add example of using tractography from another pipeline. DOC: Example that explores BIDS and includes tractography from another pipeline. Nov 4, 2020
@arokem
Copy link
Collaborator Author

arokem commented Nov 19, 2020

Reading this and the comments again, I think that maybe this is enough for now? I think that a more detailed tutorial on BIDS/pyBIDS is out of scope for us, and users can get more information from the pybids documentation. So, I think this is ready for another review and potentially even merge.

@36000
Copy link
Collaborator

36000 commented Nov 24, 2020

LGTM!

@36000 36000 merged commit c653c71 into yeatmanlab:master Nov 24, 2020
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.

None yet

5 participants