Trigger pictures improvement

[vinferrer]

Closes #15

Proposed Changes

• title shows tvs filename
• second plot in second row is plotted based on the index of the last point that exceeds the treshold
• add a third subplot in the second row, that shows the last trigger
• rescale differently the Y axis in the first row, maybe the higher between twice the max value in the trigger array and three times the threshold?

[codecov]

Codecov Report

Merging #153 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #153   +/-   ##
=======================================
  Coverage   94.15%   94.15%           
=======================================
  Files           7        7           
  Lines         565      565           
=======================================
  Hits          532      532           
  Misses         33       33           

[vinferrer]

plot as it is right now

[vinferrer]

Current state of the plot

[vinferrer]

@smoia this is how the plot looks like now if you put the wrong threshold:

[vinferrer]

If you put it right:

[vinferrer]

this is for the Test_belt_multifreq.txt

[vinferrer]

However for the plots to be correct you have to input more parameters, look what happens for file
Test2_time_trigger_CO2_O2_pulse_1000Hz_534TRs:

[vinferrer]

The threshold is fine but nevertheless the second plots are wrong, I don't know if this could confuse the user

[smoia]

Can you present it during the meeting next week? We can ask feedbacks then!

[vinferrer]

Sure

[smoia]

@vinferrer good! You might also want to change the description of the picture in this file:
https://github.com/physiopy/phys2bids/blob/master/docs/howto.rst

[smoia]

@RayStick , can I ask you to have a look at the changes in the documentation once it's ready?

[vinferrer]

Sure, so we still have to make the creation of the file only when -tr or -ntp are inputed and then, change the docs

[vinferrer]

but wait, default tr is 1. ¿could that be problematic? my idea was that if the values are not the default then plot

[eurunuela]

Is the default TR used somewhere else in the code?

[RayStick]

@smoia yep! But it may be Sunday or Monday that I can review

[eurunuela]

Now that the default TR = 0, this looks good to me!

[vinferrer]

changes to docfile done, you can start the review

[vinferrer]

@RayStick, @smoia

[RayStick]

Looking good. I tested it with the tutorial file, with a few different argument iterations, and seems to work as intended.

Apart from the minor in-line comments I gave, just one other thing -

I really like the addition of the TR x-axes at the top of the plots. However, I didn't see it at first! Is there a way to make it stand out a bit more?

[vinferrer]

@RayStick Changes done, you can check them and if there is no more changes, approve the request.

Cheers