IndexError: string index out of range in dcm2niix2bids.py

[mateuszpawlik]

Describe the bug I get IndexError: string index out of range in line

https://github.com/Donders-Institute/bidscoin/blob/2aceaa4ebd4a2894c875f0e3944ff1b3b7f71a20/bidscoin/plugins/dcm2niix2bids.py#L171

I think it is due to empty run['bids']['part']. I don't set this value in the bidsmap file and bidsmapper presets it to empty.

run['bids']['part'][-1] in the if statement causes the error.

When part set to 0 in the bidsmap file, I get TypeError: 'int' object is not subscriptable.

Another note You're checking there the ImageType DICOM field (0008, 0008) but the BIDS specification says, that this value corresponds to (0008,9208) which is ComplexImageComponent, which I can't find in my DICOM file.

To reproduce This is vague: execute bidsmapper without setting part for a bold image.

Expected behavior I'm not sure.

Screenshots First line prints run['bids']

{'task': 'rest', 'acq': 'norm', 'suffix': 'bold', 'run': '<<>>', 'ce': '', 'rec': '', 'dir': '', 'echo': '', 'part': '', 'chunk': ''}
      sys.exit(main())
    File "/home/mpawlik/Remote/anc/anc-data-acquisition/venv/lib/python3.9/site-packages/bidscoin/bidsmapper.py", line 251, in main
      bidsmapper(rawfolder    = args.sourcefolder,
    File "/home/mpawlik/Remote/anc/anc-data-acquisition/venv/lib/python3.9/site-packages/bidscoin/bidsmapper.py", line 141, in bidsmapper
      module.bidsmapper_plugin(sesfolder, bidsmap_new, bidsmap_old, template, store)
    File "/home/mpawlik/Remote/anc/anc-data-acquisition/venv/lib/python3.9/site-packages/bidscoin/plugins/dcm2niix2bids.py", line 172, in bidsmapper_plugin
      if not datasource.datatype=='' and 'part' in run['bids'] and not run['bids']['part'][-1] and run['attributes'].get('ImageType'):    # part[-1]==0 -> part is not specified
  IndexError: string index out of range

Software version

• BIDScoin master branch pip install --force-reinstall git+https://github.com/Donders-Institute/bidscoin, should be 2aceaa4ebd4a2894c875f0e3944ff1b3b7f71a20
• Debian 11

Additional context I was trying to test fixes for #229. @marcelzwiers, have you considered using pull requests to fix issues? It seems to me that there are fixes of multiple things at once on the master branch. PRs would greatly help with isolating the fixes and testing them.

[marcelzwiers]

Indeed, I test with the default template, in which part is specified as a list (which defines/translates into a dropdown menu in the bidseditor). I now made a catch for this (i.e. when part is not specified as a list) when loading the bidsmap.

Another note You're checking there the ImageType DICOM field (0008, 0008) but the BIDS specification says, that this value corresponds to (0008,9208) which is ComplexImageComponent, which I can't find in my DICOM file.

Sure I use ImageType as an attribute to identify (discriminate) acquired DICOM scans (datatypes), but that has nothing to do with the Complex Image Component Attribute you are talking about. The latter is supposed to store the part mag/phase/real/imag info, but I also don't see anyting in our DICOM headers. Hence I'm not using it to populate part

have you considered using pull requests to fix issues? It seems to me that there are fixes of multiple things at once on the master branch. PRs would greatly help with isolating the fixes and testing them.

The way I work is as follows. When I'm working on a bug or feature that is not too complex and that I'm sure I want to land in master, then I develop it straight in master, otherwise I create a feature branch. I use PRs only to merge contributions from other people. I see the advantages of doing more of my development in feature/bugfix branches (but it sometimes also makes life more complicated due to potential merge conflicts), but I don't see what the benefit would be using PR's for my own branches as opposed to merging them directly with master? Sure, there is CI on github, but I locally use tox to get the same functionality (but way faster)

[marcelzwiers]

Thanks for reporting this. You are using bidscoin in a way that I didn't intend, which is a very good thing as it makes me fix the code to be more robust and flexible :-)

[mateuszpawlik]

Another note You're checking there the ImageType DICOM field (0008, 0008) but the BIDS specification says, that this value corresponds to (0008,9208) which is ComplexImageComponent, which I can't find in my DICOM file.

Sure I use ImageType as an attribute to identify (discriminate) acquired DICOM scans (datatypes), but that has nothing to do with the Complex Image Component Attribute you are talking about. The latter is supposed to store the part mag/phase/real/imag info, but I also don't see anyting in our DICOM headers. Hence I'm not using it to populate part

The comment in the code says

https://github.com/Donders-Institute/bidscoin/blob/2a0dd3ae621f3086e7617449bf28c2860406c7f7/bidscoin/plugins/dcm2niix2bids.py#L171

So I assumed that you're trying to set this value. Therefore the comment about the DICOM field.

[mateuszpawlik]

have you considered using pull requests to fix issues? It seems to me that there are fixes of multiple things at once on the master branch. PRs would greatly help with isolating the fixes and testing them.

The way I work is as follows. When I'm working on a bug or feature that is not too complex and that I'm sure I want to land in master, then I develop it straight in master, otherwise I create a feature branch. I use PRs only to merge contributions from other people. I see the advantages of doing more of my development in feature/bugfix branches (but it sometimes also makes life more complicated due to potential merge conflicts), but I don't see what the benefit would be using PR's for my own branches as opposed to merging them directly with master? Sure, there is CI on github, but I locally use tox to get the same functionality (but way faster)

I've just noticed that you reference the issue ids in your commits, which will help me now.

In this context, a PR would group the commits relevant to resolving a specific issue, which then helps in identifying commits or simply a branch to test, before it is merged to master, especially when multiple issues are resolved in parallel. PRs also give a nice summarized view of the changes over all commits for a specific issue, which in my experience helps to focus :smiley:

[mateuszpawlik]

I can't reproduce this problem anymore on the master branch.

[marcelzwiers]

Sure I use ImageType as an attribute to identify (discriminate) acquired DICOM scans (datatypes), but that has nothing to do with the Complex Image Component Attribute you are talking about. The latter is supposed to store the part mag/phase/real/imag info, but I also don't see anything in our DICOM headers. Hence I'm not using it to populate part

The comment in the code says

https://github.com/Donders-Institute/bidscoin/blob/2a0dd3ae621f3086e7617449bf28c2860406c7f7/bidscoin/plugins/dcm2niix2bids.py#L171

So I assumed that you're trying to set this value. Therefore the comment about the DICOM field.

Ah sure, there I'm trying to guess the image type before the bidsmap is fed to the bidseditor. In the BIDS specification they mean something else when they talk about ImageType, as you noted. I wasn't sure what you were trying to say, but I suppose we are on the same line of thought here