Skip to content
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 heuristics kwarg for Limit.doit solving big-oh notation error, added tests #15512

Closed
wants to merge 1 commit into from

Conversation

avishrivastava11
Copy link
Contributor

@avishrivastava11 avishrivastava11 commented Nov 19, 2018

Added heuristics kwarg for limit.doit solving big-oh notation error, added tests

Fixes #9917
Note : I've used the changes implemented by Sergey Kirpichev in diofant here

Release Notes

  • series
    • Added heuristics kwarg for limit.doit
    • Added tests

@sympy-bot
Copy link

sympy-bot commented Nov 19, 2018

Hi, I am the SymPy bot (v134). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.4.

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.

Added heuristics `kwarg` for `limit.doit` solving big-oh notation error, added tests

Fixes #9917
Note : I've used the changes implemented by Sergey Kirpichev in diofant [here](https://github.com/diofant/diofant/commit/8b7a103fb4baeefb9efc1241452e22240eb04af0)
#### Release Notes

<!-- BEGIN RELEASE NOTES -->
* series
   * Added heuristics `kwarg` for `limit.doit`
   * Added tests 
<!-- END RELEASE NOTES -->

from sympy.abc import n
from sympy import O, oo, sin

assert O(n*sin(n) + 1, (n, oo)) == O(n*sin(n) + 1, (n, oo))
Copy link
Contributor

@normalhuman normalhuman Nov 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line, as well as those below it, do not actually test the behavior of Order. Same object on both sides of equality, so equality is assured to hold regardless of what that object is. So it holds in current master branch as well, where both sides return O(n*sin(n)).

Perhaps you meant to test

O(n*sin(n)+1, (n, oo)).expr == n*sin(n) + 1

which would indeed verify that the expression returned by Order constructor is what you expect.

@oscarbenjamin
Copy link
Collaborator

Are you still working on this?

@oscarbenjamin oscarbenjamin added the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Jan 28, 2019
@avishrivastava11
Copy link
Contributor Author

@oscarbenjamin

Are you still working on this?

Forgive me, I was a bit preoccupied with some work. I'm getting onto it.

@oscarbenjamin
Copy link
Collaborator

There's no need to be forgiven :). We all have other things to do as well!

I was just asking so it would be clear to anyone else thinking about working on this.

@czgdp1807 czgdp1807 added Please take over and removed PR: author's turn The PR has been reviewed and the author needs to submit more changes. labels Mar 9, 2020
@praneethratna
Copy link
Contributor

This PR can be closed now since issue #9917 has been fixed and tests for it are added in #22373.

@eagleoflqj eagleoflqj closed this May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

O(n*sin(n) + 1, (n, oo)) returns O(n*sin(n), (n, oo))
7 participants