From 99bd7898afcb098d67246d3be6ab587a4485e8d0 Mon Sep 17 00:00:00 2001 From: kratman Date: Tue, 19 Sep 2023 20:07:27 -0400 Subject: [PATCH 1/4] Check for empty list --- pybamm/plotting/quick_plot.py | 41 ++++++++++++++------- tests/unit/test_plotting/test_quick_plot.py | 6 ++- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/pybamm/plotting/quick_plot.py b/pybamm/plotting/quick_plot.py index 03bfeeccd4..2b627efe56 100644 --- a/pybamm/plotting/quick_plot.py +++ b/pybamm/plotting/quick_plot.py @@ -107,20 +107,7 @@ def __init__( variable_limits="fixed", ): input_solutions = solutions - solutions = [] - if not isinstance(input_solutions, (pybamm.Solution, pybamm.Simulation, list)): - raise TypeError( - "solutions must be 'pybamm.Solution' or 'pybamm.Simulation' or list" - ) - elif not isinstance(input_solutions, list): - input_solutions = [input_solutions] - for sim_or_sol in input_solutions: - if isinstance(sim_or_sol, pybamm.Simulation): - # 'sim_or_sol' is actually a 'Simulation' object here so it has a - # 'Solution' attribute - solutions.append(sim_or_sol.solution) - elif isinstance(sim_or_sol, pybamm.Solution): - solutions.append(sim_or_sol) + solutions = self.preprocess_solutions(input_solutions) models = [solution.all_models[0] for solution in solutions] @@ -242,6 +229,32 @@ def __init__( self.set_output_variables(output_variable_tuples, solutions) self.reset_axis() + @staticmethod + def preprocess_solutions(input_solutions): + solutions = [] + input_solutions = QuickPlot.check_input_validity(input_solutions) + for sim_or_sol in input_solutions: + if isinstance(sim_or_sol, pybamm.Simulation): + # 'sim_or_sol' is actually a 'Simulation' object here so it has a + # 'Solution' attribute + solutions.append(sim_or_sol.solution) + elif isinstance(sim_or_sol, pybamm.Solution): + solutions.append(sim_or_sol) + return solutions + + @staticmethod + def check_input_validity(input_solutions): + if not isinstance(input_solutions, (pybamm.Solution, pybamm.Simulation, list)): + raise TypeError( + "Solutions must be 'pybamm.Solution' or 'pybamm.Simulation' or list" + ) + elif not isinstance(input_solutions, list): + input_solutions = [input_solutions] + else: + if not input_solutions: + raise TypeError("QuickPlot requires at least 1 solution or simulation.") + return input_solutions + def set_output_variables(self, output_variables, solutions): # Set up output variables self.variables = {} diff --git a/tests/unit/test_plotting/test_quick_plot.py b/tests/unit/test_plotting/test_quick_plot.py index db7de574dc..11b1808b77 100644 --- a/tests/unit/test_plotting/test_quick_plot.py +++ b/tests/unit/test_plotting/test_quick_plot.py @@ -464,9 +464,13 @@ def test_plot_2plus1D_spm(self): pybamm.close_plots() def test_failure(self): - with self.assertRaisesRegex(TypeError, "solutions must be"): + with self.assertRaisesRegex(TypeError, "Solutions must be"): pybamm.QuickPlot(1) + def test_empty_list_failure(self): + with self.assertRaisesRegex(TypeError, "QuickPlot requires at least 1"): + pybamm.QuickPlot([]) + def test_model_with_inputs(self): parameter_values = pybamm.ParameterValues("Chen2020") model = pybamm.lithium_ion.SPMe() From 3ae6c5dc913aff358f83ee4d6d7604fb2395e188 Mon Sep 17 00:00:00 2001 From: kratman Date: Tue, 19 Sep 2023 20:35:00 -0400 Subject: [PATCH 2/4] Minor clean-up --- pybamm/plotting/quick_plot.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pybamm/plotting/quick_plot.py b/pybamm/plotting/quick_plot.py index 2b627efe56..d6828ce18a 100644 --- a/pybamm/plotting/quick_plot.py +++ b/pybamm/plotting/quick_plot.py @@ -106,8 +106,7 @@ def __init__( spatial_unit="um", variable_limits="fixed", ): - input_solutions = solutions - solutions = self.preprocess_solutions(input_solutions) + solutions = self.preprocess_solutions(solutions) models = [solution.all_models[0] for solution in solutions] @@ -230,17 +229,17 @@ def __init__( self.reset_axis() @staticmethod - def preprocess_solutions(input_solutions): - solutions = [] - input_solutions = QuickPlot.check_input_validity(input_solutions) + def preprocess_solutions(solutions): + input_solutions = QuickPlot.check_input_validity(solutions) + processed_solutions = [] for sim_or_sol in input_solutions: if isinstance(sim_or_sol, pybamm.Simulation): - # 'sim_or_sol' is actually a 'Simulation' object here so it has a + # 'sim_or_sol' is actually a 'Simulation' object here, so it has a # 'Solution' attribute - solutions.append(sim_or_sol.solution) + processed_solutions.append(sim_or_sol.solution) elif isinstance(sim_or_sol, pybamm.Solution): - solutions.append(sim_or_sol) - return solutions + processed_solutions.append(sim_or_sol) + return processed_solutions @staticmethod def check_input_validity(input_solutions): From 252c5ba7cfd5d58b8b483de2a3064f89a63baa66 Mon Sep 17 00:00:00 2001 From: kratman Date: Thu, 21 Sep 2023 08:00:54 -0400 Subject: [PATCH 3/4] Update change log and test name --- CHANGELOG.md | 1 + tests/unit/test_plotting/test_quick_plot.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cff4551ef8..bf9d5ab4e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ## Bug fixes +- Fixed a bug where empty lists passed to QuickPlot resulted in an IndexError and did not return a meaningful error message. - Fixed a bug where there was a missing thermal conductivity in the thermal pouch cell models ([#3330](https://github.com/pybamm-team/PyBaMM/pull/3330)) - Fixed a bug that caused incorrect results of “{Domain} electrode thickness change [m]” due to the absence of dimension for the variable `electrode_thickness_change`([#3329](https://github.com/pybamm-team/PyBaMM/pull/3329)). - Fixed a bug that occured in `check_ys_are_not_too_large` when trying to reference `y-slice` where the referenced variable was not a `pybamm.StateVector` ([#3313](https://github.com/pybamm-team/PyBaMM/pull/3313) diff --git a/tests/unit/test_plotting/test_quick_plot.py b/tests/unit/test_plotting/test_quick_plot.py index 11b1808b77..3415777ee8 100644 --- a/tests/unit/test_plotting/test_quick_plot.py +++ b/tests/unit/test_plotting/test_quick_plot.py @@ -463,7 +463,7 @@ def test_plot_2plus1D_spm(self): pybamm.close_plots() - def test_failure(self): + def test_invalid_input_type_failure(self): with self.assertRaisesRegex(TypeError, "Solutions must be"): pybamm.QuickPlot(1) From 243de257e28ac834a8fc5df47ca1beed51a0ccf7 Mon Sep 17 00:00:00 2001 From: "Eric G. Kratz" Date: Thu, 21 Sep 2023 08:36:18 -0400 Subject: [PATCH 4/4] Update CHANGELOG.md Co-authored-by: Ferran Brosa Planella --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6198b612a8..8a93addf65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ ## Bug fixes -- Fixed a bug where empty lists passed to QuickPlot resulted in an IndexError and did not return a meaningful error message. +- Fixed a bug where empty lists passed to QuickPlot resulted in an IndexError and did not return a meaningful error message ([#3359](https://github.com/pybamm-team/PyBaMM/pull/3359)) - Fixed a bug where there was a missing thermal conductivity in the thermal pouch cell models ([#3330](https://github.com/pybamm-team/PyBaMM/pull/3330)) - Fixed a bug that caused incorrect results of “{Domain} electrode thickness change [m]” due to the absence of dimension for the variable `electrode_thickness_change`([#3329](https://github.com/pybamm-team/PyBaMM/pull/3329)). - Fixed a bug that occured in `check_ys_are_not_too_large` when trying to reference `y-slice` where the referenced variable was not a `pybamm.StateVector` ([#3313](https://github.com/pybamm-team/PyBaMM/pull/3313)