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

[FIX] Add quiet and debug options to t2smap #123

Merged
merged 2 commits into from
Sep 3, 2018

Conversation

emdupre
Copy link
Member

@emdupre emdupre commented Sep 2, 2018

Changes proposed in this pull request:

  • HOTFIX ! Adds missing arguments to t2smap.
  • This was previously unnoticed since the t2smap workflow was only called in the context of the tedana workflow

@codecov
Copy link

codecov bot commented Sep 2, 2018

Codecov Report

Merging #123 into master will decrease coverage by 0.04%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
- Coverage   49.72%   49.68%   -0.05%     
==========================================
  Files          32       32              
  Lines        2035     2037       +2     
==========================================
  Hits         1012     1012              
- Misses       1023     1025       +2
Impacted Files Coverage Δ
tedana/workflows/t2smap.py 70.83% <33.33%> (-2.03%) ⬇️

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 e7f04ff...1a9cac3. Read the comment docs.

@emdupre
Copy link
Member Author

emdupre commented Sep 2, 2018

Merging for nipreps/fmriprep#1263, unless @tsalo has any objections !

@tsalo
Copy link
Member

tsalo commented Sep 2, 2018

Do debug and quiet need to be popped out of options before they're fed into t2smap_workflow or will it work as is?

@emdupre
Copy link
Member Author

emdupre commented Sep 2, 2018

Sorry, could you confirm what you mean by "popped out" ? I'm not sure I'm following..

@tsalo
Copy link
Member

tsalo commented Sep 2, 2018

I just meant that they might need to be removed from the options dictionary somehow before being fed into t2smap_workflow. Or maybe they could be added as arguments to t2smap_workflow?

@tsalo
Copy link
Member

tsalo commented Sep 2, 2018

To clarify, I get this error if I try running t2smap from the command line with the changes:

Traceback (most recent call last):
  File "/Users/tsalo/anaconda/envs/python3/bin/t2smap", line 11, in <module>
    load_entry_point('tedana', 'console_scripts', 't2smap')()
  File "/Users/tsalo/Documents/tsalo/tedana/tedana/workflows/t2smap.py", line 214, in _main
    t2smap_workflow(**vars(options))
TypeError: t2smap_workflow() got an unexpected keyword argument 'debug'

@emdupre
Copy link
Member Author

emdupre commented Sep 2, 2018

Oh I see, thanks ! Yes, you're right, I'll change this.

@emdupre
Copy link
Member Author

emdupre commented Sep 2, 2018

OK, I think this is fixed. I'll merge and try moving forward with the fmriprep PR pinned to this commit !

@emdupre emdupre merged commit cb01d72 into ME-ICA:master Sep 3, 2018
@emdupre
Copy link
Member Author

emdupre commented Jun 3, 2019

@all-contributors add @tsalo for reviewing and tests, please !

@allcontributors
Copy link
Contributor

@emdupre

We had trouble processing your request. Please try again later.

@allcontributors
Copy link
Contributor

@emdupre

I've put up a pull request to add @tsalo! 🎉

@emdupre
Copy link
Member Author

emdupre commented Jun 3, 2019

@all-contributors add @tsalo for reviewing pull requests, please ?

@allcontributors
Copy link
Contributor

@emdupre

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

@emdupre
Copy link
Member Author

emdupre commented Jun 3, 2019

@all-contributors add @tsalo for pull requests review, please ?

@allcontributors
Copy link
Contributor

@emdupre

I've updated the pull request to add @tsalo! 🎉

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