Repository: sympy/sympy
Pull request: check_assumptions: Improvement in the return value of the function
closes: #12146
 


Problem statement:
Improvement in check_assumptions in solvers
check_assumption just returns True or False. So rather than just returning False if the assumption about the expression that is incorrect is also returned the user would know which of all the set of assumptions is False. This would probably help him determining the expression correctly.

Hints from issue discussion:

Commit message: added `failing_assumptions()` as a helper for `check_assumptions()` and tests

Patch to review:
diff --git a/sympy/solvers/solvers.py b/sympy/solvers/solvers.py
index 5723f14c63b0..6000854756bf 100644
--- a/sympy/solvers/solvers.py
+++ b/sympy/solvers/solvers.py
@@ -361,6 +361,50 @@ def checksol(f, symbol, sol=None, **flags):
     # TODO: improve solution testing
 
 
+def failing_assumptions(expr, against=None, **assumptions):
+    """
+    Helper for ``check_assumptions``.
+
+    Returns empty list `[]` if all assumptions are True, or a list
+    of  assumption(s) that the expression does not satisfy or
+    `None` if it can't conclude.
+
+    Examples
+    ========
+
+        >>> from sympy import Symbol
+        >>> from sympy.solvers.solvers import failing_assumptions
+        >>> failing_assumptions(-5, integer=False, positive=True)
+        ['integer', 'positive']
+
+        >>> x = Symbol('x', positive=True)
+        >>> y = Symbol('y')
+        >>> failing_assumptions(6*x, positive=True)
+        []
+
+        >>> failing_assumptions(y, real=True)
+    """
+    if against is not None:
+        assumptions = against.assumptions0
+
+    expr = sympify(expr)
+
+    flag = True
+    fail_list = []
+    for key, expected in ordered(assumptions.items()):
+        if expected is None:
+            continue
+        test = getattr(expr, 'is_' + key, None)
+        if test is expected:
+            continue
+        elif test is not None:
+            fail_list.append(key)
+            flag = True
+            continue
+        flag = False
+    return fail_list if flag else None
+
+
 def check_assumptions(expr, against=None, **assumptions):
     """Checks whether expression `expr` satisfies all assumptions.
 
@@ -398,22 +442,8 @@ def check_assumptions(expr, against=None, **assumptions):
        >>> z = Symbol('z')
        >>> check_assumptions(z, real=True)
     """
-    if against is not None:
-        assumptions = against.assumptions0
-
-    expr = sympify(expr)
-
-    result = True
-    for key, expected in assumptions.items():
-        if expected is None:
-            continue
-        test = getattr(expr, 'is_' + key, None)
-        if test is expected:
-            continue
-        elif test is not None:
-            return False
-        result = None  # Can't conclude, unless an other test fails.
-    return result
+    ans = failing_assumptions(expr, against, **assumptions)
+    return None if ans is None else not ans
 
 
 def solve(f, *symbols, **flags):
diff --git a/sympy/solvers/tests/test_solvers.py b/sympy/solvers/tests/test_solvers.py
index 44c574a96b60..880e35031559 100644
--- a/sympy/solvers/tests/test_solvers.py
+++ b/sympy/solvers/tests/test_solvers.py
@@ -12,7 +12,8 @@
 from sympy.solvers import solve_linear_system, solve_linear_system_LU, \
     solve_undetermined_coeffs
 from sympy.solvers.solvers import _invert, unrad, checksol, posify, _ispow, \
-    det_quick, det_perm, det_minor, _simple_dens, check_assumptions, denoms
+    det_quick, det_perm, det_minor, _simple_dens, check_assumptions, denoms, \
+    failing_assumptions
 
 from sympy.physics.units import cm
 from sympy.polys.rootoftools import CRootOf
@@ -1301,6 +1302,14 @@ def test_check_assumptions():
     assert solve(x**2 - 1) == [1]
     assert check_assumptions(1, x) == True
 
+def test_failing_assumptions():
+    x = Symbol('x', real=True, positive=True)
+    y = Symbol('y')
+    assert failing_assumptions(6*x + y, x) == None
+    assert failing_assumptions(x**2 - 1, positive=True) == None
+    assert failing_assumptions(-2*x - 6, real=True, positive=True) == ['positive']
+    assert failing_assumptions(x**2, positive=True) == []
+
 
 def test_issue_6056():
     assert solve(tanh(x + 3)*tanh(x - 3) - 1) == []


Code review comments:
diff_hunk: @@ -361,7 +361,51 @@ def checksol(f, symbol, sol=None, **flags):
     # TODO: improve solution testing
 
 
-def check_assumptions(expr, against=None, failing_assumption=False, **assumptions):
+def failing_assumptions(expr, against=None, **assumptions):
+    """
+    Helper for ``check_assumptions``.
+
+    Returns empty list `[]` if all assumptions are True, or a list
+    of  assumption(s) that the expression does not satisfy or
+    `None` if it can't conclude.
+
+    Examples
+    ========
+
+        >>> from sympy import Symbol
+        >>> from sympy.solvers.solvers import failing_assumptions
+        >>> failing_assumptions(-5, integer=False, positive=True)
+        ['integer', 'positive']
+
+        >>> x = Symbol('x', positive=True)
+        >>> y = Symbol('y')
+        >>> failing_assumptions(6*x, positive=True)
+        []
+
+        >>> failing_assumptions(y, real=True)
+    """
+    if against is not None:
+        assumptions = against.assumptions0
+
+    expr = sympify(expr)
+
+    flag = True
path: sympy/solvers/solvers.py
text: @user1:
What do you think about this (no frills version)?

```
def failing_assumptions(expr, **assumptions):
    failed = {}
    for key in list(assumptions.keys()):
        test = getattr(expr, 'is_%s' % key, None)
        if test is not assumptions[key]:
            failed[key] = test
    return failed  # {} or {assumption: value != desired}

def check_assumptions(expr, against=None, **assumptions):
    if against:
        assert not assumptions
        assert against.is_Symbol # ?
        assumptions = against.assumptions0
    a = assumptions
    return all(getattr(expr, 'is_%s' % k, None) is a[k] for k in a)

>>> x = Symbol('x', real=True, positive=True)
>>> y = Symbol('y')
>>> failing_assumptions(6*x + y, real=True, positive=True)
{'real': None, 'positive': None}
>>> failing_assumptions(6*x + y, **x.assumptions0) == {
    'real': None,
    'imaginary': None,
    'complex': None,
    'hermitian': None,
    'positive': None, 
    'nonpositive': None, 
    'nonnegative': None, 
    'nonzero': None, 
    'negative': None, 
    'zero': None}
>>> failing_assumptions(x**2 - 1, positive=True)
{'positive': None}
>>> failing_assumptions(-2*x - 6, real=True, positive=True)
{'positive': False}
>>> failing_assumptions(x**2, positive=True)
{}
```

My thought is that if you are going to go through the trouble of returning what is wrong, you might as well tell how it is wrong. Imagine this being used interactively as a tutor and the task is to create an expression that is unknown to be positive. So you call `failing_assumptions(given, positive=None)`. Let's say they give a result that is positive. The session would then report that "no, your expression is positive" without having to recompute the value. Of course in the version you presented, only True and False values are returned so the key along with the passed values are sufficient for reconstruction. But it doesn't seem unreasonable to pass in an assumption that is None (as in the example given).

@author:
To see if we both are on the same page, what I understood is you want `failing_assumptions` to return correct value of the key along with the key instead of a key only. Is it correct?

In examples above I see if an `expr` returns `None`, all assumptions are made `None`. I am not sure but I don't think this should be done because `None` comes out when atleast one (or maybe all)assumption cannot be concluded and rest are `True`. like:
`(6*x + y, real=True, positive=True)` in this real=True is correct but positive=True cannot be determined therfore the result is `None`. I think the result should be `{'real': True, 'positive': None}`

> without having to recompute the value

I am not clear on this.

@author:
> What do you think about this (no frills version)?

I think it seems to be good, but the only question that comes to my mind is how effective this would be means will it be of any positive use(like to carry out any further computation), or will it just be a functionality that we have.

@user1:
> (6*x + y, real=True, positive=True) in this real=True is correct

Maybe you meant to make y real b/c as a vanilla y, the expression is not known to be real:

```
>>> x = Symbol('x', real=True, positive=True)
>>> y = Symbol('y')
>>> eq=6*x + y
>>> eq.is_real
>>> eq.is_positive
>>>
```

> if an expr returns None, all assumptions are made None

I'm not sure what you mean here. In this case, for all assumptions that y had the expression happened to have values of None, but this would not always be the case. For example, if we make y be an integer, it will have 11 assumptions. The expression `6*x + y` will have 6 of them that are None, not all of them:

```
>>> x = Symbol('x', real=True, positive=True)
>>> y = Symbol('y', integer=True)
>>> failing_assumptions(6*x + y, **y.assumptions0)
>>> failing_assumptions(6*x+y, **y.assumptions0)
{'integer': None, 'rational': None, 'noninteger': None, 'algebraic': None, 'irrational': None, 'transcendental': None}
>>> len(_), len(y.assumptions0)
(6, 11)
```

> will it be of any positive use

What is the use case that you are thinking of for the function -- it isn't explicitly state in the issue. (I gave an example of how one might use the information, e.g. with an interactive tutor program in which it would be useful to know the incorrect assumption and value so it can be reported to the user. Still...this seems of limited use. That's why I'm interested in seeing how you imagine such functionality to be used.)