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

Add patch_radius parameter to Patch2Self denoise workflow #2792

Merged
merged 4 commits into from
Apr 27, 2023

Conversation

pcamach2
Copy link
Contributor

Addresses #2791

Copy link
Member

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Nice! Thank you for this @pcamach2!

it is missing the part to check if it is a list of int or just a int. You can look at this example:

if isinstance(patch_radius, list) and len(patch_radius) == 1:
.

Also, it would be great if you could update the test :

def test_patch2self_flow():

Thank you!

dipy/workflows/denoise.py Outdated Show resolved Hide resolved
@skoudoro skoudoro linked an issue Apr 26, 2023 that may be closed by this pull request
@pcamach2
Copy link
Contributor Author

Thank you for your review, @skoudoro!

Please let me know if anything else needs adjusting for this PR

Copy link
Member

@skoudoro skoudoro left a 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!

Waiting for the CI's to finish, then I will go ahead and merge it.

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Merging #2792 (a44bf30) into master (976217e) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2792   +/-   ##
=======================================
  Coverage   81.48%   81.49%           
=======================================
  Files         143      143           
  Lines       20049    20051    +2     
  Branches     3191     3192    +1     
=======================================
+ Hits        16337    16340    +3     
+ Misses       2906     2904    -2     
- Partials      806      807    +1     
Impacted Files Coverage Δ
dipy/workflows/denoise.py 82.17% <0.00%> (-1.67%) ⬇️

... and 1 file with indirect coverage changes

@skoudoro
Copy link
Member

ok, all green, merging. Thanks again @pcamach2 !

@skoudoro skoudoro merged commit 1f5f7e2 into dipy:master Apr 27, 2023
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Patch2Self CLI
2 participants