Merge pull request #1099 from pybamm-team/issue-1084-kwargs-sim-plot

#1084 allow kwargs to be passed to sim.plot
pybamm-team · Jul 8, 2020 · b2488ce · b2488ce
2 parents 5ff13ed + a83b2fb
commit b2488ce
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,18 @@
+# Unreleased
+
+## Features
+
+-   Allowed keyword arguments to be passed to `Simulation.plot()` ([#1099](https://github.com/pybamm-team/PyBaMM/pull/1099))
+
+## Optimizations
+
+## Bug fixes
+
+## Breaking changes
+
+-  Renamed `quick_plot_vars` to `output_variables` in `Simulation` to be consistent with `QuickPlot`. Passing `quick_plot_vars` to `Simulation.plot()` has been deprecated and `output_variables` should be passed instead ([#1099](https://github.com/pybamm-team/PyBaMM/pull/1099))
+
+
 # [v0.2.3](https://github.com/pybamm-team/PyBaMM/tree/v0.2.3) - 2020-07-01
 
 This release enables the use of [Google Colab](https://colab.research.google.com/github/pybamm-team/PyBaMM/blob/master/) for running example notebooks, and adds some small new features and bug fixes.
@@ -33,7 +48,7 @@ This release enables the use of [Google Colab](https://colab.research.google.com
 ## Breaking changes
 
 -   `Simulation.specs` and `Simulation.set_defaults` have been deprecated. Users should create a new `Simulation` object for each different case instead ([#1090](https://github.com/pybamm-team/PyBaMM/pull/1090))
--  The solution times `t_eval` must now be provided to `Simulation.solve()` when not using an experiment or prescribing the current using drive cycle data ([#1086](https://github.com/pybamm-team/PyBaMM/pull/1086))
+-   The solution times `t_eval` must now be provided to `Simulation.solve()` when not using an experiment or prescribing the current using drive cycle data ([#1086](https://github.com/pybamm-team/PyBaMM/pull/1086))
 
 # [v0.2.2](https://github.com/pybamm-team/PyBaMM/tree/v0.2.2) - 2020-06-01
 

diff --git a/examples/notebooks/Getting Started/Tutorial 3 - Basic plotting.ipynb b/examples/notebooks/Getting Started/Tutorial 3 - Basic plotting.ipynb
@@ -29,7 +29,7 @@
     {
      "data": {
       "text/plain": [
-       "<pybamm.solvers.solution.Solution at 0x7fe21ea34c50>"
+       "<pybamm.solvers.solution.Solution at 0x7f5a7dda5048>"
       ]
      },
      "execution_count": 1,
@@ -666,7 +666,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "210ea1fb46d9436e8b48f53c70b80db7",
+       "model_id": "26530e3fbec34dfa921fda4c1490a8f5",
        "version_major": 2,
        "version_minor": 0
       },
@@ -679,8 +679,8 @@
     }
    ],
    "source": [
-    "quick_plot_vars = [\"Terminal voltage [V]\"]\n",
-    "sim.plot(quick_plot_vars=quick_plot_vars)"
+    "output_variables = [\"Terminal voltage [V]\"]\n",
+    "sim.plot(output_variables=output_variables)"
    ]
   },
   {
@@ -698,7 +698,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "8b13649c493c4561b647cf49f3867486",
+       "model_id": "d1d16e2c52654cf48e1d3cd398c4ed18",
        "version_major": 2,
        "version_minor": 0
       },
@@ -711,8 +711,8 @@
     }
    ],
    "source": [
-    "quick_plot_vars = [\"Electrolyte concentration [mol.m-3]\", \"Terminal voltage [V]\"]\n",
-    "sim.plot(quick_plot_vars=quick_plot_vars)"
+    "output_variables = [\"Electrolyte concentration [mol.m-3]\", \"Terminal voltage [V]\"]\n",
+    "sim.plot(output_variables=output_variables)"
    ]
   },
   {
@@ -730,7 +730,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "b471a3ef004a422d9ad5797c74099c6b",
+       "model_id": "3d08bfd164df409ca3838bb1edd4d4c9",
        "version_major": 2,
        "version_minor": 0
       },
@@ -748,13 +748,13 @@
   },
   {
    "cell_type": "code",
-   "execution_count": 9,
+   "execution_count": 7,
    "metadata": {},
    "outputs": [
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "ec65841022c74b27bc1c60757a8a3f1a",
+       "model_id": "c248e1ccf0c34d53b5f2c0e44115e8d5",
        "version_major": 2,
        "version_minor": 0
       },
@@ -796,7 +796,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.7.7"
+   "version": "3.6.9"
   }
  },
  "nbformat": 4,

diff --git a/pybamm/simulation.py b/pybamm/simulation.py
@@ -64,7 +64,7 @@ class Simulation:
         domain (e.g. pybamm.FiniteVolume)
     solver: :class:`pybamm.BaseSolver` (optional)
         The solver to use to solve the model.
-    quick_plot_vars: list (optional)
+    output_variables: list (optional)
         A list of variables to plot automatically
     C_rate: float (optional)
         The C_rate at which you would like to run a constant current
@@ -81,7 +81,7 @@ def __init__(
         var_pts=None,
         spatial_methods=None,
         solver=None,
-        quick_plot_vars=None,
+        output_variables=None,
         C_rate=None,
     ):
         self.parameter_values = parameter_values or model.default_parameter_values
@@ -112,7 +112,7 @@ def __init__(
         self.var_pts = var_pts or self.model.default_var_pts
         self.spatial_methods = spatial_methods or self.model.default_spatial_methods
         self.solver = solver or self.model.default_solver
-        self.quick_plot_vars = quick_plot_vars
+        self.output_variables = output_variables
 
         # Initialize empty built states
         self._model_with_set_params = None
@@ -531,28 +531,39 @@ def get_variable_array(self, *variables):
         else:
             return tuple(variable_arrays)
 
-    def plot(self, quick_plot_vars=None, testing=False):
+    def plot(self, output_variables=None, quick_plot_vars=None, **kwargs):
         """
-        A method to quickly plot the outputs of the simulation.
+        A method to quickly plot the outputs of the simulation. Creates a
+        :class:`pybamm.QuickPlot` object (with keyword arguments 'kwargs') and
+        then calls :meth:`pybamm.QuickPlot.dynamic_plot`.
 
         Parameters
         ----------
-        quick_plot_vars: list, optional
+        output_variables: list, optional
             A list of the variables to plot.
-        testing, bool, optional
-            If False the plot will not be displayed
+        quick_plot_vars: list, optional
+            A list of the variables to plot. Deprecated, use output_variables instead.
+        **kwargs
+            Additional keyword arguments passed to
+            :meth:`pybamm.QuickPlot.dynamic_plot`.
+            For a list of all possible keyword arguments see :class:`pybamm.QuickPlot`.
         """
 
+        if quick_plot_vars is not None:
+            raise NotImplementedError(
+                "'quick_plot_vars' has been deprecated. Use 'output_variables' instead."
+            )
+
         if self._solution is None:
             raise ValueError(
                 "Model has not been solved, please solve the model before plotting."
             )
 
-        if quick_plot_vars is None:
-            quick_plot_vars = self.quick_plot_vars
+        if output_variables is None:
+            output_variables = self.output_variables
 
         self.quick_plot = pybamm.dynamic_plot(
-            self._solution, output_variables=quick_plot_vars, testing=testing
+            self._solution, output_variables=output_variables, **kwargs
         )
 
     @property
@@ -625,12 +636,12 @@ def solver(self, solver):
         self._solver = solver.copy()
 
     @property
-    def quick_plot_vars(self):
-        return self._quick_plot_vars
+    def output_variables(self):
+        return self._output_variables
 
-    @quick_plot_vars.setter
-    def quick_plot_vars(self, quick_plot_vars):
-        self._quick_plot_vars = copy.copy(quick_plot_vars)
+    @output_variables.setter
+    def output_variables(self, output_variables):
+        self._output_variables = copy.copy(output_variables)
 
     @property
     def solution(self):
@@ -644,7 +655,7 @@ def specs(
         var_pts=None,
         spatial_methods=None,
         solver=None,
-        quick_plot_vars=None,
+        output_variables=None,
         C_rate=None,
     ):
         "Deprecated method for setting specs"

diff --git a/tests/unit/test_simulation.py b/tests/unit/test_simulation.py
@@ -297,6 +297,10 @@ def test_plot(self):
         sim.solve(t_eval=t_eval)
         sim.plot(testing=True)
 
+        # test quick_plot_vars deprecation error
+        with self.assertRaisesRegex(NotImplementedError, "'quick_plot_vars'"):
+            sim.plot(quick_plot_vars=["var"])
+
     def test_drive_cycle_data(self):
         model = pybamm.lithium_ion.SPM()
         param = model.default_parameter_values