-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Added tests #18452 #19660
Added tests #18452 #19660
Conversation
Getting latest updates:
✅ Hi, I am the SymPy bot (v160). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.
Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. Click here to see the pull request description that was parsed.
|
Codecov Report
@@ Coverage Diff @@
## master #19660 +/- ##
==========================================
Coverage 75.671% 75.672%
==========================================
Files 658 658
Lines 170997 171250 +253
Branches 40350 40422 +72
==========================================
+ Hits 129396 129589 +193
- Misses 35935 35990 +55
- Partials 5666 5671 +5 |
@sachin-4099 could you please review |
sympy/series/tests/test_limits.py
Outdated
assert limit(abs(log(x))**x, x, 0) == 1 | ||
assert limit(abs(log(x))**x, x, 0, "+") == 1 |
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.
By default the dir
will be +
in this case, so the testcase having dir
explicitly specified as +
can be removed.
@@ -737,6 +737,10 @@ def test_issue_18378(): | |||
def test_issue_18442(): | |||
assert limit(tan(x)**(2**(sqrt(pi))), x, oo, dir='-') == Limit(tan(x)**(2**(sqrt(pi))), x, oo, dir='-') | |||
|
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.
Maintain two line spacing between the testcases.
Please make the minor changes then squash all work to a single commit, e.g. after making modifications then, while still on this branch do:
|
dd2dbca
to
0902a42
Compare
sympy/series/tests/test_limits.py
Outdated
@@ -738,6 +738,11 @@ def test_issue_18442(): | |||
assert limit(tan(x)**(2**(sqrt(pi))), x, oo, dir='-') == Limit(tan(x)**(2**(sqrt(pi))), x, oo, dir='-') | |||
|
|||
|
|||
def test_issue_18452(): | |||
assert limit(abs(log(x))**x, x, 0, "+") == 1 |
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.
This without +
was required.
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.
By default the
dir
will be+
in this case, so the testcase havingdir
explicitly specified as+
can be removed.
As I have mentioned here.
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.
oh yes, i see. I will change
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.
Since they are same, that's why +
is not required. It will be there by default in this case.
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.
I have corrected that.
0902a42
to
fba861b
Compare
As @smichr suggested it should be squashed to a single commit. You can connect with me on gitter, if you are facing difficulties squashing it to a single commit. |
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.
looks good
Looks good to me as well. This can be merged @smichr. |
References to other Issues or PRs
Brief description of what is fixed or changed
Added tests for #18452
Other comments
Release Notes
NO ENTRY