-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Improve the set handler #20650
base: master
Are you sure you want to change the base?
Improve the set handler #20650
Conversation
…ections of intervals
✅ 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.
Click here to see the pull request description that was parsed.
|
Certain tests, for instance |
sympy/sets/handlers/intersection.py
Outdated
else: | ||
start = Max(a.start, b.start) | ||
end = Min(a.end, b.end) | ||
if a.start > b.start: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could raise for symbolic start and end. It could be if is_gt(a.start, a. end)
.
This should be |
|
Without knowing the cause of the breakage it's impossible to answer that question. What "breaks" and how? |
Both of my above comments. |
In [5]: 2 < y
Out[5]: y > 2
In [6]: S(2) < y
Out[6]: 2 < y
In [7]: Intersection(Interval(0, 1), Interval(x, y))
Out[7]: [Max(0, x), Min(1, y)] |
@@ -889,7 +889,7 @@ def test_sympy__sets__sets__Intersection(): | |||
x = Symbol('x') | |||
y = Symbol('y') | |||
S = Intersection(Interval(0, x), Interval(y, 1)) | |||
assert isinstance(S, Intersection) | |||
assert isinstance(S, Interval) | |||
assert _test_args(S) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should is supposed to test Intersection
so if it doesn't give an intersection then something else needs to be used as the arguments.
ucode_str = '[x, y] ∩ [z, w]' | ||
ascii_str = '[x, y] n [z, w]' | ||
ucode_str = '[Max(x, z), Min(w, y)]' | ||
ascii_str = '[Max(x, z), Min(w, y)]' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is supposed to be a test of printing an Intersection so if it doesn't give an intersection then something other than interval should be used as the arguments
@@ -917,7 +917,7 @@ def test_latex_union(): | |||
|
|||
def test_latex_intersection(): | |||
assert latex(Intersection(Interval(0, 1), Interval(x, y))) == \ | |||
r"\left[0, 1\right] \cap \left[x, y\right]" | |||
r"\left[\max\left(0, x\right), \min\left(1, y\right)\right]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also a test for printing Intersection
I'm not sure now whether having this evaluate automatically is a good idea or whether it should really be part of We do need a function that can rewrite an intersection of intervals using |
References to other Issues or PRs
Fixes #20535
Brief description of what is fixed or changed
The set handler can now deal with symbolic intervals
Other comments
Release Notes
NO ENTRY