Generate participants.tsv if it doesn't exist or update it if subject is missing in the file

[eurunuela]

Partly closes #242 and #185

Proposed Changes

• Generate participants.tsv if it doesn't exist already.

[codecov]

Codecov Report

Merging #244 into master will decrease coverage by 0.85%.
The diff coverage is 82.00%.

@@            Coverage Diff             @@
##           master     #244      +/-   ##
==========================================
- Coverage   94.41%   93.56%   -0.86%     
==========================================
  Files           8        8              
  Lines         681      730      +49     
==========================================
+ Hits          643      683      +40     
- Misses         38       47       +9     

Impacted Files	Coverage Δ
phys2bids/bids.py	87.91% <78.57%> (-8.01%)	⬇️
phys2bids/cli/run.py	96.55% <100.00%> (+0.12%)	⬆️
phys2bids/phys2bids.py	89.65% <100.00%> (+0.08%)	⬆️
phys2bids/utils.py	92.10% <100.00%> (+0.55%)	⬆️

[eurunuela]

@smoia @vinferrer @tsalo what should we do when participants.tsv already exists?

1. Warn the user and not update the file.
2. Warn the user and update the file with the yaml content.
3. Have an overwrite option in the parser and update the file.

Of course, this is also valid for dataset_description.json.

[tsalo]

For the participants.tsv, I think that updating an existing file will be the norm after converting the first participant, so I vote 2, except when the participant already exists in the file.

[smoia]

Totally agree with @tsalo on this.
Check if the file exists,
if so check if the subject id is present,
if so update with missing information (but without overwriting values),
otherwise add subject id (and values) to the list
Then update the file in the folder or write it.

I think we can follow this steps for all the files.

[eurunuela]

Tests pass but we're losing coverage. I'll add an extra call to phys2bids in the integration test to cover part of the new function. Given that this PR is critical for OHBM, I think we can add the unit tests later.

What do you think @smoia @vinferrer ?

[eurunuela]

Adding the extra call to phys2bids makes the test much slower... How about we add the dataset_description.json functionality first and then we work on the unit tests to avoid tests taking this long.

[vinferrer]

Adding the extra call to phys2bids makes the test much slower... How about we add the dataset_description.json functionality first and then we work on the unit tests to avoid tests taking this long.

Sure, Do you wanna open an issue so we don't forget about it?

[eurunuela]

@vinferrer I think I've covered all your comments, let me know!

[vinferrer]

Good to go then

[smoia]

Looks good!
A couple of changes here and there, plus a bit of discussion.

I agree on putting aside the unit test for now - please open an issue about them though!

One thing: this could actually call for the use of pandas. Do we want to add it as a dependency?

[eurunuela]

@smoia I think I've covered the most important comments. Please let me know.

[eurunuela]

There's still a bug on that previous commit. Fix coming.

[smoia]

LGTM!