search

Donders-Institute / bidscoin

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

Add CSA header support #220

Closed dzemanov closed 9 months ago

marcelzwiers commented 9 months ago

Mhhh, this comment from the main author doesn't sound good:

Different parts of dcmstack and dicom_parser (both abandoned)

https://github.com/nipy/nibabel/issues/977#issuecomment-1704104818

marcelzwiers commented 9 months ago

FYI

marcelzwiers commented 9 months ago

There are two CSA headers, i.e.:

Now only CSASeriesHeaderInfo is used, but I don't know if it's worth it to also parse CSAImageHeaderInfo

marcelzwiers commented 9 months ago

Unfortunately the CSA header also supports indexing, e.g.:

asCoilSelectMeas[0].tNucleus             = ""1H""
asCoilSelectMeas[0].iUsedRFactor         = 3
asCoilSelectMeas[0].asList[0].sCoilElementID.tCoilID = ""HeadMatrix""
asCoilSelectMeas[0].asList[0].sCoilElementID.lCoilCopy = 1
asCoilSelectMeas[0].asList[0].sCoilElementID.tElement = ""H3P""
asCoilSelectMeas[0].asList[0].lElementSelected = 1
asCoilSelectMeas[0].asList[0].lRxChannelConnected = 1
asCoilSelectMeas[0].asList[1].sCoilElementID.tCoilID = ""HeadMatrix""
asCoilSelectMeas[0].asList[1].sCoilElementID.lCoilCopy = 1
marcelzwiers commented 9 months ago

Alternatively, instead of dicom-parser we can also use nibabel: https://github.com/nipy/nibabel/blob/master/nibabel/nicom/csareader.py

I don't know what would be the best choice

dzemanov commented 9 months ago

There are two CSA headers, i.e.:

  • (0029, 1010) - CSA Image Header Info
  • (0029, 1020) - CSA Series Header Info

Now only CSASeriesHeaderInfo is used, but I don't know if it's worth it to also parse CSAImageHeaderInfo

I asked colleague about this and his response was that most of the "interesting" info is in CSA Series Header Info. He says he currently doesn't know about anything interesting in CSA Image Header Info. However, he says that if parsing is the same and it is easy to include both, then why not. For us, CSA Series Header Info includes all information we need.

marcelzwiers commented 9 months ago

I'm already parsing both now, and have also added support for nibabel. The test needs to be improved, currently dicom-parser and nibabel.csareader give different outputs

dzemanov commented 9 months ago

For the tags we need, dicom_parser gets them correctly and nibabel returns ''. I checked the parsed csa headers and it seems nibabel doesn't parse the part with interesting data:

   ### ASCCONV BEGIN
  ... 
   ### ASCCONV END

Differences: nibabel includes values of "syngodt", "n_items" and "last3". The value is under key "items": [value] dicom_parser has tag_no under "index" and value under key "value": whatever is needed to represent what is saved there, so can be string, array, dict. I checked top level keys and those are the same for both. Afterwards nib needs "items" and dicom_parser "value".

From what I can tell, the values (under "items" or "value") are the same except for the ASCCONV part under MrPhoenixProtocol. Difference is also when there is no value: nibabel has empty [], dicom_parser has "value": None.

dzemanov commented 9 months ago

Mhhh, this comment from the main author doesn't sound good:

Different parts of dcmstack and dicom_parser (both abandoned)

nipy/nibabel#977 (comment)

Yes, that is unfortunate regarding not finished separate csa_header package :(. I tried it and it worked correctly, but author probably wants to do more testing.

marcelzwiers commented 9 months ago

So inside the CSA header, we have to deal with "raw" DICOM fields (with that clumsy index/items structure) as well as MrPhoenixProtocol fields (which have a more straightforward structure). I don't see an easy way to handle this

dzemanov commented 9 months ago

I probably don't understand. Dicom_parser works correctly even for MrPhoenixProtocol fields. The code you wrote works and we are able to read CSA header values under "items" or "value" thanks to it :). I tried it with our data and the code mapped items correctly, also using CSA header info. For nibabel, it reads everything correctly except for MrPhoenixProtocol.

marcelzwiers commented 9 months ago

Well, it's probably me who doesn't understand the CSA header very well. I get:

>>> get_dicomfield('MrPhoenixProtocol.tProtocolName', dcmfile)
CSASeriesHeaderInfo tag: MrPhoenixProtocol
CSASeriesHeaderInfo tag: tProtocolName
CSAImageHeaderInfo tag: MrPhoenixProtocol
''

But if I read it directly I get:

>>> value = Image(dcm_file_csa).header.get('CSASeriesHeaderInfo')
>>> value['MrPhoenixProtocol']['value']['tProtocolName']
'CBU+AF8-DTI+AF8-64D+AF8-1A'

I don't have time now to look into it, but it seems to me things are not working as intended

dzemanov commented 9 months ago

Well, it's probably me who doesn't understand the CSA header very well. I get:

>>> get_dicomfield('MrPhoenixProtocol.tProtocolName', dcmfile)
CSASeriesHeaderInfo tag: MrPhoenixProtocol
CSASeriesHeaderInfo tag: tProtocolName
CSAImageHeaderInfo tag: MrPhoenixProtocol
''

But if I read it directly I get:

>>> value = Image(dcm_file_csa).header.get('CSASeriesHeaderInfo')
>>> value['MrPhoenixProtocol']['value']['tProtocolName']
'CBU+AF8-DTI+AF8-64D+AF8-1A'

I don't have time now to look into it, but it seems to me things are not working as intended

For the first example, you need to use: get_dicomfield('MrPhoenixProtocol.value.tProtocolName', dcmfile)

Does it return the same value?

marcelzwiers commented 9 months ago

Ok, couldn't help myself, I added better tests and had to make bugfixes. The code is still dodgy because I don't know how exactly the CSA header is structured

dzemanov commented 9 months ago

If you would like to see what is the structure loaded from dicom_parser or csareader, you can look in debugger when the whole csa header is in value. Or you can copy the content of value with whole csa header to file and autoformat it, so it is easy to look at.

marcelzwiers commented 9 months ago

Yes, I did that but I still find it fuzzy

Sent from my phone

Op zo 28 jan 2024 23:30 schreef DominikaZ @.***>:

If you would like to see what is the structure loaded from dicom_parser or nibabel.csa_header, you can look in debugger when the whole csa header is in value. Or you can copy the content of value with whole csa header to file and autoformat it, so it is easy to look at.

— Reply to this email directly, view it on GitHub https://github.com/Donders-Institute/bidscoin/pull/220#issuecomment-1913742894, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTUGLZ5ZTEO3LDZWJJGQ2TYQ3GPVAVCNFSM6AAAAABCJRFQKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJTG42DEOBZGQ . You are receiving this because you commented.Message ID: @.***>

dzemanov commented 9 months ago

I understand. Maybe the code could try to catch exception if there would be any problem with that.

marcelzwiers commented 9 months ago

It seems to work so I'm merging it

dzemanov commented 9 months ago

Thank you very much! This will be a big help for us :).

marcelzwiers commented 9 months ago

Note that you should only use dicom-parser when you need data from the MrPhoenixProtocol, otherwise the nibabel csa parser will much faster (because of caching) and you will get the latest pydicom version (dicom-parser restricts pydicom to an older version)

Sent from my phone

Op ma 29 jan 2024 00:40 schreef DominikaZ @.***>:

Thank you very much! This will be a big help for us :).

— Reply to this email directly, view it on GitHub https://github.com/Donders-Institute/bidscoin/pull/220#issuecomment-1913761783, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTUGL6KCBFSXAAXJKAUO4TYQ3OVPAVCNFSM6AAAAABCJRFQKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJTG43DCNZYGM . You are receiving this because you modified the open/close state.Message ID: @.***>