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

workflows : warn user for strange b0 threshold #1621

Merged
merged 4 commits into from Nov 13, 2018

Conversation

Projects
None yet
3 participants
@skoudoro
Copy link
Member

skoudoro commented Aug 21, 2018

I just added a warning when b0 threshold is smaller than all b-values.

I had to duplicate the code many times. It is not the first time. This means a new PR is needed to create a parent class (ReconstWorkflow) and avoid the redundancy.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 21, 2018

Codecov Report

Merging #1621 into master will increase coverage by 0.28%.
The diff coverage is 98.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1621      +/-   ##
==========================================
+ Coverage   87.34%   87.63%   +0.28%     
==========================================
  Files         246      248       +2     
  Lines       32687    34156    +1469     
  Branches     3555     3744     +189     
==========================================
+ Hits        28549    29931    +1382     
- Misses       3276     3335      +59     
- Partials      862      890      +28
Impacted Files Coverage Δ
dipy/workflows/tests/test_reconst_csa_csd.py 96.2% <100%> (+0.68%) ⬆️
dipy/workflows/tests/test_reconst_mapmri.py 92.64% <100%> (+1.73%) ⬆️
dipy/workflows/tests/test_reconst_dki.py 97.5% <100%> (+0.44%) ⬆️
dipy/workflows/reconst.py 81.58% <90.9%> (+0.27%) ⬆️
dipy/data/fetcher.py 42.36% <0%> (-2.3%) ⬇️
dipy/core/gradients.py 90.51% <0%> (-1.6%) ⬇️
dipy/core/graph.py 73.8% <0%> (-1.2%) ⬇️
dipy/io/tests/test_dpy.py 92.85% <0%> (-0.48%) ⬇️
dipy/io/streamline.py 94% <0%> (-0.45%) ⬇️
dipy/reconst/qtdmri.py 93.94% <0%> (ø)
... and 5 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 94a9e2d...a5f479c. Read the comment docs.

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Aug 22, 2018

Nice. Any chance to get a test of this behavior?

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Aug 22, 2018

  • What is the good way to test logging module @arokem?

  • When we enter into this specific condition, an error is always generated later in the code. It seems I should convert this warning into an error. What do you think @arokem @Garyfallidis?

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Oct 29, 2018

I added the test so this one is ready to go @arokem

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Nov 13, 2018

Hi @arokem, can you check this one?

Thank you

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Nov 13, 2018

Yep. Ready to go!

@arokem arokem merged commit 756b519 into nipy:master Nov 13, 2018

4 checks passed

codecov/patch 98.11% of diff hit (target 87.34%)
Details
codecov/project 87.63% (+0.28%) compared to 978ab60
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Nov 13, 2018

Thanks!

@skoudoro skoudoro deleted the skoudoro:warning-cli branch Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment