nipreps / dmriprep

dMRIPrep is a robust and easy-to-use pipeline for preprocessing of diverse dMRI data. The transparent workflow dispenses of manual intervention, thereby ensuring the reproducibility of the results.
https://www.nipreps.org/dmriprep
Apache License 2.0
63 stars 24 forks source link

ENH: Do not raise error in all instances of b-vecs/vals inconsistencies #100

Closed oesteban closed 4 years ago

oesteban commented 4 years ago

We raised a ValueError whenever the number of b-vals and all-zero b-vecs didn't match. Few lines later, we would overwrite nonzero b-vecs to match the b-vals, but that step was never reached. This PR allows the function to just throw a warning instead.

Resolves: #82

pull-assistant[bot] commented 4 years ago
Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     ENH: Do not raise error in all instances of b-vecs/vals inconsistencie...

Powered by Pull Assistant. Last update d1b93aa ... d1b93aa. Read the comment docs.

oesteban commented 4 years ago

This is working locally. Although I'm not super-satisfied with the logger I've assigned this warning, I think it will do for now.

codecov[bot] commented 4 years ago

Codecov Report

Merging #100 into master will decrease coverage by 0.82%. The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
- Coverage   51.64%   50.81%   -0.83%     
==========================================
  Files          21       21              
  Lines        1218     1222       +4     
  Branches      161      162       +1     
==========================================
- Hits          629      621       -8     
- Misses        578      586       +8     
- Partials       11       15       +4     
Impacted Files Coverage Δ
dmriprep/utils/vectors.py 90.24% <60.00%> (-4.14%) :arrow_down:
dmriprep/workflows/dwi/base.py 30.00% <0.00%> (-16.67%) :arrow_down:

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 2802c50...d1b93aa. Read the comment docs.

oesteban commented 4 years ago

Okay, at the risk of merging some suboptimal code I'll go ahead so that we have more stuff to revise at tomorrow's meeting (also I'm feeling some urgency of fixing master for this new dataset so that I can continue with #97).