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

dashes in DICOM field causing error in pet2bids plugin #182

Closed yehman closed 1 year ago

yehman commented 1 year ago

When using bidscoiner, an error is generated when value from DICOM field containing spaces and dashes is passed to the pet2bids plugin.

To reproduce Steps to reproduce the behavior: Run bidscoiner on PET DICOM file with spaces and dashes in the tracer field. dcm2niix4pet will throw error.

Expected behavior dcm2niix4pet is passed values with spaces and dashes removed and runs correctly.

Workaround Can set explicit values with bidseditor for problematic fields.

error log 2023-04-27 17:15:42 - ERROR | Failed to run: dcm2niix4pet "/home/syeh/Documents/EMUI/DICOM/sub-00067/ses-00067_PET/001-FI2 PF 4 LF 4 D30 AC" -d /home/syeh/Documents/EMUI/bidsconv/sub-00067/ses-00067PET/pet/sub-00067_ses-00067PET_task-FI2PF4LF4D30AC_trc-FDGfluorodeoxyglucose_run-1_pet.nii.gz --kwargs TracerName=FDG -- fluorodeoxyglucose InjectedRadioactivity=384800000 TimeZero=083000.00 FrameDuration=900000 Errorcode 2:

usage: dcm2niix4pet [-h] [--metadata-path METADATA_PATH] [--translation-script-path TRANSLATION_SCRIPT_PATH] [--destination-path DESTINATION_PATH] [--kwargs [KWARGS [KWARGS ...]]] [--silent SILENT] [--show-examples] [--set-dcm2niix-path SET_DCM2NIIX_PATH] [folder] dcm2niix4pet: error: unrecognized arguments: -- fluorodeoxyglucose InjectedRadioactivity=384800000 TimeZero=083000.00 FrameDuration=900000

marcelzwiers commented 1 year ago

@bendhouseart I think adding protecting quotes on line 287 should work, right?

command += f' {metadata_key}={metadata_value}'

to

command += f" {metadata_key}='{metadata_value}'"
yehman commented 1 year ago

I had thought maybe the values needed to be passed through the invalid characters parser elsewhere in the code to fix, but made the change on line 287, and indeed it worked.

bendhouseart commented 1 year ago

Apologies for the delay in response; outlook does a very good job of filtering it seems.

I'm glad you found the workaround. It seems like I could be a bit more clear in the help and documentation as I specify that one should escape characters with double quotes, but go on to show the usage of single in the examples.

 --kwargs [KWARGS ...], -k [KWARGS ...]
                        Include additional values in the nifti sidecar json or
                        override values extracted from the supplied nifti.
                        e.g. including `--kwargs TimeZero='12:12:12'` would
                        override the calculated TimeZero. Any number of
                        additional arguments can be supplied after --kwargs
                        e.g. `--kwargs BidsVariable1=1 BidsVariable2=2` etc
                        etc.Note: the value portion of the argument (right
                        side of the equal's sign) should alwaysbe surrounded
                        by double quotes BidsVarQuoted="[0, 1 , 3]"

# and 

example 1 (Passing PET metadata via the --kwargs argument): 

    # Note `#` denotes a comment
    # dcm2niix4pet is called with the following arguments

    # folder -> GeneralElectricSignaPETMR-NIMH/
    # destination-path -> sub-GeneralElectricSignaPETMRINIMH/pet
    # kwargs -> a bunch of key pair arguments spaced 1 space apart with the values surrounded by double quotes

    dcm2niix4pet GeneralElectricSignaPETMR-NIMH/ --destination-path sub-GeneralElectricSignaPETMRNIMH/pet 
    --kwargs TimeZero="14:08:45" Manufacturer="GE MEDICAL SYSTEMS" ManufacturersModelName="SIGNA PET/MR" 
    InstitutionName="NIH Clinical Center, USA" BodyPart="Phantom" Units="Bq/mL" TracerName="Gallium citrate" 
    TracerRadionuclide="Germanium68" InjectedRadioactivity=1 SpecificRadioactivity=23423.75 
    ModeOfAdministration="infusion" FrameTimesStart=0 
    AcquisitionMode="list mode" ImageDecayCorrected="False" FrameTimesStart="[0]" ImageDecayCorrectionTime=0 
    ReconMethodParameterValues="[1, 1]" ReconFilterType="n/a" ReconFilterSize=1

I think either quotes should be fine, but I'm going to clean up the documentation to be sure to only reference using double quotes as that is what we actually run tests with.

As to why you got this error in the first place @yehman

Inserting -- into the dcm2niix4pet cli will lead to trouble as: 1) we are using an argparse cli with pypet2bids and those are reserved characters 2) after parsing pypet2bids will attempt to convert any input it receives into a native python data type such as a list, string, float, or int. Anything it fails to understand will be wrapped in quotes and treated as a string.