-
Notifications
You must be signed in to change notification settings - Fork 48
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
New version overlap.py to fix_factorial2 #309 with unittest named test_factorial2 #316
Conversation
Test file for the document mentioned
Handling of int float and arrays
for more information, see https://pre-commit.ci
Thanks for the pull request. I'll review it and plan to merge it after #315 has been merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, thank you for this contribution! I've added a few quick comments. I'll check in more detail before merging.
for more information, see https://pre-commit.ci
@tovrstra We have followed all your comments and changed the unittest to pytest_approx. We apologize for the mistake; we were basing our work on your previous comment on issue #308, which led us to believe we should use unittest. The test_factorial2 has been updated and posted in this pull request. If you don't see it, please let us know, and we will create a new pull request. Thank you, and please let us know if you merge it. |
I just merged #315 and well merge your pull request into the current main branch to enable the Ruff linters on this pull request. |
It seems I cannot push the merge with the current main into your pull request. This may be related to the fact that you made a pull request from the main branch of your repository, but I'm not sure. (See branch protection rules in the GitHub documentation.) It is in general not recommended to develop on the main branch of a fork, because that makes it impossible to sync your fork with the original repository. You may have to remove and recreate your fork to fix this. Could you create a new pull request starting from the current main? The current main has a series of new linters enabled, which prevent many anti-patterns from being committed. When doing so, could you run the following in your local repository before making a commit? pre-commit install That way, you run the linters before committing to your git branch. Thank you! |
@LudoRNLT and I think that we have solved the fix for issue #309 described in issue #308 for this pull request. As suggested, we have added a unit test, and our code is passing both the

test_factorial2
andtest_overlap
unit tests. Please let us know if this is good and if it will be merged. Thank you.