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

Fix FiniteSet union handler for symbolic elements #18356

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

gschintgen
Copy link
Contributor

References to other Issues or PRs

Fixes #18241.

Brief description of what is fixed or changed

Before:

In [1]: Interval(0, 1).union(FiniteSet(1, x))
Out[1]: [0, 1]

After:

In [1]: Interval(0, 1).union(FiniteSet(1, x))
Out[1]: [0, 1] ∪ {x}

Reason:
Incorrect handling of .contains() ternary logic
(True or False or Contains/Relational)

Tests are added.

Other comments

Release Notes

  • sets
    • Fixed a bug in FiniteSet's union handler that could lead to some elements being "dropped" if their membership in the other set can't be determined.

Fixes sympy#18241.

Before:
In [1]: Interval(0, 1).union(FiniteSet(1, x))
Out[1]: [0, 1]

After:
In [1]: Interval(0, 1).union(FiniteSet(1, x))
Out[1]: [0, 1] ∪ {x}

Reason:
Incorrect handling of `.contains()` ternary logic
(True or False or Contains/Relational)

Tests are added.
@sympy-bot
Copy link

sympy-bot commented Jan 16, 2020

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:

  • sets
    • Fixed a bug in FiniteSet's union handler that could lead to some elements being "dropped" if their membership in the other set can't be determined. (#18356 by @gschintgen)

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

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.

#### References to other Issues or PRs

Fixes #18241.

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

Before:
```
In [1]: Interval(0, 1).union(FiniteSet(1, x))
Out[1]: [0, 1]
```

After:
```
In [1]: Interval(0, 1).union(FiniteSet(1, x))
Out[1]: [0, 1] ∪ {x}
```

Reason:
Incorrect handling of `.contains()` ternary logic
(True or False or Contains/Relational)

Tests are added.

#### 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 -->
* sets
  * Fixed a bug in `FiniteSet`'s union handler that could lead to some elements being "dropped" if their membership in the other set can't be determined.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@codecov
Copy link

codecov bot commented Jan 16, 2020

Codecov Report

Merging #18356 into master will decrease coverage by 0.002%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##            master    #18356       +/-   ##
=============================================
- Coverage   75.013%   75.011%   -0.003%     
=============================================
  Files          649       649               
  Lines       167348    167348               
  Branches     39417     39417               
=============================================
- Hits        125534    125530        -4     
- Misses       36275     36284        +9     
+ Partials      5539      5534        -5

@oscarbenjamin
Copy link
Contributor

Looks good.

@oscarbenjamin oscarbenjamin merged commit 2dbaf0d into sympy:master Jan 16, 2020
@gschintgen gschintgen deleted the fix-18241-finiteset-union branch January 17, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interval(0, 1).union(FiniteSet(1, x)) drops 'x'
3 participants