-
-
Notifications
You must be signed in to change notification settings - Fork 409
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
Convert quantities to arrays when plotting in SDEC plot #1832
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1832 +/- ##
=======================================
Coverage 58.04% 58.04%
=======================================
Files 66 66
Lines 6747 6747
=======================================
Hits 3916 3916
Misses 2831 2831 Continue to review full report at Codecov.
|
10939a9
to
0382a34
Compare
Before a pull request is accepted, it must meet the following criteria:
|
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 fixes the problem, but do we want to do this, or pin the matplotlib version?
We definitely want this. Pinning the matplotlib version was a backup in case this problem was not easy enough to fix. |
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.
My only concern here is do we allow users to use different units when plotting? And either way, we should probably have the units printed on the axes, something like this.
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.
Thanks!
I think that we infer unit to be shown on axes from quantity. If not, then that's case for another issue/PR @smithis7 @atharva-2001 - it's out of scope for this PR. |
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.
Looks good
This fixes the docs build that was caused due to the new matplotlib release.
Description
Using quantities with
matplotlib
'sfill_between
method was causing an error. This PR changes quantities to arrays, just when plotting, everywhere in the SDEC plot so that no problems arise later as well.Motivation and context
How has this been tested?
Examples
Link to a failed documentation build
working docs build
Type of change
Checklist