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
RecursionError in integrate fixed #21053
Conversation
✅ Hi, I am the SymPy bot (v161). 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.8. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
sympy/core/tests/test_arit.py
Outdated
e2 = -I*log((re(asin(5)) + I*im(asin(5)))/sqrt(re(asin(5))**2 + im(asin(5))**2))/pi | ||
assert integrate(f1, x) == \ | ||
-x**6/(6*asin(5)**4) - x**2*cosh(x + log(asin(5))) + 5*x**2/2 + 2*x*sinh(x + log(asin(5))) - 2*cosh(x + log(asin(5))) | ||
assert f1.is_zero is 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.
Looking at this again there's really just one bug here which is this:
expr = -I*log((re(asin(5)) + I*im(asin(5)))/sqrt(re(asin(5))**2 + im(asin(5))**2))/pi
expr.round() # hangs
We can just add a test for that here in this file. The integral test should go in sympy.integrals
somewhere.
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.
on it.
also in sympy.integrals
I'm adding it to sympy/interals/test_integrals.py
is it fine?
Does this also fix the other integral from the issue? |
No, the other one still gives |
Okay, since it doesn't fully fix the issue I've changed the OP to say "partial fix". If the OP says "fixes #12345" then when it is merged the issue will be closed but we don't want to close the issue unless it is fully fixed. I've also edited the release not to make it clear that this was a bug in This looks good so I'll merge it now. Thanks! |
References to other Issues or PRs
Partial fix for #21034
Brief description of what is fixed or changed
Applies the diff mentions by @oscarbenjamin here. This fixes one of the problems in the issue, i.e.
RecurstionError
caused due tonot x.is_extended_real
insympy/core/expr.py
Release Notes
Expr.round
that could lead to infinite recursion inintegrate
.