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
solve
now acknowledges dict=True
for Relational args
#17723
Conversation
Solve method will return a list of dictionaries for Relational args. Currently, it only supports solution with `And`, `Or` only.
✅ Hi, I am the SymPy bot (v149). 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. |
What should |
Codecov Report
@@ Coverage Diff @@
## master #17723 +/- ##
=============================================
+ Coverage 74.689% 74.742% +0.052%
=============================================
Files 634 633 -1
Lines 165811 166107 +296
Branches 38998 39077 +79
=============================================
+ Hits 123844 124152 +308
+ Misses 36480 36444 -36
- Partials 5487 5511 +24 |
Someone, please help me with, https://travis-ci.org/sympy/sympy/jobs/600244489 |
Figured out. Thanks. |
Since, all the tests are passing, the PR will be merged after 24 hours, if no critical objections are raised within the mentioned period. Thanks. |
(x, y, z,), dict=True) == [{x: 1, y: -1, z: 1}, {x: 1, y: 1, z: 1}, | ||
{x: 2, y: -1, z: 1}, {x: 2, y: 1, z: 1}, | ||
{x: 3, y: -1, z: 1}, {x: 3, y: 1, z: 1}] | ||
assert solve(Gt(x, 0), (x,), dict=True) == And(Lt(0, x), Lt(x, oo)) |
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.
@oscarbenjamin this last test is likely what is throwing the UserWarning (?)
if flags.get('dict', False): | ||
solution = reduce_inequalities(f, symbols=symbols) | ||
if isinstance(solution, Equality): | ||
return [{solution.lhs: solution.rhs}] |
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 looks bad, were are enumerating over things. We probably shouldn't return one of them...
Certainly no criticism intended to author of this. In particular I see other suspicious looking returns up above too! Line 973 and 975 for example...
@cbm755 Let me know if a follow up PR is needed for this. |
I'm not sure, still learning here myself.... I guess it may get reverted for 1.5. But I like your idea of having dict=True do its thing when it can! But maybe its needs some revision... |
Refer, #11661 (comment) |
reverted changes of #17723 for 1.5
References to other Issues or PRs
Fixes #13534
Closes #13540
Brief description of what is fixed or changed
Other comments
Release Notes
solve
now acknowledgesdict=True
for Relational args