Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving trigger pictures. #15

Closed
3 tasks
smoia opened this issue Oct 9, 2019 · 13 comments · Fixed by #153
Closed
3 tasks

Improving trigger pictures. #15

smoia opened this issue Oct 9, 2019 · 13 comments · Fixed by #153
Assignees
Labels
Enhancement New feature or request

Comments

@smoia
Copy link
Member

smoia commented Oct 9, 2019

So far, the trigger picture created around this line have a simple title, and show the expected start and expected end of the sequence.
It would be nice to:

  • add the name of the corresponding tsv output file in the title
  • 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?
@smoia smoia added the Good first issue Good for newcomers label Oct 9, 2019
@smoia smoia added this to the OHBM poster milestone Oct 9, 2019
@smoia smoia modified the milestones: OHBM poster, OHBM presentation Dec 2, 2019
@vinferrer
Copy link
Collaborator

I think this is a good first issue but since no one aproach it maybe we should restart it stefano

@vinferrer
Copy link
Collaborator

@smoia

@smoia
Copy link
Member Author

smoia commented Feb 26, 2020

No rush on this, but yes, we can restart it. Wanna take care of it?

@smoia smoia added Enhancement New feature or request and removed Good first issue Good for newcomers labels Feb 26, 2020
@vinferrer
Copy link
Collaborator

one question. What's the difference between the two plots in the second row?

image

@smoia
Copy link
Member Author

smoia commented Feb 28, 2020

They should be different - showing the first and the last TR trigger. It's made so that if the sequence is long but you are asking the program to detect beginning and length of the sequence, you can see how the program performed and eventually correct issues.
How did you obtain this output?

@vinferrer
Copy link
Collaborator

phys2bids -in Test_belt_pulse_multifreq.txt -chtrig 3
I change the title only everything else comes from the master branch

@smoia
Copy link
Member Author

smoia commented Feb 28, 2020

I see. In this case, the fact that the two subplots are the same might arise from the fact that there is no start and length check. We could add a flag, to not draw the second line when this option is not required - or to expand the plot on the left to the whole line.

another thing would be to substitute the time line (orange) with something else - maybe a semi-transparent rectangle on the background?

@vinferrer
Copy link
Collaborator

vinferrer commented Feb 28, 2020

Well I am checking the code and the repetition of the plot it's due to the fact that you are plotting the same exact line in both plots:

    subplot = fig.add_subplot(223)
    subplot.set_xlim([-options.tr * 4, options.tr * 4])
    subplot.set_ylim([-0.2, options.thr * 3])
    subplot.secondary_xaxis('top', functions=(time2ntr, ntr2time))
    subplot.plot(time, trigger, '-', time, time, '-')
    subplot = fig.add_subplot(224)
    subplot.set_xlim([options.tr * (options.num_timepoints_expected - 4),
                      options.tr * (options.num_timepoints_expected + 4)])
    subplot.set_ylim([-0.2, options.thr * 3])
    subplot.secondary_xaxis('top', functions=(time2ntr, ntr2time))
    subplot.plot(time, trigger, '-', time, time, '-')

@vinferrer
Copy link
Collaborator

I see no difference between the 5 and the last line

@vinferrer
Copy link
Collaborator

Oh, sry i see the xlim now

@vinferrer
Copy link
Collaborator

Yeah so basically options.num_timepoints_expected is zero in this case

@vinferrer
Copy link
Collaborator

So I have changed the function a bit and now this is the output with the file:
image

@vinferrer
Copy link
Collaborator

I can share the new code by PR but maybe we should discuss the third plot. Why do you wanna plot the last trigger only? this can be seen in the second figure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants