karanphil commented 4 months ago

Quick description

Small PR to remove the need for a --peak_vals argument in scil_gradients_validate_correct.py when giving multiple peaks per voxel. Since the peaks we use (from scil_fodf_metrics.py) are already ordered, we don't need the values to find the maximum amplitude. Also, it would work with evecs.nii.gz from scil_dti_metrics.py, but the format is different and we always have access to evecs_v1.nii.gz anyway, so I did not add support for it.

We will also need to add a test for multiple peaks, once we have cleaner test data.

...

Type of change

Check the relevant options.

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Provide data, screenshots, command line to test (if relevant)

...

Checklist

[ ] My code follows the style guidelines of this project (run autopep8)
[ ] I added relevant citations to scripts, modules and functions docstrings and descriptions
[ ] I have performed a self-review of my code
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] My changes generate no new warnings
[ ] I moved all functions from the script file (except the argparser and main) to scilpy modules
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] New and existing unit tests pass locally with my changes

pep8speaks commented 4 months ago

Hello @karanphil, Thank you for updating !

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2024-03-05 16:01:35 UTC

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.40%. Comparing base (4c2848c) to head (fce54f8). Report is 48 commits behind head on master.

:exclamation: Current head fce54f8 differs from pull request most recent head eeeb65e. Consider uploading reports for the commit eeeb65e to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #928 +/- ## ========================================== + Coverage 69.17% 69.40% +0.22% ========================================== Files 389 389 Lines 20963 20993 +30 Branches 3233 3230 -3 ========================================== + Hits 14501 14570 +69 + Misses 5132 5109 -23 + Partials 1330 1314 -16 ``` | [Components](https://app.codecov.io/gh/scilus/scilpy/pull/928/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | Coverage Δ | | |---|---|---| | [Scripts](https://app.codecov.io/gh/scilus/scilpy/pull/928/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | `71.84% <100.00%> (+0.24%)` | :arrow_up: | | [Library](https://app.codecov.io/gh/scilus/scilpy/pull/928/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scilus) | `65.25% <ø> (+0.20%)` | :arrow_up: |

scilus / scilpy