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

Rough code adding some support for manifest #18

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Shotgunosine
Copy link

No description provided.

mtnhuck and others added 20 commits January 12, 2018 17:09
scanner_software_versions_pd
get from SoftwareVersions
edited at line 165:
dict_append(image03_dict, 'scanner_software_versions_pd',
metadata.get("HardcopyDeviceSoftwareVersion", ""))
to
dict_append(image03_dict, 'scanner_software_versions_pd',
metadata.get("SoftwareVersions", ""))

image_slice_thickness SliceThickness
Inserted at line 170
dict_append(image03_dict, 'image_slice_thickness',
metadata.get("SliceThickness", ""))
Edited to include image_orientation

image_orientation: modified from
https://stackoverflow.com/questions/34782409/understanding-dicom-image-a
ttributes-to-get-axial-coronal-sagittal-cuts and
rordenlab/dcm2niix#153 (comment)

photomet_interpret: from DICOM/JSON field PhotometricInterpretation
Added a notes section talking about required experiment_id field
Updated for SliceThickness and PhotometricInterpretation
    There was a redundant if/else confusing things. I removed it.
    Also a value always has to be assigned to each element of
    image03dict the conversion to a dataframe fails
I think its the id listed for collection on the NDA site.
* commit 'bd3fb46fc8c1c3bb02abe07ecb0bb4c92df23d89':
  add FLASH to possible scantypes

# Conflicts:
#	bids2nda/main.py
@yarikoptic
Copy link
Collaborator

I am pushing more changes to #13 to get that one usable. I guess conflicts will pile up :-/

But I am not clear from the description of this PR what manifests we are talking about?

@Shotgunosine
Copy link
Author

Sorry for the lack of clarity. Manifests are the NDA's way of specifying an image file and some associated non-image files along with a directory structure in which to place them. In other words, manifests are going to be the best way to represent bids data structures in NDA datasets. There is not a lot of overhead to dealing with manifests, but there is some. Would it be better for me to submit this as a PR to mtnhuck:master or just wait for that PR to be closed, rebase my branch, and resubmit?

@yarikoptic
Copy link
Collaborator

What is there beast official source to read about them more?
You will just need to merge that branch I guess... Hopefully some feedback will come soon and we improve/merge it

@Shotgunosine
Copy link
Author

As far as I know, this is the best documentation for the Manifest feature: https://docs.google.com/document/d/1Qo3etvKgDx1lQH37lbKik5IPuLaif9-LHEaUVIBbiz4/edit#heading=h.wng6mfab9jp2
@ushnaahmad is there more documentation on the feature somewhere else?

@obenshaindw
Copy link
Collaborator

@yarikoptic and @Shotgunosine this was early documentation for piloting the feature while it was in development. We have some detail on the NDA site, which links out to https://github.com/NDAR/manifest-data. The GitHub repo is the most recent documentation and provides some helper scripts for creating manifests. @ushnaahmad has left NDA for a new opportunity with with GeneDx. You may also be interested in feedback from @ericearl as he is about to submit all ABCD Study imaging data in BIDS format sing NDA manifests.

@yarikoptic
Copy link
Collaborator

@Shotgunosine would you be interested to come back to this PR to facelift it and finalize? I can promise to be active too. ----experiment-id was also proposed in another PR #37 by @ljchang : so it is required only for functional and not anatomicals/dwi? (in #37 it is mandatory)

@Shotgunosine
Copy link
Author

@yarikoptic I haven't had to interact with NDA uploading for a few years now, so my memories of all of this are pretty dim. I don't really have the bandwidth to fully re-establish context on this, but I might be able to chip in on small stuff. @obenshaindw, I know you're not and NDA anymore, but do you know anyone there who might be willing to give us some guidance on this?

@ericearl
Copy link

experiment-id was also proposed in another PR #37 by @ljchang : so it is required only for functional and not anatomicals/dwi? (in #37 it is mandatory)

In both of these cases, experiment_id is "Conditional" on whether scan_type == 'fMRI':

  1. https://nda.nih.gov/data_structure.html?short_name=fmriresults01
  2. https://nda.nih.gov/data_structure.html?short_name=imagingcollection01

But I don't know all the cases.

@agt24
Copy link

agt24 commented Jul 15, 2023

@obenshaindw, I know you're not and NDA anymore, but do you know anyone there who might be willing to give us some guidance on this?

I'd suggest Greg Magdits and/or Karl Kuntzelman. But neither of them seem to be @-able.

@agt24
Copy link

agt24 commented Jul 21, 2023

I emailed Greg and Karl and got the following helpful info from Greg:

@ericearl 's answer regarding experiment-id’s is correct – they are only required at the moment for fmriresults and imagingcollection data if the associated scan_type is “fMRI”. Information about when specific data-elements need to be specified can be retrieved from NDA’s datadictionary api

https://nda.nih.gov/api/datadictionary/datastructure/fmriresults01
https://nda.nih.gov/api/datadictionary/datastructure/imagingcollection01

Our internal team sometimes make changes to the definitions of the data-structures in NDA so its possible that the condition expression for an element in a data structure ( in the case of experiment-id, “scan_type == ‘fMRI’ ”) to change from one submission cycle to the next.

@agt24
Copy link

agt24 commented Jul 21, 2023

Also, the Google Doc that @Shotgunosine linked above is dated 2018. The manifest documentation in the NDA github repo is a bit newer, but I'm not sure if it as comprehensive.

https://github.com/NDAR/manifest-data

@ericearl
Copy link

ericearl commented Aug 1, 2023

@yarikoptic Did you know this was a thing?

https://ndabids.readthedocs.io

It's a set of tools I helped create in the DCAN Labs to upload the ABCD-BIDS Community Collection (ABCC) data to NDA collection 3165. It relies on YAML and a little JSON to map files as-is to NDA data structures and upload in batches of records. I'm happy to answer any questions or pull other people into the conversation.

This was the manifest "code" used there:

https://github.com/DCAN-Labs/nda-bids-upload/blob/main/records.py#L259-L281

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.

7 participants