bids.util.tsvwrite does not correctly write fields and values from struct to output tsv

[mnoergaard]

Imported a *.tsv file with 6 columns with 11 rows of values (excluding headers) using bids.util.tsvread.

The data is loaded in as a struct with 6 fields (name of headers) each with the size [11,1] double.

Currently in the code, the [11,1] array gets converted into a cell, and when concatenated into var on line 40, this becomes a 2x6 cell.
The consequence is that only 2x6 are written to the output *.tsv because the cells in the second row of var are not unrolled, and hence not looped through when writing to the file.

A quick fix would be to replace the following on line 40: var = [fn'; horzcat(var{:})];

Then the correct values are written to the output *.tsv file.

[Remi-Gau]

hey @mnoergaard
Thanks for reporting this.
Will have a look a bit later and try to make sure I can reproduce the bug. :-)

[mnoergaard]

Sounds good, @Remi-Gau. This example file will make it easier to reproduce :-)

https://www.dropbox.com/s/hnk2tvytx3lib31/sub-01_recording-autosampler_blood.tsv?dl=0

[Remi-Gau]

Awesome!! Thanks!!!

[Remi-Gau]

hey @mnoergaard

Just to make sure that I get what you want: you would expect that reading the content of a tsv and writing it right away, should not change the content of the file, correct?

something like this ⏬ ?

 %% ensure that reading and then writing does not change the format
  
  % read file
  tsv_file = fullfile(pth, 'data', 'sub-01_recording-autosampler_blood.tsv');
  output = bids.util.tsvread(tsv_file);

  % write file  
  new_tsv_file = fullfile(pth, 'data', 'sub-01_recording-autosampler_blood_new.tsv');
  bids.util.tsvwrite(new_tsv_file, output);
  
  % reread the new file and makes sure its content matches the previous one
  new_output = bids.util.tsvread(new_tsv_file);
  assertEqual(output, new_output);

If so then indeed this test fails.

elements are not equal

First input: 1x1 struct
                         time: [1614×1 double]
    whole_blood_radioactivity: [1614×1 double]

Second input: 1x1 struct
                         time: 0
    whole_blood_radioactivity: 0.1004

  test_read_write:100 (/home/remi/github/BIDS-matlab/tests/test_tsvwrite.m)

--------------------------------------------------

FAILED (passed=1, failure=1)

I also think this is a VERY reasonable behavior to expect. So I will flag this as a bug and try to push a fix.

[gllmflndn]

Thanks both for this. Is it a recent regression as it seems to work fine with the original functions?

>> tsv = spm_load('sub-01_recording-autosampler_blood.tsv')

tsv = 

  struct with fields:

                         time: [1614×1 double]
    whole_blood_radioactivity: [1614×1 double]

>> spm_save('test.tsv',tsv);
>> tsv2 = spm_load('test.tsv');
>> isequal(tsv,tsv2)

ans =

  logical

   1

[Remi-Gau]

yeah I was just looking at the original code from spm to see where I had messed up when porting it.

🤦

[gllmflndn]

Not blaming, just curious! Reaching the same conclusion that line 40 should read:

var = [fn'; var{:}];

[Remi-Gau]

Yes I am there too. problem is that it is making the previous tests fail.

  tsv_file = fullfile(pth, 'sub-01_task-STRUCTURE_events.tsv');

  logFile(1, 1).onset = 2;
  logFile(1, 1).trial_type = 'motion';
  logFile(1, 1).duration = 1;
  logFile(1, 1).speed = [];
  logFile(1, 1).is_fixation = true;

  logFile(2, 1).onset = NaN;
  logFile(2, 1).trial_type = 'static';
  logFile(2, 1).duration = 4;
  logFile(2, 1).is_fixation = 3;

  bids.util.tsvwrite(tsv_file, logFile);

Dimensions of matrices being concatenated are not consistent.
  tsvwrite:40 (/home/remi/github/BIDS-matlab/+bids/+util/tsvwrite.m)

This is a "shape" of structure I have used in our lab to collect data event after event while experiments run: but we are not using bids-matlab to save them. So I don't why we should support them.

Will rewrite the first test.

[gllmflndn]

I see, with input stored row-wise rather than column-wise. It's a different format but we could try to support it too, by adding a special handling if numel(var) > 1 (or size(var,1) > 1 depending where you look) ?

[Remi-Gau]

I see, with input stored row-wise rather than column-wise. It's a different format but we could try to support it too, by adding a special handling if numel(var) > 1 (or size(var,1) > 1 depending where you look) ?

is that something we want to support though?

while rewriting the test I realized that this other format allows a slightly more flexible missing of types within the same field.

will open a PR to let you see what I meant/

[Remi-Gau]

@all-contributors please add @mnoergaard for bug, ideas

[allcontributors]

@Remi-Gau

I've put up a pull request to add @mnoergaard! 🎉

[Remi-Gau]

@mnoergaard this should now be fixed.

If you pull the latest version of the master branch, this should work as expected.

Also added the fiox to dev branch.

[mnoergaard]

Nice, @Remi-Gau! Good job!

[Remi-Gau]

Thanks for spotting it and reporting it. 😉 🚀