From b0382e6f1e08ef28987be852fe7fad2f94cee750 Mon Sep 17 00:00:00 2001 From: "Eric G. Kratz" Date: Mon, 4 Nov 2024 20:28:32 -0500 Subject: [PATCH] Cleanup formatting in error messages (#4565) * Remove some unneeded links * Cleaning up some formatting * Update src/pybamm/discretisations/discretisation.py * Update src/pybamm/expression_tree/operations/convert_to_casadi.py --- CONTRIBUTING.md | 6 +++--- src/pybamm/discretisations/discretisation.py | 12 +++++------- src/pybamm/expression_tree/averages.py | 8 ++++---- src/pybamm/expression_tree/binary_operators.py | 12 ++++++------ .../operations/convert_to_casadi.py | 6 ++---- src/pybamm/expression_tree/unary_operators.py | 4 ++-- src/pybamm/expression_tree/vector.py | 4 +--- src/pybamm/meshes/one_dimensional_submeshes.py | 4 ++-- src/pybamm/meshes/scikit_fem_submeshes.py | 10 +++++----- src/pybamm/models/base_model.py | 14 +++++--------- src/pybamm/simulation.py | 14 ++++++-------- src/pybamm/solvers/casadi_algebraic_solver.py | 8 +++----- 12 files changed, 44 insertions(+), 58 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fd3426bd3b..eb510f7054 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,7 @@ You now have everything you need to start making changes! ### B. Writing your code -6. PyBaMM is developed in [Python](https://www.python.org)), and makes heavy use of [NumPy](https://numpy.org/) (see also [NumPy for MatLab users](https://numpy.org/doc/stable/user/numpy-for-matlab-users.html) and [Python for R users](https://www.rebeccabarter.com/blog/2023-09-11-from_r_to_python)). +6. PyBaMM is developed in [Python](https://www.python.org), and makes heavy use of [NumPy](https://numpy.org/). 7. Make sure to follow our [coding style guidelines](#coding-style-guidelines). 8. Commit your changes to your branch with [useful, descriptive commit messages](https://chris.beams.io/posts/git-commit/): Remember these are publicly visible and should still make sense a few months ahead in time. @@ -116,8 +116,8 @@ PyBaMM provides a utility function `import_optional_dependency`, to check for th Optional dependencies should never be imported at the module level, but always inside methods. For example: -``` -def use_pybtex(x,y,z): +```python +def use_pybtex(x, y, z): pybtex = import_optional_dependency("pybtex") ... ``` diff --git a/src/pybamm/discretisations/discretisation.py b/src/pybamm/discretisations/discretisation.py index 2ca8c87649..3d9579ff9c 100644 --- a/src/pybamm/discretisations/discretisation.py +++ b/src/pybamm/discretisations/discretisation.py @@ -500,8 +500,8 @@ def check_tab_conditions(self, symbol, bcs): if domain != "current collector": raise pybamm.ModelError( - f"""Boundary conditions can only be applied on the tabs in the domain - 'current collector', but {symbol} has domain {domain}""" + "Boundary conditions can only be applied on the tabs in the domain " + f"'current collector', but {symbol} has domain {domain}" ) # Replace keys with "left" and "right" as appropriate for 1D meshes if isinstance(mesh, pybamm.SubMesh1D): @@ -893,11 +893,9 @@ def _process_symbol(self, symbol): y_slices = self.y_slices[symbol] except KeyError as error: raise pybamm.ModelError( - f""" - No key set for variable '{symbol.name}'. Make sure it is included in either - model.rhs or model.algebraic in an unmodified form - (e.g. not Broadcasted) - """ + f"No key set for variable '{symbol.name}'. Make sure it is included in either " + "model.rhs or model.algebraic in an unmodified form " + "(e.g. not Broadcasted)" ) from error # Add symbol's reference and multiply by the symbol's scale # so that the state vector is of order 1 diff --git a/src/pybamm/expression_tree/averages.py b/src/pybamm/expression_tree/averages.py index 5fa6c5f00f..11538ea153 100644 --- a/src/pybamm/expression_tree/averages.py +++ b/src/pybamm/expression_tree/averages.py @@ -251,8 +251,8 @@ def z_average(symbol: pybamm.Symbol) -> pybamm.Symbol: # Symbol must have domain [] or ["current collector"] if symbol.domain not in [[], ["current collector"]]: raise pybamm.DomainError( - f"""z-average only implemented in the 'current collector' domain, - but symbol has domains {symbol.domain}""" + "z-average only implemented in the 'current collector' domain, " + f"but symbol has domains {symbol.domain}" ) # If symbol doesn't have a domain, its average value is itself if symbol.domain == []: @@ -285,8 +285,8 @@ def yz_average(symbol: pybamm.Symbol) -> pybamm.Symbol: # Symbol must have domain [] or ["current collector"] if symbol.domain not in [[], ["current collector"]]: raise pybamm.DomainError( - f"""y-z-average only implemented in the 'current collector' domain, - but symbol has domains {symbol.domain}""" + "y-z-average only implemented in the 'current collector' domain, " + f"but symbol has domains {symbol.domain}" ) # If symbol doesn't have a domain, its average value is itself if symbol.domain == []: diff --git a/src/pybamm/expression_tree/binary_operators.py b/src/pybamm/expression_tree/binary_operators.py index 1d630887b2..42f2f74e24 100644 --- a/src/pybamm/expression_tree/binary_operators.py +++ b/src/pybamm/expression_tree/binary_operators.py @@ -36,7 +36,7 @@ def _preprocess_binary( # Check both left and right are pybamm Symbols if not (isinstance(left, pybamm.Symbol) and isinstance(right, pybamm.Symbol)): raise NotImplementedError( - f"""BinaryOperator not implemented for symbols of type {type(left)} and {type(right)}""" + f"BinaryOperator not implemented for symbols of type {type(left)} and {type(right)}" ) # Do some broadcasting in special cases, to avoid having to do this manually @@ -389,9 +389,9 @@ def _binary_jac(self, left_jac, right_jac): return left @ right_jac else: raise NotImplementedError( - f"""jac of 'MatrixMultiplication' is only - implemented for left of type 'pybamm.Array', - not {left.__class__}""" + f"jac of 'MatrixMultiplication' is only " + "implemented for left of type 'pybamm.Array', " + f"not {left.__class__}" ) def _binary_evaluate(self, left, right): @@ -1541,8 +1541,8 @@ def source( if left.domain != ["current collector"] or right.domain != ["current collector"]: raise pybamm.DomainError( - f"""'source' only implemented in the 'current collector' domain, - but symbols have domains {left.domain} and {right.domain}""" + "'source' only implemented in the 'current collector' domain, " + f"but symbols have domains {left.domain} and {right.domain}" ) if boundary: return pybamm.BoundaryMass(right) @ left diff --git a/src/pybamm/expression_tree/operations/convert_to_casadi.py b/src/pybamm/expression_tree/operations/convert_to_casadi.py index 274fd95154..af53b9acd8 100644 --- a/src/pybamm/expression_tree/operations/convert_to_casadi.py +++ b/src/pybamm/expression_tree/operations/convert_to_casadi.py @@ -231,8 +231,6 @@ def _convert(self, symbol, t, y, y_dot, inputs): else: raise TypeError( - f""" - Cannot convert symbol of type '{type(symbol)}' to CasADi. Symbols must all be - 'linear algebra' at this stage. - """ + f"Cannot convert symbol of type '{type(symbol)}' to CasADi. Symbols must all be " + "'linear algebra' at this stage." ) diff --git a/src/pybamm/expression_tree/unary_operators.py b/src/pybamm/expression_tree/unary_operators.py index aa90fd6f4c..f41897e2de 100644 --- a/src/pybamm/expression_tree/unary_operators.py +++ b/src/pybamm/expression_tree/unary_operators.py @@ -992,8 +992,8 @@ def __init__(self, name, child, side): if side in ["negative tab", "positive tab"]: if child.domain[0] != "current collector": raise pybamm.ModelError( - f"""Can only take boundary value on the tabs in the domain - 'current collector', but {child} has domain {child.domain[0]}""" + "Can only take boundary value on the tabs in the domain " + f"'current collector', but {child} has domain {child.domain[0]}" ) self.side = side # boundary value of a child takes the primary domain from secondary domain diff --git a/src/pybamm/expression_tree/vector.py b/src/pybamm/expression_tree/vector.py index 6dc358afb0..e9067a4ffd 100644 --- a/src/pybamm/expression_tree/vector.py +++ b/src/pybamm/expression_tree/vector.py @@ -29,9 +29,7 @@ def __init__( entries = entries[:, np.newaxis] if entries.shape[1] != 1: raise ValueError( - f""" - Entries must have 1 dimension or be column vector, not have shape {entries.shape} - """ + f"Entries must have 1 dimension or be column vector, not have shape {entries.shape}" ) if name is None: name = f"Column vector of length {entries.shape[0]!s}" diff --git a/src/pybamm/meshes/one_dimensional_submeshes.py b/src/pybamm/meshes/one_dimensional_submeshes.py index 8f27049411..027c6d0421 100644 --- a/src/pybamm/meshes/one_dimensional_submeshes.py +++ b/src/pybamm/meshes/one_dimensional_submeshes.py @@ -297,8 +297,8 @@ def __init__(self, lims, npts, edges=None): if (npts + 1) != len(edges): raise pybamm.GeometryError( - f"""User-suppled edges has should have length (npts + 1) but has length - {len(edges)}.Number of points (npts) for domain {spatial_var.domain} is {npts}.""".replace( + "User-suppled edges has should have length (npts + 1) but has length " + f"{len(edges)}.Number of points (npts) for domain {spatial_var.domain} is {npts}.".replace( "\n ", " " ) ) diff --git a/src/pybamm/meshes/scikit_fem_submeshes.py b/src/pybamm/meshes/scikit_fem_submeshes.py index ba624c7f48..2d769f3ca2 100644 --- a/src/pybamm/meshes/scikit_fem_submeshes.py +++ b/src/pybamm/meshes/scikit_fem_submeshes.py @@ -92,8 +92,8 @@ def read_lims(self, lims): # check coordinate system agrees if spatial_vars[0].coord_sys != spatial_vars[1].coord_sys: raise pybamm.DomainError( - f"""spatial variables should have the same coordinate system, - but have coordinate systems {spatial_vars[0].coord_sys} and {spatial_vars[1].coord_sys}""" + "spatial variables should have the same coordinate system, " + f"but have coordinate systems {spatial_vars[0].coord_sys} and {spatial_vars[1].coord_sys}" ) return spatial_vars, tabs @@ -360,9 +360,9 @@ def __init__(self, lims, npts, y_edges=None, z_edges=None): # check that npts equals number of user-supplied edges if npts[var.name] != len(edges[var.name]): raise pybamm.GeometryError( - f"""User-suppled edges has should have length npts but has length {len(edges[var.name])}. - Number of points (npts) for variable {var.name} in - domain {var.domain} is {npts[var.name]}.""" + f"User-supplied edges has should have length npts but has length {len(edges[var.name])}. " + f"Number of points (npts) for variable {var.name} in " + f"domain {var.domain} is {npts[var.name]}." ) # check end points of edges agree with spatial_lims diff --git a/src/pybamm/models/base_model.py b/src/pybamm/models/base_model.py index f7e8f70f32..6cb3af2fd7 100644 --- a/src/pybamm/models/base_model.py +++ b/src/pybamm/models/base_model.py @@ -1110,7 +1110,7 @@ def check_ics_bcs(self): for var in self.rhs.keys(): if var not in self.initial_conditions.keys(): raise pybamm.ModelError( - f"""no initial condition given for variable '{var}'""" + f"no initial condition given for variable '{var}'" ) def check_variables(self): @@ -1132,11 +1132,9 @@ def check_variables(self): for var in all_vars: if var not in vars_in_keys: raise pybamm.ModelError( - f""" - No key set for variable '{var}'. Make sure it is included in either - model.rhs or model.algebraic, in an unmodified form - (e.g. not Broadcasted) - """ + f"No key set for variable '{var}'. Make sure it is included in either " + "model.rhs or model.algebraic, in an unmodified form " + "(e.g. not Broadcasted)" ) def check_no_repeated_keys(self): @@ -1550,9 +1548,7 @@ def check_and_convert_bcs(self, boundary_conditions): # Check types if bc[1] not in ["Dirichlet", "Neumann"]: raise pybamm.ModelError( - f""" - boundary condition types must be Dirichlet or Neumann, not '{bc[1]}' - """ + f"boundary condition types must be Dirichlet or Neumann, not '{bc[1]}'" ) return boundary_conditions diff --git a/src/pybamm/simulation.py b/src/pybamm/simulation.py index da0ac08316..ab22972741 100644 --- a/src/pybamm/simulation.py +++ b/src/pybamm/simulation.py @@ -519,14 +519,12 @@ def solve( dt_eval_max = np.max(np.diff(t_eval)) if dt_eval_max > np.nextafter(dt_data_min, np.inf): warnings.warn( - f""" - The largest timestep in t_eval ({dt_eval_max}) is larger than - the smallest timestep in the data ({dt_data_min}). The returned - solution may not have the correct resolution to accurately - capture the input. Try refining t_eval. Alternatively, - passing t_eval = None automatically sets t_eval to be the - points in the data. - """, + f"The largest timestep in t_eval ({dt_eval_max}) is larger than " + f"the smallest timestep in the data ({dt_data_min}). The returned " + "solution may not have the correct resolution to accurately " + "capture the input. Try refining t_eval. Alternatively, " + "passing t_eval = None automatically sets t_eval to be the " + "points in the data.", pybamm.SolverWarning, stacklevel=2, ) diff --git a/src/pybamm/solvers/casadi_algebraic_solver.py b/src/pybamm/solvers/casadi_algebraic_solver.py index 2dd6f2d341..b139199f8c 100644 --- a/src/pybamm/solvers/casadi_algebraic_solver.py +++ b/src/pybamm/solvers/casadi_algebraic_solver.py @@ -146,11 +146,9 @@ def _integrate(self, model, t_eval, inputs_dict=None, t_interp=None): ) else: raise pybamm.SolverError( - f""" - Could not find acceptable solution: solver terminated - successfully, but maximum solution error ({casadi.mmax(casadi.fabs(fun))}) - above tolerance ({self.tol}) - """ + "Could not find acceptable solution: solver terminated " + f"successfully, but maximum solution error ({casadi.mmax(casadi.fabs(fun))}) " + f"above tolerance ({self.tol})" ) # Concatenate differential part