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

Add float sanitization to xlim and ylim in plot #15265

Merged
merged 2 commits into from Sep 19, 2018
Merged

Conversation

sylee957
Copy link
Member

@sylee957 sylee957 commented Sep 18, 2018

References to other Issues or PRs

Fixes #15261

Brief description of what is fixed or changed

I found that the issue had been caused by sympy number types directly passed to matplotlib's backend, and getting checked by numpy's isfinite(), which cannot handle sympy's numbers.

Any sympy's constants, or sympified numbers passed into plot's xlim and ylim would repeat the issue in this context.

>>> from sympy import *
>>>
>>> x    = symbols('x')
>>> eqn  = sin(x)
>>> p    = plot(eqn,(x,-10,10), xlim=(-5, sympify('5')), ylim=(-1, 1))
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

I think every sympy's numbers should be sanitized into python default float type, before passed into matplotlib backend.

And also, I had added is_real check and is_finite check, to make sure that float() can be safely applied to, and to implement numpy's isfinite() check in sympy's own way.

I think range parameter has implicit way of sanitizing sympy's types unlike xlim or ylim,
so that the method could possibly be a workaround for original poster.

Other comments

Release Notes

  • plotting
    • Fixed xlim and ylim from plot() not properly handling sympy's number classes.

@sympy-bot
Copy link

sympy-bot commented Sep 18, 2018

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

  • plotting
    • Fixed xlim and ylim from plot() not properly handling sympy's number classes. (#15265 by @sylee957)

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

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.

<!-- 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://github.com/blog/1506-closing-issues-via-pull-requests .-->
Fixes #15261

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

I found that the issue had been caused by sympy number types directly passed to matplotlib's backend, and getting checked by numpy's `isfinite()`, which cannot handle sympy's numbers.

Any sympy's constants, or sympified numbers passed into plot's `xlim` and `ylim` would repeat the issue in this context.

```
>>> from sympy import *
>>>
>>> x    = symbols('x')
>>> eqn  = sin(x)
>>> p    = plot(eqn,(x,-10,10), xlim=(-5, sympify('5')), ylim=(-1, 1))
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''
```

I think every sympy's numbers should be sanitized into python default `float` type, before passed into matplotlib backend.

And also, I had added `is_real` check and `is_finite` check, to make sure that `float()` can be safely applied to, and to implement numpy's `isfinite()` check in sympy's own way.

I think `range` parameter has implicit way of sanitizing sympy's types unlike `xlim` or `ylim`,
so that the method could possibly be a workaround for original poster.

#### 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 -->
- plotting
  - Fixed `xlim` and `ylim` from `plot()` not properly handling sympy's number classes.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@sylee957 sylee957 changed the title Add float sanitization to xlim and ylim setting in plot Add float sanitization to xlim and ylim in plot Sep 18, 2018
@smichr
Copy link
Member

smichr commented Sep 18, 2018

+1

@smichr smichr merged commit 65885f0 into sympy:master Sep 19, 2018
@sylee957 sylee957 deleted the plot_fix branch September 21, 2018 18:25
sylee957 added a commit to sylee957/sympy that referenced this pull request Sep 22, 2018
jksuom added a commit that referenced this pull request Sep 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plot sine function using radians with xlim
3 participants