Fixed set is equal to set only #20208
Conversation
|
✅ 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:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.7. Click here to see the pull request description that was parsed.
Update The release notes on the wiki have been updated. |
Codecov Report
@@ Coverage Diff @@
## master #20208 +/- ##
==============================================
+ Coverage 44.413% 64.384% +19.970%
==============================================
Files 671 671
Lines 173799 174149 +350
Branches 41067 41093 +26
==============================================
+ Hits 77191 112125 +34934
+ Misses 90709 55600 -35109
- Partials 5899 6424 +525 |
|
Pls review |
sympy/core/relational.py
Outdated
| @@ -1342,6 +1343,10 @@ def is_eq(lhs, rhs): | |||
| return retval | |||
|
|
|||
| retval = _eval_is_eq(lhs, rhs) | |||
|
|
|||
| if isinstance(lhs, Set) and not isinstance(rhs, Set) or not isinstance(lhs, Set) and isinstance(rhs, Set): | |||
| retval = False | |||
There was a problem hiding this comment.
A better fix would be to add _eval_Is_eq handlers in sets/handlers. We can have a handler for Set, Basic that always returns False and then a handler for Set, Set that returns None.
There was a problem hiding this comment.
Yes , thats what I tried at first but the two modules are so closely linked that their circular import issue couldn't be solved.
There was a problem hiding this comment.
There will be some way to solve the circular import. It just sometimes involves moving some imports around in other places
There was a problem hiding this comment.
I'll give it another try
|
@oscarbenjamin Review pls |
| assert Eq(1, FiniteSet(1, 2)) == False | ||
| assert FiniteSet(1) in B | ||
| A = FiniteSet(1, 2) | ||
| assert not A.issubset(B) |
There was a problem hiding this comment.
Maybe test A in B as well.
There was a problem hiding this comment.
Perhaps it would be good to test an example that is a subset as well.
sympy/sets/tests/test_sets.py
Outdated
| def test_issue_20089(): | ||
| B = FiniteSet(FiniteSet(1, 2), FiniteSet(1)) | ||
| assert not 1 in B | ||
| assert Eq(1, FiniteSet(1, 2)) == False |
There was a problem hiding this comment.
This should be is S.false rather than == False.
|
This looks good but there could be a few more tests |
|
@oscarbenjamin I have added more test cases. |
|
Looks good |
Can it be merged now? |
|
Yes, I was just waiting for the tests to pass |
References to other Issues or PRs
Fixes #20089
Brief description of what is fixed or changed
Earlier #in #subset gave wrong output due to inconsistency of #Eq with sets and expr
Other comments
Release Notes