-
Notifications
You must be signed in to change notification settings - Fork 3
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
BUG: Use the appropriate index to extract b-values #72
BUG: Use the appropriate index to extract b-values #72
Conversation
b613d8d
to
729ff14
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
+ Coverage 68.75% 69.15% +0.40%
==========================================
Files 20 20
Lines 957 963 +6
Branches 121 119 -2
==========================================
+ Hits 658 666 +8
Misses 254 254
+ Partials 45 43 -2 ☔ View full report in Codecov by Sentry. |
729ff14
to
edcaf94
Compare
A round-trip test is probably missing to prevent issues like the one in 63bdf03. Do not have the bandwidth to do that now. |
f4e401c
to
3fd773a
Compare
Re: #72 (comment) There is a round-trip test in nifreeze/test/test_data_dmri.py Line 123 in 7322e93
But there is definitely something not working or missing there that is not being captured, as otherwise the test should have failed due to the bug that this PR addresses. I cannot debug this due to issue #73. |
Re #72 (comment): issue #79. Since fixing the issue seems to require a bit of work, including maybe some discussion or clarification as I am not convinced by the |
Use the appropriate index to extract b-values from the gradient data when serializing b-values.
Add gradient-related properties to DWI data class: add a property to retrieve the b-vectors and another one to retrieve the b-values. Co-authored-by: Oscar Esteban <[email protected]>
Do not transpose bvecs array when serializing: make the shape of the serialized data match what is written in/expected from commets: https://github.com/nipreps/nifreeze/blob/7322e93bc3eda2ebe5d2036b1f90c0f10596716c/src/nifreeze/data/dmri.py#L270 https://github.com/nipreps/nifreeze/blob/7322e93bc3eda2ebe5d2036b1f90c0f10596716c/src/nifreeze/data/dmri.py#L235 https://github.com/nipreps/nifreeze/blob/7322e93bc3eda2ebe5d2036b1f90c0f10596716c/src/nifreeze/data/dmri.py#L317 Conversely, transpose the bvals array when serializing: add a new axis before the existing axis to make the shape of the serialized data match what is written in/expected from comments: https://github.com/nipreps/nifreeze/blob/7322e93bc3eda2ebe5d2036b1f90c0f10596716c/src/nifreeze/data/dmri.py#L272 https://github.com/nipreps/nifreeze/blob/7322e93bc3eda2ebe5d2036b1f90c0f10596716c/src/nifreeze/data/dmri.py#L321
Simplify getting the root name for DWI serialization: leverage the `pathlib.Path` methods to remove all suffixes and avoid a two-step process relying on expected extensions.
45ee74c
to
25f6f27
Compare
Mark the `insert_b0` parameter as optional.
Add decimal places parameter for bval/bvec serialization. Provide some sensible default values. Change the default precision for b-values from 6 to 2.
e374a27
to
138573f
Compare
insert_b0
parameter as optional