Donders-Institute / bidscoin

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

Append dcm2niix postfix to acq only if acq is not defined #214

Closed dzemanov closed 8 months ago

dzemanov commented 9 months ago

I suggest not appending to acq label dcm2niix postfix if user has defined the acq label in bidsmap, since that is the acq label user needs.

marcelzwiers commented 9 months ago

I'll look into this, but I'm sick now, then there is x-mas + new year, so I'm afraid it won't be quick :-(

marcelzwiers commented 9 months ago

So if not in the acq-label, where do you want to store the string that was appended by dcm2niix? You can't just throw it away, it contains meaningful information (such as coil number, echo number, etc)

dzemanov commented 8 months ago

No worries, thank you for the quick response!

I understand your concerns, you are right that the information is important.

The reason why I suggested this change is that we have some datasets that are concretely mapped with defined acq label and we would like to use that one, however, not handled dcm2niix postfixes are appended to it as well. I could create a script that just strips away appended dcm2niix postfixes, but I am not sure if I can correctly identify them with regex and not strip away by mistake part of our acq label. The script would then need to handle bidsname change regarding incrementing runindex and propagating changes to IntendedFor field, scans...

We have this problem with appended phMag postfix, which could be relatively easily stripped. I'm considering whether it's actually redundant, given that similar information might be available in the 'ImageType' attribute (specifically, the 'P' and 'M' indicators). However, I'm not entirely certain about this redundancy. Or if additional steps need to be made for the file to be BIDS valid, since I found only part-phase and part-mag BIDS examples.

Another postfix, _iN, presents a challenge. I think automatically removing it might lead to errors.

A potential solution could be to introduce a unique keyword in BIDScoin naming convention of acq label signaling the start of any dcm2niix-generated postfixes. This way, we can identify what part of acq label is our and what part is generated by dcm2niix.

An alternative approach could be to store dcm2niix postfixes in bids_mappings.tsv file from #213 if it gets merged. In this file, we could add a new column titled 'dicom2niix postfixes' where we list all the postfixes generated by dcm2niix per output file, so the information won't be lost.

For echo number, I thought this gets extracted to echo label, is there a case when echo is extracted to acq label?

I am relatively new to this, the old scripts we used for bids conversion just threw away iN image number or phMag, so files would get incremented run index if needed. Is there a defined way how to handle these cases?

marcelzwiers commented 8 months ago

How about making it an dcm2niix2bids option? For instance adding a field like: {fallback: acq}. You can leave that empty if needed

marcelzwiers commented 8 months ago

I made an effort to store unintentional dcm2niix suffixes in the appropriate BIDS entities, but if there isn't any (e.g. coil number), only then I store it in the acq label. But that should really be the last resort, if nothing else works

marcelzwiers commented 8 months ago

For the echo number, yes, certain sequences can produce multi echo data, but then there isn't always an echo entity in BIDS for it

dzemanov commented 8 months ago

How about making it an dcm2niix2bids option? For instance adding a field like: {fallback: acq}. You can leave that empty if needed

That would be amazing!

marcelzwiers commented 8 months ago

Mhh, our commits crossed each other, I just fixed it as well :-) See commit efcd7ac9c2b98c49d025d381d19a9b875fca1909

dzemanov commented 8 months ago

Yes, apologies, I didn't notice. Yours is much better! Thank you very much for looking into this.

marcelzwiers commented 8 months ago

Thanks for your PR, please re-open if it doesn't work (I didn't test it :-))