-
Notifications
You must be signed in to change notification settings - Fork 505
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
[MRG] Refactor: improve readibility in the convolutional module #709
[MRG] Refactor: improve readibility in the convolutional module #709
Conversation
Why do the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #709 +/- ##
==========================================
+ Coverage 97.06% 97.08% +0.01%
==========================================
Files 98 98
Lines 19955 19937 -18
==========================================
- Hits 19370 19355 -15
+ Misses 585 582 -3 |
Hi @framunoz, Thank you for doing some cleaning. I think that we just forgot to remove exceptions in these functions. Could you please remove them and adjust the tests ? |
Ok, I see why they methods raise these errors. In that functions they assign a value in a specific element (in the line |
Ok thanks for the reminder, we should fix that soon. That would be much appreciated if you have time to deal with that in another PR. |
Types of changes
Refactor the
ot.bregman._convolutional.py
module to improve the readability of the module. These includes:convol_img
function.Motivation and context / Related issue
There is a lot of duplicate code in this module, which can be encapsulated in functions to make it more modular.
How has this been tested (if it applies)
PR checklist