Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix switch to ruff format #3492

Closed
wants to merge 5 commits into from
Closed

Fix switch to ruff format #3492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d69819f
Fix switch to ruff format for that done some changes in pre-commit-co…
Rjchauhan18 Oct 31, 2023
6288242
removed arg : [--check] from pre-commit-config.yaml
Rjchauhan18 Nov 1, 2023
948030c
style: pre-commit fixes
pre-commit-ci[bot] Nov 1, 2023
10ec2d0
Added activated standard pre-commits for ruff-format to .git-blame-ig…
Rjchauhan18 Nov 4, 2023
2ff87e1
style: pre-commit fixes
pre-commit-ci[bot] Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .git-blame-ignore-revs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
a63e49ece0f9336d1f5c2562f7459e555c6e6693
# activated standard pre-commits - https://github.com/pybamm-team/PyBaMM/pull/3192
5273214b585c5a4286609aed40e0b092d0e05f42
# activated standard pre-commits for ruff-format - https://github.com/pybamm-team/PyBaMM/pull/3492
948030c5eddcae78e5faf2401e16be3080af30fb
2 changes: 2 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ repos:
args: [--fix, --show-fixes]
types_or: [python, pyi, jupyter]

- id: ruff-format
Comment on lines 12 to +13
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- id: ruff-format
- id: ruff-format
types_or: [python, pyi, jupyter]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: "v0.1.3"
    hooks:
      - id: ruff
        args: [--fix, --show-fixes]
   

      - id: ruff-format
        types_or: [python, pyi, jupyter]

It is removing ; from .ipynb

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a lot of the semi-colons just stop output like:

<pybamm.solvers.solution.Solution at 0x13cc02f10>

This does not seem like a huge problem.

Another way to resolve that printing is to save output objects to a discarded variable:

_ = sim.solve()

instead of

sim.solve();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, _ = sim.solve() this is in the jupyter notebook

But for plotting graphs without line like this [<matplotlib.lines.Line2D at 0x7f4123ea7ac0>] we need semi-colons.

One Solution is to put # fmt: off and # fmt: on where semi-colons appear

# fmt: off
plt.plot(sol["Time [s]"].data, sol["Voltage [V]"].data);
# fmt: on

Should I use these comments or try to find something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would prefer to avoid adding tool specific comments if there are other ways.

In this case:

_ = plt.plot(sol["Time [s]"].data, sol["Voltage [V]"].data)

works since plot() also has a return

Copy link
Contributor Author

@Rjchauhan18 Rjchauhan18 Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @Saransh-cpp
which changes you prefer ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semicolon thing was fixed in ruff upstream. You might have to reset your commits and introduce ruff format in an entirely new commit, given that we want all the pre-commit related changes bundled in a single commit.


- repo: https://github.com/adamchainz/blacken-docs
rev: "1.16.0"
hooks:
Expand Down
2 changes: 1 addition & 1 deletion examples/scripts/compare_comsol/discharge_curve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
plt.grid(True)
plt.xlabel(r"Discharge Capacity (Ah)", fontsize=20)
plt.ylabel(r"$\vert V - V_{comsol} \vert$", fontsize=20)
colors = iter(plt.cycler(color='bgrcmyk'))
colors = iter(plt.cycler(color="bgrcmyk"))

for key, C_rate in C_rates.items():
current = 24 * C_rate
Expand Down
47 changes: 24 additions & 23 deletions noxfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"SUNDIALS_INST": f"{homedir}/.local",
"LD_LIBRARY_PATH": f"{homedir}/.local/lib:",
}
VENV_DIR = Path('./venv').resolve()
VENV_DIR = Path("./venv").resolve()


def set_environment_variables(env_dict, session):
Expand Down Expand Up @@ -122,12 +122,13 @@ def set_dev(session):
session.run("virtualenv", os.fsdecode(VENV_DIR), silent=True)
python = os.fsdecode(VENV_DIR.joinpath("bin/python"))
if sys.platform == "linux":
session.run(python,
"-m",
"pip",
"install",
".[all,dev,jax,odes]",
external=True,
session.run(
python,
"-m",
"pip",
"install",
".[all,dev,jax,odes]",
external=True,
)
else:
session.run(python, "-m", "pip", "install", "-e", ".[all,dev]", external=True)
Expand All @@ -153,26 +154,26 @@ def build_docs(session):
# Local development
if session.interactive:
session.run(
"sphinx-autobuild",
"-j",
"auto",
"--open-browser",
"-qT",
".",
f"{envbindir}/../tmp/html",
"sphinx-autobuild",
"-j",
"auto",
"--open-browser",
"-qT",
".",
f"{envbindir}/../tmp/html",
)
# Runs in CI only, treating warnings as errors
else:
session.run(
"sphinx-build",
"-j",
"auto",
"-b",
"html",
"-W",
"--keep-going",
".",
f"{envbindir}/../tmp/html",
"sphinx-build",
"-j",
"auto",
"-b",
"html",
"-W",
"--keep-going",
".",
f"{envbindir}/../tmp/html",
)


Expand Down
8 changes: 2 additions & 6 deletions pybamm/discretisations/discretisation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,9 +514,7 @@ def check_tab_conditions(self, symbol, bcs):
if domain != "current collector":
raise pybamm.ModelError(
"""Boundary conditions can only be applied on the tabs in the domain
'current collector', but {} has domain {}""".format(
symbol, domain
)
'current collector', but {} has domain {}""".format(symbol, domain)
)
# Replace keys with "left" and "right" as appropriate for 1D meshes
if isinstance(mesh, pybamm.SubMesh1D):
Expand Down Expand Up @@ -900,9 +898,7 @@ def _process_symbol(self, symbol):
No key set for variable '{}'. Make sure it is included in either
model.rhs or model.algebraic in an unmodified form
(e.g. not Broadcasted)
""".format(
symbol.name
)
""".format(symbol.name)
)
# Add symbol's reference and multiply by the symbol's scale
# so that the state vector is of order 1
Expand Down
8 changes: 2 additions & 6 deletions pybamm/expression_tree/averages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ def z_average(symbol):
if symbol.domain not in [[], ["current collector"]]:
raise pybamm.DomainError(
"""z-average only implemented in the 'current collector' domain,
but symbol has domains {}""".format(
symbol.domain
)
but symbol has domains {}""".format(symbol.domain)
)
# If symbol doesn't have a domain, its average value is itself
if symbol.domain == []:
Expand Down Expand Up @@ -225,9 +223,7 @@ def yz_average(symbol):
if symbol.domain not in [[], ["current collector"]]:
raise pybamm.DomainError(
"""y-z-average only implemented in the 'current collector' domain,
but symbol has domains {}""".format(
symbol.domain
)
but symbol has domains {}""".format(symbol.domain)
)
# If symbol doesn't have a domain, its average value is itself
if symbol.domain == []:
Expand Down
8 changes: 2 additions & 6 deletions pybamm/expression_tree/binary_operators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,7 @@ def _binary_jac(self, left_jac, right_jac):
raise NotImplementedError(
"""jac of 'MatrixMultiplication' is only
implemented for left of type 'pybamm.Array',
not {}""".format(
left.__class__
)
not {}""".format(left.__class__)
)

def _binary_evaluate(self, left, right):
Expand Down Expand Up @@ -1341,9 +1339,7 @@ def source(left, right, boundary=False):
if left.domain != ["current collector"] or right.domain != ["current collector"]:
raise pybamm.DomainError(
"""'source' only implemented in the 'current collector' domain,
but symbols have domains {} and {}""".format(
left.domain, right.domain
)
but symbols have domains {} and {}""".format(left.domain, right.domain)
)
if boundary:
return pybamm.BoundaryMass(right) @ left
Expand Down
4 changes: 2 additions & 2 deletions pybamm/expression_tree/concatenations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ def __init__(self, *children):
*children,
name="numpy_concatenation",
check_domain=False,
concat_fun=np.concatenate
concat_fun=np.concatenate,
)

def _concatenation_jac(self, children_jacs):
Expand Down Expand Up @@ -340,7 +340,7 @@ def __init__(self, *children):
*children,
name="sparse_stack",
check_domain=False,
concat_fun=concatenation_function
concat_fun=concatenation_function,
)

def _concatenation_new_copy(self, children):
Expand Down
2 changes: 1 addition & 1 deletion pybamm/expression_tree/functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ def __init__(self, child):

def _function_diff(self, children, idx):
"""See :meth:`pybamm.Function._function_diff()`."""
return 2 / np.sqrt(np.pi) * exp(-children[0] ** 2)
return 2 / np.sqrt(np.pi) * exp(-(children[0] ** 2))


def erf(child):
Expand Down
4 changes: 1 addition & 3 deletions pybamm/expression_tree/operations/convert_to_casadi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,5 @@ def _convert(self, symbol, t, y, y_dot, inputs):
"""
Cannot convert symbol of type '{}' to CasADi. Symbols must all be
'linear algebra' at this stage.
""".format(
type(symbol)
)
""".format(type(symbol))
)
4 changes: 1 addition & 3 deletions pybamm/expression_tree/symbol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,7 @@ def create_copy(self):
"""
raise NotImplementedError(
"""method self.new_copy() not implemented
for symbol {!s} of type {}""".format(
self, type(self)
)
for symbol {!s} of type {}""".format(self, type(self))
)

def new_copy(self):
Expand Down
4 changes: 1 addition & 3 deletions pybamm/expression_tree/vector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ def __init__(
raise ValueError(
"""
Entries must have 1 dimension or be column vector, not have shape {}
""".format(
entries.shape
)
""".format(entries.shape)
)
if name is None:
name = "Column vector of length {!s}".format(entries.shape[0])
Expand Down
8 changes: 2 additions & 6 deletions pybamm/input/parameters/lithium_ion/Ai2020.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ def graphite_electrolyte_exchange_current_density_Dualfoil1998(
E_r = 5000 # activation energy for Temperature Dependent Reaction Constant [J/mol]
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def graphite_entropy_Enertech_Ai2020_function(sto, c_s_max):
Expand Down Expand Up @@ -272,9 +270,7 @@ def lico2_electrolyte_exchange_current_density_Dualfoil1998(c_e, c_s_surf, c_s_m
E_r = 5000
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def lico2_entropic_change_Ai2020_function(sto, c_s_max):
Expand Down
8 changes: 2 additions & 6 deletions pybamm/input/parameters/lithium_ion/Chen2020.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ def graphite_LGM50_electrolyte_exchange_current_density_Chen2020(
E_r = 35000
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def nmc_LGM50_ocp_Chen2020(sto):
Expand Down Expand Up @@ -141,9 +139,7 @@ def nmc_LGM50_electrolyte_exchange_current_density_Chen2020(c_e, c_s_surf, c_s_m
E_r = 17800
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def electrolyte_diffusivity_Nyman2008(c_e, T):
Expand Down
12 changes: 3 additions & 9 deletions pybamm/input/parameters/lithium_ion/Chen2020_composite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ def graphite_LGM50_electrolyte_exchange_current_density_Chen2020(
E_r = 35000
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def silicon_ocp_lithiation_Mark2016(sto):
Expand Down Expand Up @@ -167,9 +165,7 @@ def silicon_LGM50_electrolyte_exchange_current_density_Chen2020(
E_r = 35000
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def nmc_LGM50_ocp_Chen2020(sto):
Expand Down Expand Up @@ -238,9 +234,7 @@ def nmc_LGM50_electrolyte_exchange_current_density_Chen2020(c_e, c_s_surf, c_s_m
E_r = 17800
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def electrolyte_diffusivity_Nyman2008(c_e, T):
Expand Down
8 changes: 2 additions & 6 deletions pybamm/input/parameters/lithium_ion/Ecker2015.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,7 @@ def graphite_electrolyte_exchange_current_density_Ecker2015(c_e, c_s_surf, c_s_m
E_r / (pybamm.constants.R * 296.15)
)

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def nco_diffusivity_Ecker2015(sto, T):
Expand Down Expand Up @@ -292,9 +290,7 @@ def nco_electrolyte_exchange_current_density_Ecker2015(c_e, c_s_surf, c_s_max, T
E_r / (pybamm.constants.R * 296.15)
)

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def electrolyte_diffusivity_Ecker2015(c_e, T):
Expand Down
4 changes: 1 addition & 3 deletions pybamm/input/parameters/lithium_ion/Ecker2015_graphite_halfcell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,7 @@ def graphite_electrolyte_exchange_current_density_Ecker2015(c_e, c_s_surf, c_s_m
E_r / (pybamm.constants.R * 296.15)
)

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def electrolyte_diffusivity_Ecker2015(c_e, T):
Expand Down
8 changes: 2 additions & 6 deletions pybamm/input/parameters/lithium_ion/Marquis2019.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,7 @@ def graphite_electrolyte_exchange_current_density_Dualfoil1998(
E_r = 37480
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def graphite_entropic_change_Moura2016(sto, c_s_max):
Expand Down Expand Up @@ -221,9 +219,7 @@ def lico2_electrolyte_exchange_current_density_Dualfoil1998(c_e, c_s_surf, c_s_m
E_r = 39570
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def lico2_entropic_change_Moura2016(sto, c_s_max):
Expand Down
14 changes: 5 additions & 9 deletions pybamm/input/parameters/lithium_ion/Mohtat2020.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ def graphite_electrolyte_exchange_current_density_PeymanMPM(c_e, c_s_surf, c_s_m
E_r = 37480
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def graphite_entropic_change_PeymanMPM(sto, c_s_max):
Expand Down Expand Up @@ -208,9 +206,7 @@ def NMC_electrolyte_exchange_current_density_PeymanMPM(c_e, c_s_surf, c_s_max, T
E_r = 39570
arrhenius = np.exp(E_r / pybamm.constants.R * (1 / 298.15 - 1 / T))

return (
m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5
)
return m_ref * arrhenius * c_e**0.5 * c_s_surf**0.5 * (c_s_max - c_s_surf) ** 0.5


def NMC_entropic_change_PeymanMPM(sto, c_s_max):
Expand Down Expand Up @@ -244,9 +240,9 @@ def NMC_entropic_change_PeymanMPM(sto, c_s_max):
- 0.5623 * 10 ** (-4) * np.exp(109.451 * sto - 100.006)
)

du_dT = (
-800 + 779 * u_eq - 284 * u_eq**2 + 46 * u_eq**3 - 2.8 * u_eq**4
) * 10 ** (-3)
du_dT = (-800 + 779 * u_eq - 284 * u_eq**2 + 46 * u_eq**3 - 2.8 * u_eq**4) * 10 ** (
-3
)

return du_dT

Expand Down
Loading
Loading