Add multifrequency detection to txt files

[vinferrer]

Closes #56
Adds multi frequency support for labchart files

Proposed Changes

-Adds multi frequency support for labchart files

[codecov]

Codecov Report

Merging #150 into master will increase coverage by 1.61%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
+ Coverage   65.04%   66.66%   +1.61%     
==========================================
  Files           7        7              
  Lines         515      540      +25     
==========================================
+ Hits          335      360      +25     
  Misses        180      180

Impacted Files	Coverage Δ
phys2bids/interfaces/txt.py	96% <100%> (+0.79%)	⬆️

[eurunuela]

LGTM but we're gonna have to add a test for this function in test_txt.py

[vinferrer]

unit test added closing #151

[smoia]

Good start, let's see if we can improve it a bit more!

[eurunuela]

I would not check the channel at random positions. I think it would be wiser to check the start, middle and end of the channel unless we are certain that whatever random positions we choose the result will always be the same.

[smoia]

It's actually more probable that the results will be the same checking three random positions in the middle rather than at the start and the end - in files that start earlier than the beginning of the sequence and have padding at the end, the trigger channel (and any other similar thing) might have the same value at the beginning and at the end for more than your sampling. This might lead to incorrect frequency classification.

[smoia]

Solution: why don't we open an issue about it, and merge in this PR?

[vinferrer]

We can discuss about this, I propose that the function does the process that is doing right now but in 3 subarrays that are seeded in random positions and put the size of these positions as an optional parameter

[vinferrer]

but if you want we can merge this for the moment

[vinferrer]

So, what do we do here?

[smoia]

LGTM, let's merge it in now and open an issue about improving the multifreq check.

[smoia]

As soon as @eurunuela agrees on the changes, you can merge this in!

[vinferrer]

let me solve the merge conflic

[smoia]

However, please update your branch with respect to the master branch

[vinferrer]

@smoia, @eurunuela I think with this last commit I solved the problem with the while loop. Now instead of limiting the while loop to a fix number, I get the maximum number of repeated samples as a reference, This way the number of samples that are not tested for multi frequency in each channel is reduce to a few hundred but the hole array is inspected without need of dividing it. This means we don't have to open an issue about improving the multifrequency check

[vinferrer]

therefore I think @smoia needs to check again the function

[vinferrer]

Sry for ruining your approval

[eurunuela]

It looks good to me in general aside from the triple loop. However I think a couple of points should be resolved before merging.

[vinferrer]

Nice, Once @smoia finishes with the review I will merge

[smoia]

LGTM!