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

[fli-iam#1864] Add NIRS type BIDS #1865

Merged
merged 14 commits into from
Nov 24, 2023

Conversation

youennmerel
Copy link
Collaborator

NIRS BIDS files can now be imported into Shanoir

Specs : https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/11-near-infrared-spectroscopy.html

How to test :

  • Import NIRS modality BIDS into Shanoir through the BIDS import
  • Check the created examinations/acquisitions/datasets

Data samples :
https://osf.io/juxy8
https://github.com/bids-standard/bids-examples#nirs-datasets

@youennmerel youennmerel linked an issue Sep 19, 2023 that may be closed by this pull request
@youennmerel youennmerel self-assigned this Sep 19, 2023
@youennmerel youennmerel added REVIEWABLE Has to be reviewed and removed WorkInProgress labels Sep 20, 2023
@michaelkain
Copy link
Contributor

Hi, I think we could extend the import description to add NIRS [anat/func/dwi/eeg/ieeg/pet/..] on the web page to show, what modality we support.

@michaelkain
Copy link
Contributor

Hi, I think we might have to verify, if today's acquisition equipment fits to NIRS. So, when we create an unknown equipment, is it possible for somebody to enter the equipment info for NIRS OR do we risk to create unknown-unknown equipments? I know for MR, CT (not yet in BIDS) and PT we are fine, but I think there might be a gap with NIRS?

@youennmerel
Copy link
Collaborator Author

@michaelkain added 'Nirs' as BIDS data type in study card
image

@michaelkain
Copy link
Contributor

michaelkain commented Sep 21, 2023

Very good! Concering my comment on the equipment: I am right, that it remains empty for NIRS in the ds_acquisition? What is by the way totally fine for me. I just wanted to avoid dangling "unknown" equipments.

@youennmerel
Copy link
Collaborator Author

Added manufacturer model modality type 'Nirs'

@youennmerel
Copy link
Collaborator Author

Very good! Concering my comment on the equipment: I am right, that it remains empty for NIRS in the ds_acquisition? What is by the way totally fine for me. I just wanted to avoid dangling "unknown" equipments.

It will remain empty if none are found. However we now can also create 'Nirs' equipment in Shanoir.

@youennmerel
Copy link
Collaborator Author

Hi @michaelkain,

Should be OK now !

@michaelkain
Copy link
Contributor

Hi Youenn, thank you for your corrections. I downloaded one of the big NIRS examples above and did some tests, that showed some strange outputs:

  1. Good point: the import finishes with success: OK
  2. The import message in jobs has an error: [null (n°1)] Successfully created datasets for subject [DemoStudy_06] in examination [1] : the study name should not be null in the jobs message
  3. Solr download, as BIDS dataset: my subject folder sub-06 contains 34 files in my original BIDS structure: when I download all datasets imported, I have only 5 files, what should not be the case. The question is, how do we consider the datasets generated from BIDS. And when I download I have a full failures.txt and the file names in my zip file contain null_null values.
  4. the produced BIDS tree looks quite correct (study details), but contains a sub-2-DemoStudy06_scans.tsv file in Shanoir with 35 lines, but the original contains 36 lines on my disk (to correct)
  5. how do you map the files into the datasets in Shanoir? do you interpret the scans.tsv file, that contains 36 fnirs files?
  6. the other files in the tree are complete, and have the same naming

@michaelkain
Copy link
Contributor

michaelkain commented Sep 29, 2023

Hi, sorry, there is another issue:
Screenshot from 2023-09-29 11-15-24
the file names are wrong: they contain two times the subject
so you have to remove the old "sub-06" from the file name, as the subject has been imported into shanoir

@michaelkain
Copy link
Contributor

more issues:

  • the link to the created examination in shanoir is broken, from the created datasets
  • same for the created dataset acquisition
  • and no equipment is linked, neither created from the information of the BIDS structure

@jcomedouteau jcomedouteau removed the REVIEWABLE Has to be reviewed label Oct 11, 2023
Copy link
Contributor

@michaelkain michaelkain left a comment

Choose a reason for hiding this comment

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

Hi @youennmerel, please see my comments. Thank you!

@youennmerel youennmerel added REVIEWABLE Has to be reviewed and removed WorkInProgress labels Oct 24, 2023
@youennmerel
Copy link
Collaborator Author

Hi @michaelkain,

I took your comments into account, except for (as discussed in daily) :

  • Solr BIDS download : should be dealt with in another issue
  • scans.tsv content : in your example, the imported scans.tsv is incorrect, the generated one is.

@michaelkain
Copy link
Contributor

Hi @youennmerel, thank you! The 2 issues above are now fixed. I found two new ones, see my screenshots. I think with multiple sessions you just have to remove the trailing "-" from "ses-1-". I am wondering, as we have the acquisition time in the scans.tsv, if we could not use this as exam date, without time stamp maybe?
Screenshot from 2023-11-21 17-15-19

@michaelkain
Copy link
Contributor

and it is strange, that in the Shanoir tree, the dataset do not have the same name "subject name" contained as in the BIDS tree, but maybe this is normal...?
Screenshot from 2023-11-21 17-23-03
and I think the subject_name BIDS-test06 should be the same everywhere?

@michaelkain
Copy link
Contributor

michaelkain commented Nov 22, 2023

Hi @youennmerel, I hope you had a good start into your day. For the <label>s, see BIDS spec:
label - An alphanumeric value, possibly prefixed with arbitrary number of 0s for consistent indentation, for example, it is rest in task-rest following task- specification. Note that labels MUST not collide when casing is ignored (see Case collision intolerance).
sub

Full name: Subject
Format: sub-<label>
Definition: A person or animal participating in the study.
ses

Full name: Session
Format: ses-<label>

@youennmerel
Copy link
Collaborator Author

youennmerel commented Nov 23, 2023

Hi @michaelkain,

  • Fixed label : now remove non-alphanumeric characters in BIDS tree
  • Fixed acq_time parsing : now create examination with acq_time date

Copy link
Contributor

@michaelkain michaelkain left a comment

Choose a reason for hiding this comment

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

Nice job @youennmerel !!! Much better looking BIDS tree now, thank you!!!
Screenshot from 2023-11-24 10-58-45

@michaelkain michaelkain merged commit 052cb2f into fli-iam:develop Nov 24, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REVIEWABLE Has to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow upload of fNIRS BIDS
3 participants