Skip to content

Conversation

@marcobarilari
Copy link
Collaborator

fix #197, this way the .mat files contains only the trial_type entries that are present in the events file. This way if eg responses are missing, they will not be put in the model.

(I have created this pr from the submodule of my analysis repo, it is the first so let me know if this way is correct)

@codecov
Copy link

codecov bot commented Nov 29, 2020

Codecov Report

Merging #210 (6240998) into dev (64b24bc) will increase coverage by 0.16%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #210      +/-   ##
==========================================
+ Coverage   45.20%   45.36%   +0.16%     
==========================================
  Files          96       96              
  Lines        1701     1706       +5     
==========================================
+ Hits          769      774       +5     
  Misses        932      932              
Impacted Files Coverage Δ
src/subject_level/createAndReturnOnsetFile.m 100.00% <ø> (ø)
src/subject_level/convertOnsetTsvToMat.m 90.62% <100.00%> (+1.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64b24bc...6240998. Read the comment docs.

@Remi-Gau
Copy link
Contributor

Cool. This looks good.

Let me update the test to make sure this thing behaves like we hope it should.

(I have created this pr from the submodule of my analysis repo, it is the first so let me know if this way is correct)

I tend to often do that.

This is is fine by me but I think only collaborators on the upstream repo will be able to do that. Not a big issue I think.

@Remi-Gau Remi-Gau changed the title fix #197 [FIX] Missing trial_types are not added to a model specification / fixes #197 Nov 29, 2020
Copy link
Contributor

@Remi-Gau Remi-Gau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Will add a test. :-)

@Remi-Gau Remi-Gau merged commit c62f6e7 into dev Nov 29, 2020
@marcobarilari marcobarilari deleted the marco_fix-#197 branch November 29, 2020 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐛 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FFX specify&estimate fails in case in a run a specific tryal_type specified in the model is missing

3 participants