bpo-35431: Add a function computing binomial numbers to the math module by KellerFuchs · Pull Request #11018 · python/cpython (original) (raw)
#11414 was already conflict free and had function name as comb, you needlessly worked by naming and renaming etc. The changes that you made in last 7-8 commits were already there. You were already invited as collaborator.
Regarding “the last 7-8 commits” you talk of, I have no idea what you are going off, given that all I added since the initial merge 4 days ago (before you commented on this PR or added me as a collaborator) were commits afb3d36 (trivial changes, fixes the testsuite) and e4942bf (trivial rename).
Not sure either what you mean by “conflict-free” since there's no conflict between this and master (nor are there ever been).
On the other hand, recreating all that work on your PR would indeed be unnecessary work.
The test method names still need to be changed to testComb
Not sure that's entirely necessary, but I just did that to avoid having an argument, since it's a quick search-and-replace.
The factorial test suite has 3 functions and is pretty old, and one of those methods needed a decorator. 6+ methods are bound to cause maintenance problems in the future.
I am absolutely counfunded why you would expect a single function, testing a bunch of unrelated properties, to be more maintainable than separate tests.
I assure you, cramming everything into a single test does not make for maintainable testsuites.
Some exception tests are still missing
Which ones? I am pretty sure I kept them all when merging both testsuites.
"combinations" isstill written everywhere except tests. Renaming and writing them all will be useless effort as all that is already done.
Clearly not:
$ grep -i combinations Modules/mathmodule.c Lib/test/test_math.py
[Empty output, no match found]
I would still insist you to work on that PR or merge that branch here again. Or maybe make the changes in that branch and then merge it with this.
All the things you mentionned existed all along (with the exception of a single search-and-replace for test names), so I'm not sure why you want me to recreate all that on the other PR.
There were just some changes in 2 or 3 comments so it will be much more simpler to work on that branch.
There were changes to the testsuite, which was the whole point of that merge in the first place.