-
Notifications
You must be signed in to change notification settings - Fork 62
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
Volumes scripts #947
Volumes scripts #947
Conversation
Hello @EmmaRenauld, Thank you for updating ! There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2024-04-03 12:52:56 UTC |
2c6a779
to
3409946
Compare
@frheault Could you please help us have a better doc of the scil_volume_b0_synthesis? Why does it take as input a b0 to create a b0? Thanks!! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #947 +/- ##
==========================================
+ Coverage 66.98% 67.65% +0.66%
==========================================
Files 396 397 +1
Lines 21411 21418 +7
Branches 3253 3249 -4
==========================================
+ Hits 14343 14490 +147
+ Misses 5752 5605 -147
- Partials 1316 1323 +7
|
32fb80d
to
f6d4e61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GTG but one quick comment
in_b0, 'b0_mask.nii.gz', | ||
'b0_synthesized.nii.gz', '-v') | ||
assert ret.success | ||
if have_tensorflow: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked François' way better... what made you change it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't work. If you didn't have tensorflow, the whole file was skipped, even the help test above, not this method only. You can see in the code coverage that the script had 0% coverage, so not even the help: https://app.codecov.io/gh/scilus/scilpy/blob/master/scripts%2Fscil_volume_b0_synthesis.py
We checked with Alex, this is the solution we found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Just two minor comments before merging.
33d19f6
to
ef3859a
Compare
Quick description
Cleaning the scripts in the volume column.
Type of change
Check the relevant options.
Provide data, screenshots, command line to test (if relevant)
...
Checklist