-
-
Notifications
You must be signed in to change notification settings - Fork 46.6k
added type hints and doctests to arithmetic_analysis/bisection.py #2241
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
Conversation
continuing in line with #2128
if __name__ == "__main__": | ||
print(bisection(f, 1, 1000)) |
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.
Why remove this code?
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.
Hmm, I thought I was supposed to move all tests into the doctest. I guess I misunderstood. I can put it back in for sure.
arithmetic_analysis/bisection.py
Outdated
print("couldn't find root in [a,b]") | ||
return | ||
# then this algorithm can't find the root | ||
print("couldn't find root in [", a, ",", b, "]") |
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.
CONTRIBUTING.md says that algorithmic functions should not print. Please raise an exception instead of returning None.
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.
OK, got it.
Put back print statement at the end, replaced algorithm's print statement with an exception.
Hey @spamegg1, TravisCI finished with status TravisBuddy Request Identifier: c68a55b0-d005-11ea-81e3-a59d363db46c |
Removed unnecessary type import "Optional"
arithmetic_analysis/bisection.py
Outdated
>>> bisection(lambda x: x ** 2 - 4 * x + 3, 4, 1000) | ||
Traceback (most recent call last): | ||
... | ||
Exception: could not find root in given interval. |
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.
ValueError please, not generic Exception.
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.
OK.
Replaced generic Exception with ValueError.
fixed doctests
My first time ever going through this CI process, I apologize for the mistakes and my too many commits. Thank you for your patience. |
Travis tests have failedHey @spamegg1, TravisBuddy Request Identifier: 1d5a89d0-d008-11ea-81e3-a59d363db46c |
Getting conflicting messages from Travis. It says tests failed, mentions a log but no link to it? And all checks have passed? I got the green check mark. Anyway, let me know what I should do. |
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.
Awesome work! Thanks.
Whew. That was quite stressful! I'll need a bit more practice with it. Thanks a lot, @cclauss |
May you explain please what was the issue ? |
…eAlgorithms#2241) * added type hints and doctests to arithmetic_analysis/bisection.py continuing in line with TheAlgorithms#2128 * modified arithmetic_analysis/bisection.py Put back print statement at the end, replaced algorithm's print statement with an exception. * modified arithmetic_analysis/bisection.py Removed unnecessary type import "Optional" * modified arithmetic_analysis/bisection.py Replaced generic Exception with ValueError. * modified arithmetic_analysis/bisection.py fixed doctests
continuing in line with #2128
Describe your change:
Checklist: