Donders-Institute / bidscoin

BIDScoin converts your source-level neuroimaging data to BIDS
https://bidscoin.readthedocs.io
GNU General Public License v3.0
130 stars 35 forks source link

'vox' field does not get parsed from bidsmap #157

Closed schorschinho closed 2 years ago

schorschinho commented 2 years ago

Describe the bug I have added an entry in the vox field in my bidsmap:

    attributes:
      examination_name: ''
      scan_id: PRESS
      echo_time: '30'
      repetition_time: '2000'
      samples: '2048'
      averages: '48'
      ap_size: '30'
      lr_size: '26'
      cc_size: '26'
    bids:
      task:
      acq: press
      inv:
      nuc:
      vox: cso                                       ## <-------------- this field right here
      type:
      run: <<1>>
      suffix: svs

Unfortunately, vox doesn't seem to get picked up. All other fields are correctly parsed into the BIDS output filename. The vox entry doesn't show up in bidseditor and also does not appear in the output files' filenames generated by bidscoiner.

Screenshot 2022-10-12 150403

marcelzwiers commented 2 years ago

That's because vox is not a BIDS-compliant field? BIDScoin checks for that and makes sure to produce valid output

marcelzwiers commented 2 years ago

It should work (i.e. include the vox key-value pair) if you make it an extra_data datatype

schorschinho commented 2 years ago

Thanks a lot. I hadn't added the vox field manually, it was just there right in the bidsmap template, so I assumed it'd be BIDS-compliant and went ahead and populated it!

Hmm, I'd really like to be able to use the mrs datatype. The BIDS-MRS extension proposal has a bunch of MRS-specific labels:

sub-<label>[_ses-<label>][_task-<label>][_acq-<label>][_rec-<label>][_run-<index>][_echo-<index>][_inv-<index>][_nuc-<label>][_voi-<label>]_<suffix>.nii[.gz]

(From the extension proposal draft: https://docs.google.com/document/d/1pWCb02YNv5W-UZZja24fZrdXLm4X7knXMiZI7E2z7mY/edit#)

Is it possible to include these in the mrs datatype in bidscoin? That would be an immense step forwards. I think the MRS BEP is going to be submitted to the steering committee soon.

marcelzwiers commented 2 years ago

Ah, I see. Well, I think I tried to include as much from the BEPs as I could, but I don't do MRS myself -- which is why I didn't notice that BIDScoin's BIDS validation procedures stripped the vox field (as this field is not yet part of BIDS). I'll see if I can make the routines for this BIDS compliance optional

schorschinho commented 2 years ago

Fantastic. Thank you so much again for being so responsive and receptive.

marcelzwiers commented 2 years ago

Mhhh, I am now building the bids filename from the entities in this BIDS schema file: https://github.com/bids-standard/bids-specification/blob/master/src/schema/rules/entities.yaml Voxel (vox) is not in there (yet), which is the root of this issue. However, I intentionally designed it like this, so that even a faulty bidsmap still produces BIDS-valid output -- I considered that more important than the BEPs. But I think I can make it such that it allows strict BIDS compliance only, except for modalities that are in .bidsignore.

marcelzwiers commented 2 years ago

I have implemented it such that BIDS compliance is now only enforced on datatypes that are not in .bidsignore. I haven't fully tested things, but you can try it out yourself if you upgrade your install to the latest github commit:

pip install --upgrade git+https://github.com/Donders-Institute/bidscoin
schorschinho commented 2 years ago

Hi Marcel,

I upgraded my bidscoin install but vox is still not picked up, unfortunately. It's not displayed in the bidseditor, and no vox key-value label pair is added to the filename.

My source data is organized as follows:

raw
|-- sub-001
|   |-- ses-01
|   |   |-- CSO
|   |   |   |-- subjectname_PRESS_raw_act.sdat
|   |   |   |-- subjectname_PRESS_raw_act.spar
|   |   |   |-- subjectname_PRESS_raw_ref.sdat
|   |   |   |-- subjectname_PRESS_raw_ref.spar
|   |   |-- PCC
|   |   |   |-- subjectname_PRESS_raw_act.sdat
|   |   |   |-- subjectname_PRESS_raw_act.spar
|   |   |   |-- subjectname_PRESS_raw_ref.sdat
|   |   |   |-- subjectname_PRESS_raw_ref.spar

Here are the relevant attributes from the bidsmap:

    properties:
      filepath: .*CSO.*
      filename: .*raw_act.*
      filesize:
      nrfiles:
    attributes:
      examination_name: ''
      scan_id: PRESS
      echo_time: '30'
      repetition_time: '2000'
      samples: '2048'
      averages: '48'
      ap_size: '30'
      lr_size: '26'
      cc_size: '26'
    bids:
      task:
      acq: press
      inv:
      nuc:
      vox: cso
      type:
      run: <<1>>
      suffix: svs

I can successfully use the wildcards to map filenames containing the string raw_act to use the BIDS suffix svs and those containing the string raw_ref to use the suffix ref. But I still fail to do the same to make the distinction for filepaths containing CSO or PCC so they successfully map on the vox field being cso or pcc.

Appreciate any help :) Thanks, Georg

marcelzwiers commented 2 years ago

How does your bidsignore option of your bidsmap look like? Is mrs still in there (it should be because mrs is still a BEP)? And for the filepath properties, I am puzzled a bit, but could it be because the attributes are different (i.e. not all matching)? What happens if you just use properties and not attributes? Or use some wildcards in your attributes (or leave some fields empty)?

marcelzwiers commented 2 years ago

Btw, there is this known issue from pip: https://github.com/pypa/pip/issues/9397

If your mrs / bidsignore run-items are not orange now instead of red, this should force your upgrade:

pip install --upgrade --no-deps --force-reinstall git+https://github.com/Donders-Institute/bidscoin
schorschinho commented 2 years ago

YES that did it! Amazing - thank you!!!

marcelzwiers commented 2 years ago

Also the filepath issue? Then I'll close this issue and perhaps make a 3.7.4 release soon...

marcelzwiers commented 2 years ago

Btw, you are building a bidsmap template, here's a tip: try making the properties & attribute "filter" as broad as possible, i.e. don't filter on precise attributes like echo_time, but leave those empty. These will be automatically filled in your study bidsmap (i.e. after running the bidsmapper), so that you automatically get a narrow / study-specific set of filters for each study

schorschinho commented 2 years ago

Yes, I managed to set it up to run as intended:

Screen Shot 2022-10-17 at 16 51 34

Thanks again. Incredibly, incredibly helpful.

schorschinho commented 2 years ago

Thanks for the bidsmap building tip! In fact, I did an initial bidsmapper run (that filled most of the attributes from the spar headers) after which I refined the bidsmap manually with the properties. I'm still learning my ways how to fully leverage the regular expressions efficiently, but that'll come with practice.

marcelzwiers commented 2 years ago

Think of it like this: The bidsmapper does datatype discovery and uses the broad prior knowledge contained in the template bidsmap. The discovered datatypes (and their specific mapping to BIDS) are stored in the study template, which is used by the bidscoiner to do the actual conversion. A smart/good template can be used for all your studies (hopefully), and will make that your runs are already automatically classified correctly. If not, any misclassification needs to be manually corrected in the GUI of the bidseditor