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
Combine nargs == 1 with nargs < 1 #136
Conversation
b434dea
to
5fee2e4
Compare
Codecov Report
@@ Coverage Diff @@
## master #136 +/- ##
==========================================
- Coverage 75.50% 75.49% -0.02%
==========================================
Files 15 15
Lines 4479 4472 -7
==========================================
- Hits 3382 3376 -6
+ Misses 1097 1096 -1
Continue to review full report at Codecov.
|
Hmm, that's not reassuring. Probably best not to merge this until that code has some coverage. |
5fee2e4
to
88c8ff4
Compare
The rank = 1 code path is just a loop unrolling of the general code path. That's not useful, lets not do it. Also replaces 0 with S(0), and makes nargs == 0 work for the heck of it by using the initial argument to reduce.
88c8ff4
to
0c235a4
Compare
Coverage drop is to be expected because this removes lines of code. Patch coverage is still high. |
The rank = 1 code path is just a loop unrolling of the general code path. That's not useful, lets not do it.
Also replaces 0 with S(0), and makes nargs == 0 work for the heck of it by using the initial argument to reduce.
First commit is #337, let's get that in first before merging this, so we can see if the lines this touches are covered.