-
-
Notifications
You must be signed in to change notification settings - Fork 553
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
Implement create_gif
in QuickPlot
#1754
Implement create_gif
in QuickPlot
#1754
Conversation
Can you merge in develop? |
@@ -85,6 +85,7 @@ def __init__( | |||
self.C_rates = C_rates | |||
self.repeats = repeats | |||
self.permutations = permutations | |||
self.quick_plot = None |
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.
Not sure if this is the best way to implement this but I had to check if an object of QuickPlot
has been created by the user (for calling the QuickPlot.create_gif
method, same for the Simulation
class)
Also I think I messed something up while resolving conflicts, I'll check locally |
Codecov Report
@@ Coverage Diff @@
## develop #1754 +/- ##
========================================
Coverage 99.28% 99.28%
========================================
Files 343 343
Lines 18868 18894 +26
========================================
+ Hits 18733 18759 +26
Misses 135 135
Continue to review full report at Codecov.
|
""" | ||
|
||
if self.quick_plot is None: | ||
self.quick_plot = pybamm.QuickPlot(self.sims) |
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.
I thought of 3 options here (when the user hasn't called the plot
method) -
- raise an error
- pass in
kwargs
maybe, and then create aQuickPlot
object (didn't try) - create a
QuickPlot
object with default parameters (current implementation)
(same for the Simulation
class)
8c699e3
to
8b34425
Compare
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 to me thanks
pybamm/plotting/quick_plot.py
Outdated
@@ -103,7 +104,7 @@ def __init__( | |||
variable_limits="fixed", | |||
): | |||
input_solutions = solutions | |||
solutions = [] | |||
self.solutions = [] |
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.
how come you need to store solutions as an attribute now?
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 bad, I don't really remember why I did this 😅. Thanks!
Description
QuickPlot
pybamm.Simulation
andpybamm.BatchStudy
imageio
Fixes #1752
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ flake8
$ python run-tests.py --unit
$ cd docs
and then$ make clean; make html
You can run all three at once, using
$ python run-tests.py --quick
.Further checks: