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

ENH: b0 reference and skullstrip workflow #50

Merged
merged 41 commits into from Jan 15, 2020
Merged

ENH: b0 reference and skullstrip workflow #50

merged 41 commits into from Jan 15, 2020

Conversation

josephmje
Copy link
Collaborator

No description provided.

@pull-assistant
Copy link

pull-assistant bot commented Jan 13, 2020

Score: 0.79

Best reviewed: commit by commit


Optimal code review plan (12 warnings)

enh: initial implementation of b0 reference and skull-stripping workfl...

dmriprep/interfaces/images.py 52% changes removed in fix: various nodes f...

dmriprep/workflows/dwi/util.py 42% changes removed in enh: split out repor...

     Update dmriprep/workflows/dwi/util.py

     Update dmriprep/interfaces/images.py

     Update dmriprep/interfaces/images.py

     update to use inputs according to #26

initial b0 registration adapted from niworkflows

dmriprep/workflows/dwi/util.py 73% changes removed in add mcflirt and fix ...

fix masking

dmriprep/interfaces/images.py 51% changes removed in fix: various nodes f...

dmriprep/workflows/dwi/util.py 47% changes removed in add mcflirt and fix ...

update docs and doctests

dmriprep/workflows/dwi/util.py 50% changes removed in docs: revise docstri...

     add workflow test

     add pytest fixtures

     docs: revise docstrings for napoleon parsing

add mcflirt and fix rescale_b0 inputs and outputs

dmriprep/workflows/dwi/util.py 50% changes removed in fix: amend previous ...

     fix imputs again

joining gradtable and skullstrip

...riprep/config/reports-spec.yml 62% changes removed in fix: remove diffusio...

...prep/workflows/dwi/__init__.py 60% changes removed in sty: fix all dunder

add fake metadata for docs

dmriprep/workflows/dwi/base.py 50% changes removed in fix: remove diffusio...

     fix typo

     remove outputs file

add summary and validation datasinks

dmriprep/workflows/dwi/base.py 68% changes removed in enh: split out repor...

fix layout grabbing

dmriprep/workflows/dwi/base.py 50% changes removed in fix doctests

     fix doctests

     fix flake8 issues

     ordering is sensitive to spaces

     add fake files for docs

     add missing arg

     add missing arg

     fix docstring

     fix doctests

     fix typos

     Apply suggestions from code review

address Oscar's comments

dmriprep/interfaces/images.py 50% changes removed in fix: various nodes f...

     fix tests

     remove automodules

     enh: connect workflow

     fix: enable execution of the diffusion workflow

fix: various nodes failing

dmriprep/workflows/dwi/base.py 50% changes removed in fix: amend previous ...

     fix: amend previous commit - left bad connection name

     fix: amend previous commit - left bad connection name

     fix: remove diffusionsummary

enh: split out reports to a new module

...riprep/config/reports-spec.yml 83% changes removed in fix indentation in r...

     sty: fix all dunder

     fix indentation in reports config

Powered by Pull Assistant. Last update ac8753e ... 51da317. Read the comment docs.

@pep8speaks
Copy link

pep8speaks commented Jan 13, 2020

Hello @josephmje, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 dmriprep.

Comment last updated at 2020-01-15 16:46:44 UTC

@codecov
Copy link

codecov bot commented Jan 14, 2020

Codecov Report

Merging #50 into master will decrease coverage by 3.14%.
The diff coverage is 38.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   55.51%   52.37%   -3.15%     
==========================================
  Files          11       16       +5     
  Lines         762      907     +145     
  Branches      110      114       +4     
==========================================
+ Hits          423      475      +52     
- Misses        338      431      +93     
  Partials        1        1
Impacted Files Coverage Δ
dmriprep/interfaces/reports.py 60.71% <ø> (-0.4%) ⬇️
dmriprep/workflows/dwi/__init__.py 100% <100%> (ø)
dmriprep/config/__init__.py 100% <100%> (ø) ⬆️
dmriprep/workflows/base.py 29.57% <25%> (-1.31%) ⬇️
dmriprep/workflows/dwi/util.py 27.02% <27.02%> (ø)
dmriprep/workflows/dwi/outputs.py 38.46% <38.46%> (ø)
dmriprep/interfaces/images.py 41.17% <41.17%> (ø)
dmriprep/workflows/dwi/base.py 41.66% <41.66%> (ø)
... and 3 more

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 5bd7fcf...51da317. Read the comment docs.

Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Had a quick pass so that you can get things moving along

dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
josephmje and others added 26 commits January 14, 2020 22:38
initial port from niworkflows

add initial pre-mask step

fix doc

add pre_mask

address review comments

add period

extract b0s first

fix function name typo

initial port from niworkflows

add initial pre-mask step

fix doc

add pre_mask

address review comments

add period

extract b0s first

fix function name typo

remove MatchHeader
Co-Authored-By: Oscar Esteban <code@oscaresteban.es>
Co-Authored-By: Oscar Esteban <code@oscaresteban.es>
Co-Authored-By: Oscar Esteban <code@oscaresteban.es>
@josephmje
Copy link
Collaborator Author

@oesteban
Copy link
Member

I tried a fix to the niworkflows problem and then rolled back. I'm going to go for it. Thanks for this precious contribution!

@oesteban oesteban merged commit c695f4c into nipreps:master Jan 15, 2020
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.

None yet

3 participants