Skip to content
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

MT/ihMT scripts: cleanup and adding B1+ correction #870

Merged
merged 57 commits into from
Feb 21, 2024

Conversation

karanphil
Copy link
Contributor

@karanphil karanphil commented Dec 15, 2023

Quick description

Hi,

I added B1+ field inhomogeneity correction to scil_mti_maps_MT.py and scil_mti_maps_ihMT.py. Doing so, I also cleaned up those scripts and the functions they call in scilpy.reconst.mti. I also added a script (scil_mti_adjust_B1_header.py) to correct the bug in the B1 map header, with the scaling issue. There is now a new correction method for B1+, called "model_based" and it requires .mat files obtained with Matlab at the moment. I will send these to the reviewers.

I tested the scripts and they produce the same results as before (without B1 correction). @Manonedde, you should still test them to make sure everything works fine.

Feel free to comment on the choices I made ;)

This PR doesn't have to be merged quickly.
...

Type of change

Check the relevant options.

  • 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

@karanphil karanphil self-assigned this Dec 15, 2023
@pep8speaks
Copy link

pep8speaks commented Dec 15, 2023

Hello @karanphil, Thank you for updating !

Line 318:80: E501 line too long (82 > 79 characters)
Line 319:80: E501 line too long (81 > 79 characters)

Comment last updated at 2024-02-21 16:05:10 UTC

@karanphil karanphil removed their assignment Dec 15, 2023
@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 53 lines in your changes are missing coverage. Please review.

Comparison is base (41956e7) 1.95% compared to head (1008f29) 68.17%.
Report is 25 commits behind head on master.

❗ Current head 1008f29 differs from pull request most recent head 8cb2a5b. Consider uploading reports for the commit 8cb2a5b to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #870       +/-   ##
===========================================
+ Coverage    1.95%   68.17%   +66.22%     
===========================================
  Files         384      387        +3     
  Lines       13876    20865     +6989     
  Branches     1754     3213     +1459     
===========================================
+ Hits          271    14225    +13954     
+ Misses      13561     5358     -8203     
- Partials       44     1282     +1238     
Components Coverage Δ
Scripts 70.85% <82.74%> (+68.93%) ⬆️
Library 63.56% <71.12%> (+61.58%) ⬆️

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

scripts/scil_mti_maps_ihMT.py Show resolved Hide resolved
scripts/scil_mti_maps_ihMT.py Show resolved Hide resolved
scripts/scil_mti_maps_ihMT.py Show resolved Hide resolved
@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

Copy link
Contributor

@Manonedde Manonedde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some details for Warning messages, otherwise it's good for me!

INFO:root:Scilpy version: 1.7.dev0
WARNING:root:Repetition time found in mton.json does not seem to be given in seconds. MTsat results might be affected.
INFO:root:Loaded mton_Warped.nii.gz of shape (180, 240, 240) and data_type float64.
INFO:root:Loaded mtoff_Warped.nii.gz of shape (180, 240, 240) and data_type float64.
INFO:root:Loaded mt_t1.nii.gz of shape (180, 240, 240) and data_type int16.

scripts/scil_mti_maps_MT.py Show resolved Hide resolved
scripts/scil_mti_maps_MT.py Outdated Show resolved Hide resolved
@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@arnaudbore arnaudbore merged commit 89ac8ce into scilus:master Feb 21, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants