Skip to content
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

Remove args and func overriding of AppliedPredicate #20847

Merged
merged 10 commits into from Apr 13, 2021
Merged

Remove args and func overriding of AppliedPredicate #20847

merged 10 commits into from Apr 13, 2021

Conversation

JSS95
Copy link
Collaborator

@JSS95 JSS95 commented Jan 22, 2021

References to other Issues or PRs

Required for #20635

Brief description of what is fixed or changed

Previously, AppliedPredicate.func and AppliedPredicate.args were overridden. This PR makes AppliedPredicate just inherit Basic.func and Basic.args.

Other comments

Release Notes

  • assumptions
    • func property of AppliedPredicate returns AppliedPredicate class instead of unapplied predicate object. User must use function property to get the predicate. This breaks backwards compatibility.
    • args property of AppliedPredicate returns unapplied predicate object and arguments. User must use arguments property to get the arguments without unapplied predicate. This breaks backwards compatibility.

@sympy-bot
Copy link

sympy-bot commented Jan 22, 2021

Hi, I am the SymPy bot (v161). 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:

  • assumptions
    • func property of AppliedPredicate returns AppliedPredicate class instead of unapplied predicate object. User must use function property to get the predicate. This breaks backwards compatibility. (#20847 by @JSS95 and @oscarbenjamin)

    • args property of AppliedPredicate returns unapplied predicate object and arguments. User must use arguments property to get the arguments without unapplied predicate. This breaks backwards compatibility. (#20847 by @JSS95 and @oscarbenjamin)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.9.

Click here to see the pull request description that was parsed.
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

Required for #20635

#### Brief description of what is fixed or changed

Previously, `AppliedPredicate.func` and `AppliedPredicate.args` were overridden. This PR makes `AppliedPredicate` just inherit `Basic.func` and `Basic.args`.

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
- assumptions
    - `func` property of `AppliedPredicate` returns `AppliedPredicate` class instead of unapplied predicate object. User must use `function` property to get the predicate. This breaks backwards compatibility.
    - `args` property of `AppliedPredicate` returns unapplied predicate object and arguments. User must use `arguments` property to get the arguments without unapplied predicate. This breaks backwards compatibility.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@JSS95
Copy link
Collaborator Author

JSS95 commented Jan 22, 2021

@oscarbenjamin

I had to introduce _find_freepredicate() in order to replace expr.atoms(Predicate) used in sathandlers.py. Q.positive(x).atoms(Predicate) refers to Q.positive(x).args. Previously, .args returned (x,), so Q.positive(x).atoms(Predicate) returned empty set. Now, it returns (Q.positive, x), therefore {Q.positive} is returned. This raised error in UnevaluatedOnFree.

Change `.args` to `.arguments` in UnevaluatedOnFree.__new__.
@JSS95
Copy link
Collaborator Author

JSS95 commented Jan 22, 2021

There are a lot of other errors which cannot be easily resolved.

__________________________________________________________________________________________________________________________________________________________________________________________________
_______________________________________________________________________ sympy/assumptions/tests/test_query.py:test_bounded _______________________________________________________________________
Traceback (most recent call last):
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/tests/test_query.py", line 591, in test_bounded
    assert ask(Q.finite(a),
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/ask.py", line 417, in ask
    res = key(*args)._eval_ask(assumptions)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/assume.py", line 161, in _eval_ask
    return self.function.eval(self.arguments, assumptions)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/assume.py", line 355, in eval
    result = func(*args, assumptions)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/handlers/calculus.py", line 95, in _
    _bounded = ask(Q.finite(arg), assumptions)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/ask.py", line 397, in ask
    raise ValueError("inconsistent assumptions %s" % assumptions)
ValueError: inconsistent assumptions Q.finite(x) & Q.finite(y) & Q.positive(x) & ~Q.positive(y)
__________________________________________________________________________________________________________________________________________________________________________________________________
_____________________________________________________________________ sympy/assumptions/tests/test_query.py:test_issue_6732 ______________________________________________________________________
Traceback (most recent call last):
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/tests/test_query.py", line 2131, in test_issue_6732
    raises(ValueError, lambda: ask(Q.positive(x), Q.positive(x) & Q.negative(x)))
  File "/home/jisoosong/codes/sympy/sympy/sympy/testing/pytest.py", line 104, in raises
    raise Failed("DID NOT RAISE")
sympy.testing.pytest.Failed: DID NOT RAISE
__________________________________________________________________________________________________________________________________________________________________________________________________
_______________________________________________________________ sympy/assumptions/tests/test_sathandlers.py:test_UnevaluatedOnFree _______________________________________________________________
Traceback (most recent call last):
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/tests/test_sathandlers.py", line 33, in test_UnevaluatedOnFree
    assert b.rcall(x) == UnevaluatedOnFree(Q.positive(x) | Q.negative(x))
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/basic.py", line 609, in rcall
    return Basic._recursive_call(self, args)
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/basic.py", line 628, in _recursive_call
    return type(expr_to_call)(*args)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/sathandlers.py", line 86, in __new__
    obj.pred = arg.xreplace(Transform(lambda e: e.func, lambda e:
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/basic.py", line 1142, in xreplace
    value, _ = self._xreplace(rule)
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/basic.py", line 1164, in _xreplace
    return self.func(*args), True
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/operations.py", line 489, in __new__
    _args = frozenset(cls._new_args_filter(args))
  File "/home/jisoosong/codes/sympy/sympy/sympy/logic/boolalg.py", line 834, in _new_args_filter
    args = BooleanFunction.binary_check_and_simplify(*args)
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/operations.py", line 482, in <genexpr>
    args = (_sympify_(arg) for arg in args)
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/sympify.py", line 512, in _sympify
    return sympify(a, strict=True)
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/sympify.py", line 352, in sympify
    raise SympifyError(a)
sympy.core.sympify.SympifyError: SympifyError: <class 'sympy.assumptions.assume.AppliedPredicate'>
__________________________________________________________________________________________________________________________________________________________________________________________________
____________________________________________________________________ sympy/assumptions/tests/test_sathandlers.py:test_AllArgs ____________________________________________________________________
Traceback (most recent call last):
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/tests/test_sathandlers.py", line 60, in test_AllArgs
    assert a.rcall(x*y) == to_NNF(And(Q.zero(x), Q.zero(y)))
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/basic.py", line 609, in rcall
    return Basic._recursive_call(self, args)
  File "/home/jisoosong/codes/sympy/sympy/sympy/core/basic.py", line 628, in _recursive_call
    return type(expr_to_call)(*args)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/sathandlers.py", line 88, in __new__
    applied = obj.apply(obj.expr)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/sathandlers.py", line 97, in apply
    return self._eval_apply(expr, pred)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/sathandlers.py", line 134, in _eval_apply
    return AND(*[pred.rcall(arg) for arg in expr.args])
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/sathandlers.py", line 134, in <listcomp>
    return AND(*[pred.rcall(arg) for arg in expr.args])
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/cnf.py", line 35, in rcall
    lit = self.lit(expr)
  File "/home/jisoosong/codes/sympy/sympy/sympy/assumptions/assume.py", line 118, in __new__
    raise TypeError("%s is not a Predicate." % predicate)
TypeError: x is not a Predicate.

@oscarbenjamin
Copy link
Contributor

This can fix some of the errors:

diff --git a/sympy/assumptions/ask.py b/sympy/assumptions/ask.py
index fb00dac5b3..a42d669e84 100644
--- a/sympy/assumptions/ask.py
+++ b/sympy/assumptions/ask.py
@@ -290,7 +290,7 @@ def _extract_all_facts(expr, symbols):
             if isinstance(literal.lit, AppliedPredicate):
                 if literal.lit.arg in symbols:
                     # Add literal if it has 'symbol' in it
-                    args.append(Literal(literal.lit.func, literal.is_Not))
+                    args.append(Literal(literal.lit.function, literal.is_Not))
                 else:
                     # If any of the literals doesn't have 'symbol' don't add the whole clause.
                     break

@oscarbenjamin
Copy link
Contributor

Also this:

diff --git a/sympy/assumptions/satask.py b/sympy/assumptions/satask.py
index d672f75cfc..e72d2f2df8 100644
--- a/sympy/assumptions/satask.py
+++ b/sympy/assumptions/satask.py
@@ -92,7 +92,7 @@ def find_symbols(pred):
             req_keys |= tmp_keys
         keys |= {l for l in lkeys if find_symbols(l) & req_keys != set()}
 
-        exprs = {key.args[0] if isinstance(key, AppliedPredicate) else key for key in keys}
+        exprs = {key.arguments[0] if isinstance(key, AppliedPredicate) else key for key in keys}
         return exprs, relevant_facts
 
     for expr in exprs:

@oscarbenjamin
Copy link
Contributor

Also this:

diff --git a/sympy/assumptions/sathandlers.py b/sympy/assumptions/sathandlers.py
index 135e561f5a..6d856709dc 100644
--- a/sympy/assumptions/sathandlers.py
+++ b/sympy/assumptions/sathandlers.py
@@ -83,7 +83,7 @@ def __new__(cls, arg):
             raise ValueError("The AppliedPredicates in arg must be applied to a single expression.")
         obj = BooleanFunction.__new__(cls, arg)
         obj.expr = predicate_args.pop()
-        obj.pred = arg.xreplace(Transform(lambda e: e.func, lambda e:
+        obj.pred = arg.xreplace(Transform(lambda e: e.function, lambda e:
             isinstance(e, AppliedPredicate)))
         applied = obj.apply(obj.expr)
         if applied is None:

@JSS95
Copy link
Collaborator Author

JSS95 commented Apr 13, 2021

@oscarbenjamin
This PR can be merged now, which means that #20635 can be resolved as well.

@oscarbenjamin
Copy link
Contributor

Looks good

@oscarbenjamin oscarbenjamin merged commit 7aa05e9 into sympy:master Apr 13, 2021
@JSS95 JSS95 deleted the predicate_args branch April 13, 2021 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants