Current GitHub version fails to process using JSON

[araikes]

@dvm-shlee and @gdevenyi

I pulled the current master to take advantage of the fixes implemented in #96 but I cannot convert using bids_convert and the -j option. I get:

(brkraw038_20221118) [adamraikes@r1u25n1 ifn_gamma_BIDS]$ brkraw bids_convert dicoms nifti/.brkraw/dicominfo_final.csv -o nifti/ -j nifti/.brkraw/dicominf.json 
Inspect input BIDS datasheet...
Converting 20210721_134024_apoemouse_IFN_Y_C1_QA1136_1_155...
Traceback (most recent call last):
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/bin/brkraw", line 8, in <module>
    sys.exit(main())
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/lib/python3.7/site-packages/brkraw/scripts/brkraw.py", line 476, in main
    build_bids_json(dset, row, fname, json_fname, slope=slope, offset=offset)
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/lib/python3.7/site-packages/brkraw/lib/utils.py", line 412, in build_bids_json
    metadata=ref, condition=condition)
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/lib/python3.7/site-packages/brkraw/lib/loader.py", line 548, in save_json
    json_obj = self._parse_json(scan_id, reco_id, metadata)
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/lib/python3.7/site-packages/brkraw/lib/loader.py", line 506, in _parse_json
    val = meta_get_value(v, acqp, method, visu_pars)
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/lib/python3.7/site-packages/brkraw/lib/utils.py", line 170, in meta_get_value
    return meta_check_express(value, acqp, method, visu_pars)
  File "/home/u26/adamraikes/.conda/envs/brkraw038_20221118/lib/python3.7/site-packages/brkraw/lib/utils.py", line 252, in meta_check_express
    exec('{} = {}'.format(k, val))
  File "<string>", line 1, in <module>
NameError: name 'VisuAcqEchoTime' is not defined

EDIT: It appears that it's the VisuAcqEchoTime keyword specifically for this dataset. Doing more digging to investigate.

[araikes]

Alright.. Figured it out the issue but don't know how to fix it. The acquisition was a multi-echo T2 RARE so I have:

##$VisuAcqEchoTime=( 6 )
7 21 35 49 63 77

Interestingly, brkraw tonii 20210721_134024_apoemouse_IFN_Y_C1_QA1136_1_155/ --bids -s 4 produces the correct files (n = 6 individual NIFTIs) along with a single JSON with the multiple TEs.

[gdevenyi]

Looks like a corner case of the bids_convert combined with the mutli-echo data.

[araikes]

I tried setting modality to "MESE" in the dicominfo.csv manually but it still failed.

[gdevenyi]

can you go over the full steps you're doing for setting up the bids output and share the bids files your're piping in?

[araikes]

1. brkraw bids_helper dicoms/ nifti/.brkraw/dicominfo -j
2. Manually edit the CSV to clean up animal names and set the "dir" parameter for the DWI data (LR/RL). Resave as dicominfo_final.csv
3. brkraw bids_convert dicoms/ nifti/.brkraw/dicominfo_final.csv -o nifti/ -j nifti/.brkraw/dicominf.json (note that this is the JSON file produced. There's an error in the code that sets the file name as it should be dicominfo.json).

I'm attaching the CSV file. I can share a dataset too if you send me a location to send a link.
dicominfo_final.csv

[araikes]

@gdevenyi,

I now have the same problem converting a dataset with no multiecho data. This would suggest that the JSON parser differs between tonii -b and bids_convert. I've been trying to track it down but haven't found it.

[gdevenyi]

Am I correct that this used to work with an earlier version? If so, you could try a git bisect to narrow down the commit which broke it, which would narrow down the code.

[araikes]

I'll try that, but I found the general error. The error codes weren't sufficient to get to it, but it was actually the B0 Map that we acquired with the RARE. It does not have the VisuAcqEchoTime field and instead of defaulting to Value was not specified (previous behavior) it simply crashes the process.

[grandjeanlab]

I just had the same issue. Fields maps it is.

[jeremie-fouquet]

Currently running into the same issue. Using git bisect, the problematic commit seems to be 3b871e2.

Before this commit, in the case of a B0 map, meta_check_source() returned the output of get_value when called with an inexistent key_string parameter (i.e., None). After this commit, meta_check_source() returned key_string if key_string was an inexistent parameter (like VisuAcqEchoTime for the B0 map).

Still, I think the code after this commit is an improvement. The code before the commit was checking for the presence of Visu, PVM, or ACQ in the parameter name key_string, which could lead to several issues, like issue #92. I think we should not revert back the code to before the commit, but simply change the current behaviour in case of an inexistent parameter. An inexistent parameter should lead to Value was not specified in the json file, or simply to the absence of this parameter. To accomplish this, I would simply output None when the parameter key_string is inexistent, as was previously (and inadvertently?) done for VisuAcqEchoTime for the B0 map. @gdevenyi and @dvm-shlee, do you agree?

This being said, in the case of the B0 map produced by Bruker, we fall closer to the case described here in the BIDS format. We are in the presence of a fieldmap already computed by Paravision, in Hertz units. We should hence include the Units field in the json file. Also, Unlike GE, ParaVision does not produce a magnitude image by default, so I guess we don't have to deal with this.

[dvm-shlee]

@jeremie-fouquet Thank you for identifying the source of the issue. I'm not certain why I originally made it return the input key value, but if this change doesn't impact other areas (which it shouldn't, as my original code actually raises an exception), then returning None makes sense to me.

On another note, regarding the multi-echo image,@araikes, do we need to separate them for each echo, or can they be a single image? I believe the original brkraw output was for each TE, but I'm not sure how this was managed during the BIDS conversion. I'll need to review the data.

If possible, could you share an example BIDS csv file and a single sample pvdataset (compressed in a zip file)? Having these materials would be helpful for testing.

[araikes]

@dvm-shlee

Multi-echo images should be individual echo images with the appropriate suffix (e.g., MESE or MEGRE). See https://bids-specification.readthedocs.io/en/stable/appendices/file-collections.html

[dvm-shlee]

@araikes, I'm reopening this issue because I realized I haven't tested the merge with the problematic dataset. Could you provide a publicly sharable multi-echo dataset for testing the brkraw functionality, especially for multi-echo data? I'm working on creating a test logic for this and plan to add some test datasets we've collected to the rawdata repository in our organization. If confidentiality is a concern, I can limit the use of the dataset to personal testing. Thanks!

[dvm-shlee]

As @jeremie-fouquet recommended, I'm updating the overall BIDS generator this month to enhance its flexibility and clarity. I'll keep you posted on the progress. This month, I'll be dedicating my efforts to brkraw, aiming to improve its readability and documentation for this project.