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

bidsConcatBetaTmaps. fix #425

Merged
merged 7 commits into from
Nov 1, 2021
Merged

Conversation

CerenB
Copy link
Collaborator

@CerenB CerenB commented Oct 22, 2021

minor fixes with:

  • how to call specifyContrast.m with the recent version
  • comma in the structure
  • use_schema option set to false otherwise it is crashing but maybe another way is better?

@CerenB CerenB requested a review from Remi-Gau October 22, 2021 21:15
@codecov
Copy link

codecov bot commented Oct 22, 2021

Codecov Report

Merging #425 (ccf3256) into main (816f73d) will increase coverage by 2.58%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
+ Coverage   60.60%   63.18%   +2.58%     
==========================================
  Files         120      120              
  Lines        2000     2010      +10     
==========================================
+ Hits         1212     1270      +58     
+ Misses        788      740      -48     
Impacted Files Coverage Δ
src/workflows/bidsConcatBetaTmaps.m 77.41% <87.50%> (+77.41%) ⬆️
src/utils/setGraphicWindow.m 50.00% <0.00%> (+50.00%) ⬆️
src/workflows/setUpWorkflow.m 100.00% <0.00%> (+100.00%) ⬆️

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 f58b6f1...ccf3256. Read the comment docs.

@Remi-Gau
Copy link
Contributor

Will add tests to this workflow.

@Remi-Gau
Copy link
Contributor

Got the the following error from the test suite (so unrelated to the code):

error: Error using MOxUnitFunctionHandleTestCase/run>trim_stack (line 91)
This should not happen

This should not happen <--- I mean seriously

I am still laughing.

@Remi-Gau
Copy link
Contributor

@CerenB Can you check that the minor changes I did, did not break anything?

@CerenB
Copy link
Collaborator Author

CerenB commented Nov 1, 2021

all good!

@Remi-Gau
Copy link
Contributor

Remi-Gau commented Nov 1, 2021

OK will merge and apply those changes to the dev branch?

@Remi-Gau Remi-Gau merged commit 56746e1 into cpp-lln-lab:main Nov 1, 2021
@CerenB
Copy link
Collaborator Author

CerenB commented Nov 1, 2021

OK will merge and apply those changes to the dev branch?

should work on dev branch as well, yes. I did not do anything which would be problematic.
That being said, is there anything I can do? e.g. PR to dev branch as well? (I'm currently working on main branch)

@Remi-Gau
Copy link
Contributor

Remi-Gau commented Nov 1, 2021

I already applied the patch to dev a couple of merge conflicts but nothing that I could not handle.

I have some failing tests so I will see what is up with that and also update to use the latest version of bids-matlab everywhere: this means that CPP-ROI will also start diverging so I will make a realease so we know which version the main branch of CPP_SPM is using. I don't your code to start breaking because of that.

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

Successfully merging this pull request may close these issues.

2 participants