Add comments and tidy _rewrite function. #59
Conversation
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
==========================================
+ Coverage 85.18% 85.26% +0.08%
==========================================
Files 19 19
Lines 1343 1344 +1
==========================================
+ Hits 1144 1146 +2
+ Misses 199 198 -1
Continue to review full report at Codecov.
|
|
Could you split the style fixes and code changes ? It is indeed hardly readable and it's one of the reason I move it to a separate package to at least remove any JuMP-specific stuff and try to make it simpler. I agree that it could still be improved with refactoring. |
I started fixing a bug for Julia 1.6, and got thoroughly confused with what this function was doing. Hopefully this is more readable.
|
It's |
By "not", you mean "now" ? |
Haha. Yes. |
| minus, | ||
| current_sum, | ||
| left_factors, | ||
| (esc(inner_factor),), |
There was a problem hiding this comment.
The bug for 1.6 is here. If inner_factor contains a sum(1 for _ = 1:0), then this evaluates it and throws an error. I'll think about what extra logic needs to be added to fix it in #60
I started fixing a bug for Julia 1.6, and got thoroughly confused
with what this function was doing. Hopefully this is more readable.