-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Replaced i with a dummy symbol with no assumptions for get_residue_factor #23507
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
✅ Hi, I am the SymPy bot (v167). 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.11. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
I will add a test case once all the checks are successful, |
The release note could be
|
Thank you @oscarbenjamin : ) |
It says Python version 3.8 or above is required for SymPy when I run bin/test_optional_dependencies.py I see in SymPy 1.11 dev documentation it is mentioned SymPy officially supports Python 3.8, 3.9, 3.10, and PyPy that means it no longer supports 3.7 now. |
The next release of sympy will not support 3.7 so "support" is already dropped on master. Probably you can just edit that line in the script though so that it still runs though. It isn't actually necessary to run that script though: if you have the dependencies installed then the ordinary test runner will run the same tests. The failures in the optional dependencies are only seen under Python 3.10 and 3.11 though so to reproduce them you need to install Python 3.10 or Python 3.11b1 (and also many of the optional dependencies). More importantly those failures are unrelated to this PR. Those jobs fail on the master branch which is why they are not required to pass for the PR to be merged. |
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) before after ratio
[77f1d79c] [b418faaa]
<sympy-1.10.1^0>
+ 97.5±0.3ms 181±0.8ms 1.86 sum.TimeSum.time_doit
Full benchmark results can be found as artifacts in GitHub Actions |
You may add test case now. The failing job doesn't matter. |
Thank you @oscarbenjamin for your help (I always thought release notes should be something like one liner by looking at the examples mentioned while writing comment for a PR, also now I think writing Release Notes is a good practice now instead of simply writing NO ENTRY if change is something other than documentation) @eagleoflqj I have added the test case now and updated Release Notes : ) |
A dummy symbol with no assumptions set for get_residue_factor
References to other Issues or PRs
Fixes #23491
Brief description of what is fixed or changed
In sympy/concrete/summations.py get_residue_factor(), 'i' was used with assumption set integer = True but the problem is that cot(S.Pi * i) gives zoo(complex infinity) therefore we need to use a dummy symbol with no assumption set to get_residue_factor to calculate the residue around the pole rather than evaluate the expression at the pole.
Other comments
Release Notes
integer=True
assumption set.