[sympy] WIP - issue 3551 (#1691)
Ronan Lamy
You can merge this Pull Request by running:
git pull https://github.com/rlamy/sympy real-solve
Or view, comment on, or merge it at:
https://github.com/sympy/sympy/pull/1691
Commit Summary
- check_assumptions(): handle only bool values.
- check_assumptions(): Don't use new-style assumptions.
File Changes
- M sympy/solvers/solvers.py (13)
- M sympy/solvers/tests/test_solvers.py (2)
Patch Links
—
Reply to this email directly or view it on GitHub.
Aaron Meurer
So are the dicts actually sanitized to bool/None?
Aaron Meurer
I thought new style assumptions were supported by just passing the assumptions to the solve command. Does this remove that?
Ronan Lamy
So are the dicts actually sanitized to bool/None?
Er... there's nothing to sanitize, the function is only used with assumption dicts, which only contain bool/None.
I thought new style assumptions were supported by just passing the assumptions to the solve command. Does this remove that?
No, it's unrelated. What I removed was an attempt to evaluate old-style assumptions using new-style ones, but based only on the info provided by the old system...
Ronan Lamy
This is now ready for merging, I think.
Aaron Meurer
I mean, you removed an instance of real=1. Would that have been changed to True in the final dict?
Ronan Lamy
The old version of the test now fails, if that's what you're asking. I think it's a feature: if you start thinking that you can stick integers and whatnot into FactKBs (i.e. assumption dicts), you will just break stuff.
Aaron Meurer
That's what I mean by sanitize. real=whatever should be converted to {'real': bool(whatever)} in the dict. I agree it's bad style, but people do it to save typing.
Ronan Lamy
They would save a lot more typing if they used x.is_real instead of check_assumptions(x, real=1).
Aaron Meurer
SymPy Bot Summary: 
Passed after merging rlamy/real-solve (4dd39fe) into master (14452a4).
Python 2.5.0-final-0: pass
Python 2.6.6-final-0: pass
Python 2.7.2-final-0: pass
Python 2.6.8-final-0: pass
Python 2.7.3-final-0: pass
PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
Python 3.2.2-final-0: pass
Python 3.3.0-final-0: pass
Python 3.2.3-final-0: pass
Python 3.3.0-final-0: pass
Python 3.3.0-final-0: pass
**Sphinx 1.1.3:** pass
Aaron Meurer
I mean
In [1]: a = Symbol('a', real=1) In [3]: a.assumptions0 Out[3]: {commutative: True, complex: True, hermitian: True, imaginary: False, real: 1} In [4]: a.is_real Out[4]: 1
We clearly need to sanitize, since code that uses the assumptions can't just use the if not a.is_real idiom, since that doesn't do the right thing with None. Either that or raise TypeError when the assumption is not True, False, or None, but that seems unnecessarily restrictive.
Aaron Meurer
Anyway, it's a separate issue. I am +1 to what's here.
Julien Rioux
SymPy Bot Summary: Passed after merging rlamy/real-solve (1c7b658) into master (501534b).
PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
Python 2.7.2-final-0: pass
Python 3.2.1-final-0: pass
Sphinx 1.1.3: pass
Docs build command: make clean && make html-errors && make latex && cd _build/latex && xelatex sympy-*.tex
Ronan Lamy
I agree that sanitising FactKBs on creation would be better. However, I think that there are a few places (secondquant?) that put random stuff into the assumptions. They would need to be fixed first.
I guess I can merge this now. Thanks for the review.
Ronan Lamy
Merged #1691.
Aaron Meurer
I opened https://code.google.com/p/sympy/issues/detail?id=3567 for this. It occurs to me that probably quite a few people are (ab)using assumptions to tack on arbitrary information to Symbols. If these are still True/False assumptions, these are probably OK (it just means that we support arbitrary is_something assumptions even if the system doesn't explicitly know about them), but if it also means tacking on arbitrary objects with the assumption, the correct thing to do is to create a subclass. We could also consider adding some simple attribute to Symbol to let people do this for very simple cases.