-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
diophantine: note re parametric soln; remove gcd #17348
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 (v147). 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.5. 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.
Update The release notes on the wiki have been updated. |
|
Unless there are suggestions for changes I plan nothing else for this. |
Codecov Report
@@ Coverage Diff @@
## master #17348 +/- ##
=============================================
+ Coverage 74.603% 74.692% +0.088%
=============================================
Files 627 631 +4
Lines 162864 163167 +303
Branches 38214 38282 +68
=============================================
+ Hits 121503 121874 +371
+ Misses 35979 35959 -20
+ Partials 5382 5334 -48 |
|
Edited release note entry. |
sympy/solvers/diophantine.py
Outdated
| >>> parametrize_ternary_quadratic(4*x**2 + 2*y**2 - 3*z**2) | ||
| (2*p**2 - 3*q**2, -4*p**2 + 12*p*q - 6*q**2, 4*p**2 - 8*p*q + 6*q**2) | ||
| More than one solution can be represented by a given ``p`` and |
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.
Is the extra indentation necessary?
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.
My feeling was that this is more of a parenthetical comment and I wanted it to appear as such. Perhaps putting it under Notes would be better?
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.
That might be better unless the presence of examples will confuse Sphinx.
|
This should clarify the nature of the parametrization, pointing out that primitive solutions are not necessarily among those parametrized by integers. |
References to other Issues or PRs
closes #11697
Brief description of what is fixed or changed
A docstring has been updated to indicate the relationship between the parameterized solution and the actual solution: the parameterized solution does not produce coprime results from coprime input.
I also noticed that symbolic gcd was not being handled so updated the routine that removes the gcd of parameterized solutions.
Other comments
Release Notes