Dynamic values are evaluated although a sequence is not a match for run-item

[mateuszpawlik]

Describe the bug I have a sequence with SeriesDescription func-bold_task-rest and a matching run-item rule SeriesDescription: ^func.*. However, dynamic values from other run-items are evaluated on that sequence leading to

Traceback (most recent call last):                                                                                                                                         
  File "/usr/local/bin/bidsmapper", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.9/dist-packages/bidscoin/bidsmapper.py", line 233, in main
    bidsmapper(rawfolder    = args.sourcefolder,
  File "/usr/local/lib/python3.9/dist-packages/bidscoin/bidsmapper.py", line 133, in bidsmapper
    module.bidsmapper_plugin(sesfolder, bidsmap_new, bidsmap_old, template, store)
  File "/usr/local/lib/python3.9/dist-packages/bidscoin/plugins/dcm2niix2bids.py", line 136, in bidsmapper_plugin
    run, _ = bids.get_matching_run(datasource, template)
  File "/usr/local/lib/python3.9/dist-packages/bidscoin/bids.py", line 1449, in get_matching_run
    run_['bids'][bidskey] = datasource.dynamicvalue(bidsvalue, runtime=runtime)
  File "/usr/local/lib/python3.9/dist-packages/bidscoin/bids.py", line 246, in dynamicvalue
    sourcevalue += str(self.properties(val[0])) + str(self.attributes(val[0]))
  File "/usr/local/lib/python3.9/dist-packages/bidscoin/bids.py", line 179, in attributes
    attributeval = match[0]     # The first match is most likely the most informative (?)
IndexError: list index out of range

This of course happens if my sequence doesn't match dynamic values from other run-items, in my case SeriesDescription: ^anat.*

I'm not sure if it happens always. My original data has many more sequences.

To reproduce Steps to reproduce the behavior:

1. Generate DICOM files using DICOM generator and the following configuration (dyn-val.json):

   
   [
   {
   "ImageType" : "anat",
   "SeriesDescription" : "anat-T1w_acq-XYZ"
   },
   {
   "ImageType" : "fmri",
   "SeriesDescription" : "func-bold_task-rest"
   }
   ]

python3 generate_dicom.py dyn-val.json dataset/sourcedata/sub-1/

2. Execute `dicomsort`.

singularity exec --bind dataset/:/data bidscoin_3.7.2.sif dicomsort /data/sourcedata --subprefix sub

4. Execute `bidsmapper` with the map (`bidscoin_map.yaml`):
```yml
Options:

  bidscoin:
    version: 3.7.2
    bidsignore: extra_data/
    subprefix: sub-
    sesprefix: ses-
    datatypes: [anat, func]   # A list of datatypes that are converted to BIDS
    unknowntypes: [extra_data]    # A list of datatypes that are converted to BIDS-like datatype folders
    ignoretypes: [exclude]        # A list of datatypes that are excluded / not converted to BIDS
  plugins:
    dcm2niix2bids:
      path: ''
      args: -b y -z n -i n

DICOM:

  subject: <<filepath:/sub-(.*?)/>> #<<SourceFilePath>>
  session: <<filepath:/ses-(.*?)/>> #<<SourceFilePath>>

  anat:
  - provenance:
    attributes:
      SeriesDescription: ^anat.*
    bids:
      acq: <SeriesDescription:acq-(.*)$>
      suffix: T1w

  func:
  - provenance:
    attributes:
      SeriesDescription: ^func.*
    bids:
      task: <SeriesDescription:task-(.*)$>
      suffix: bold

singularity exec --bind dataset:/data bidscoin_3.7.2.sif bidsmapper --template $PWD/bidscoin_map.yaml /data/sourcedata /data

Expected behavior The dynamic values of only the matched run-item should be evaluated on a sequence.

Adding logger outputs on what went wrong in line https://github.com/Donders-Institute/bidscoin/blob/e7555166c148496d12afa629efb43a530e52c578/bidscoin/bids.py#L1449 could help.

Software version

• BIDScoin version 3.7.2
• Apptainer BIDScoin image on Debian host

[marcelzwiers]

Thanks for your excellent bug report. It seems to touch the core of BIDScoin's design, I'll look into it right away and, if solved, I will do a new point release

[mateuszpawlik]

Thank you so much. It took me more than four hours to find the problem. I don't want to rush any changes. For the moment I solved it with explicit mapping of most sequences by their full SeriesDescription without dynamic values.

[marcelzwiers]

It's a miracle this never surfaced before! I hope it is solved now, can you confirm this? I'll do my point release quickly after

[mateuszpawlik]

I built the image with your commit 2286d71. Now I get the following error:

Traceback (most recent call last):
  File "/usr/local/bin/bidsmapper", line 33, in <module>
    sys.exit(load_entry_point('bidscoin==3.7.3.dev0', 'console_scripts', 'bidsmapper')())
  File "/usr/local/lib/python3.9/dist-packages/bidscoin-3.7.3.dev0-py3.9.egg/bidscoin/bidsmapper.py", line 243, in main
    bidsmapper(rawfolder    = args.sourcefolder,
  File "/usr/local/lib/python3.9/dist-packages/bidscoin-3.7.3.dev0-py3.9.egg/bidscoin/bidsmapper.py", line 60, in bidsmapper
    metasubprefix  = [char for char in subprefix if char in ('^', '$', '+', '{', '}', '[', ']', '\\', '|', '(', ')')]
TypeError: 'NoneType' object is not iterable

It seems like you don't set a default value in argparse.

[marcelzwiers]

I see, that's due to the previous patch I made to account for users that like to use wildcards in the sub-prefix. I've now tried to catch this too (see commit 8cf5c4de628de7be406977656fae2d1a62c23f05).

[mateuszpawlik]

Now it worked :+1: I can verify it on my original data next week. Here's the output for the example in this issue:

2022-07-01 20:55:44 - INFO | 
2022-07-01 20:55:44 - INFO | -------------- START BIDSmapper ------------
2022-07-01 20:55:44 - INFO | >>> bidsmapper sourcefolder=/data/sourcedata bidsfolder=/data bidsmap=bidsmap.yaml template=/home/mpawlik/Remote/bids-misc-scripts/dicom-util/bidscoin_map.yaml plugins=[] subprefix=None sesprefix=None store=False force=False
2022-07-01 20:55:44 - INFO | No existing bidsmap file found: /usr/local/lib/python3.9/dist-packages/bidscoin-3.7.3.dev0-py3.9.egg/bidscoin/heuristics/bidsmap.yaml
2022-07-01 20:55:44 - INFO | Reading: /home/mpawlik/Remote/bids-misc-scripts/dicom-util/bidscoin_map.yaml
2022-07-01 20:55:44 - INFO | BIDScoiner version difference: /home/mpawlik/Remote/bids-misc-scripts/dicom-util/bidscoin_map.yaml was created with version 3.7.2, but this is version 3.7.3-dev. This is normally ok but check the https://bidscoin.readthedocs.io/en/latest/CHANGELOG.html
2022-07-01 20:55:48 - INFO | Adding missing DICOM>anat>T1w entity key: run
2022-07-01 20:55:48 - INFO | Adding missing DICOM>anat>T1w entity key: ce
2022-07-01 20:55:48 - INFO | Adding missing DICOM>anat>T1w entity key: rec
2022-07-01 20:55:48 - INFO | Adding missing DICOM>anat>T1w entity key: part
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: acq
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: ce
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: rec
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: dir
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: run
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: echo
2022-07-01 20:55:48 - INFO | Adding missing DICOM>func>bold entity key: part
2022-07-01 20:55:48 - INFO | Mapping: /data/sourcedata/sub-1 (subject 1/1)                                                                                                 
2022-07-01 20:55:48 - INFO | Executing plugin: dcm2niix2bids.py -> /data/sourcedata/sub-1                                                                                  
2022-07-01 20:55:48 - INFO | Discovered 'anat' DICOM sample: /data/sourcedata/sub-1/001-anat-T1w_acq-XYZ/MR001.dcm                                                         
2022-07-01 20:55:48 - INFO | Discovered 'func' DICOM sample: /data/sourcedata/sub-1/002-func-bold_task-rest/MR002.dcm                                                      
2022-07-01 20:55:48 - INFO | Opening the bidseditor                                                                                                                        
QStandardPaths: error creating runtime directory '/run/user/1000' (No such file or directory)
2022-07-01 20:55:53 - INFO | User is editing /data/sourcedata/sub-1/001-anat-T1w_acq-XYZ/MR001.dcm
2022-07-01 20:56:02 - INFO | User has canceled the edit
2022-07-01 20:56:04 - INFO | User is editing /data/sourcedata/sub-1/002-func-bold_task-rest/MR002.dcm
2022-07-01 20:56:09 - INFO | User has canceled the edit
2022-07-01 20:56:12 - INFO | -------------- FINISHED! -------------------
2022-07-01 20:56:12 - INFO | 
2022-07-01 20:56:12 - INFO | No BIDScoin errors or warnings were reported
2022-07-01 20:56:12 - INFO | 
2022-07-01 20:56:12 - INFO | For the complete log see: /data/code/bidscoin/bidsmapper.log
NB: Files in /data/code/bidscoin may contain privacy sensitive information, e.g. pathnames in logfiles and provenance data samples

[marcelzwiers]

Great, I'll do a bit more testing myself, update the docs a bit and wait for the successful verification on your original data before I release v3.7.3 :-)

[mateuszpawlik]

I'm sorry but I didn't find time to test the fix. I'll try to do it tomorrow. Update: I've started but unfortunately haven't finished. I'm sorry. I'll test it latest on Monday.

[mateuszpawlik]

I've tested 8cf5c4d with the following map file and it works as expected.

More complex map file

``` Options: bidscoin: version: 3.7.0 bidsignore: extra_data/ subprefix: sub- sesprefix: ses- datatypes: [fmap, anat, func, dwi] # A list of datatypes that are converted to BIDS unknowntypes: [extra_data] # A list of datatypes that are converted to BIDS-like datatype folders ignoretypes: [exclude] # A list of datatypes that are excluded / not converted to BIDS plugins: dcm2niix2bids: path: '' args: -b y -z n -i n DICOM: subject: <> #<> session: <> #<> anat: - provenance: attributes: &anat_dicomattr ImageType: SeriesDescription: bids: &anat_bidsattr suffix: T1w acq: T1wHCPLI - provenance: attributes: <<: *anat_dicomattr SeriesDescription: (?i)^anat-.* bids: <<: *anat_bidsattr suffix: func: - provenance: attributes: &func_dicomattr ImageType: SeriesDescription: bids: &func_bidsattr acq: task: suffix: bold - provenance: attributes: <<: *func_dicomattr SeriesDescription: (?i)(.*task-aw.*|.*task-vw.*) bids: <<: *func_bidsattr run: - provenance: attributes: <<: *func_dicomattr SeriesDescription: (?i)^func.* bids: <<: *func_bidsattr fmap: - provenance: attributes: &fmap_dicomattr ImageType: SeriesDescription: bids: &fmap_bidsattr acq: suffix: magnitude1 meta: &fmap_metaattr IntendedFor: task - provenance: attributes: <<: *fmap_dicomattr SeriesDescription: (?i)^fmap.* ImageType: .*'M'.* bids: <<: *fmap_bidsattr suffix: magnitude2 # Due to magnitude1 and magnitude2 images in the same series (Siemens). dcm2niix will deal with it correctly. meta: <<: *fmap_metaattr - provenance: attributes: <<: *fmap_dicomattr SeriesDescription: (?i)^fmap.* ImageType: '[''ORIGINAL'', ''PRIMARY'', ''P'', ''ND'']' bids: <<: *fmap_bidsattr suffix: phasediff meta: <<: *fmap_metaattr dwi: - provenance: attributes: &dwi_dicomattr ImageType: SeriesDescription: bids: &dwi_bidsattr suffix: dwi - provenance: attributes: <<: *dwi_dicomattr SeriesDescription: dwi_ses-.*_acq-dwi_dir-AP bids: <<: *dwi_bidsattr acq: 256dir dir: AP - provenance: attributes: <<: *dwi_dicomattr SeriesDescription: fmap_ses-.*_acq-dwi_dir_PA bids: <<: *dwi_bidsattr acq: sixdir dir: PA - provenance: attributes: <<: *dwi_dicomattr SeriesDescription: fmap_ses-.*_acq-dwi_dir_AP bids: <<: *dwi_bidsattr acq: sixdir dir: AP exclude: - provenance: attributes: SeriesDescription: (?i)(.*scout.*|rf_map|.*TB1$|.*RB1cor$|.*MPM.*) bids: suffix: ```

[marcelzwiers]

Thanks again, it should now all work in the new release :-)

[mateuszpawlik]

Happy to help :smiley: :+1: