Enable Flake8-bugbear #3859

lorenzofavaro · 2024-03-03T18:20:59Z

Description

Flake8-bugbear has been enabled to improve code quality and make the codebase more consistent and compliant with style standards.

Fixes #3854

Several style issues and deprecated practices were identified, which have been addressed in this PR:

Type of change

New feature (non-breaking change which adds functionality)
Optimization (back-end change that speeds up the code)
Bug fix (non-breaking change which fixes an issue)

Key checklist:

No style issues: $ pre-commit run (or $ nox -s pre-commit)
All tests pass: $ python run-tests.py --all (or $ nox -s tests)
The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

review-notebook-app · 2024-03-03T18:21:05Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

codecov · 2024-03-03T19:12:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (5fa1070) to head (12a0f67).
Report is 2 commits behind head on develop.

❗ Current head 12a0f67 differs from pull request most recent head 3600062. Consider uploading reports for the commit 3600062 to get more accurate results

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3859   +/-   ##
========================================
  Coverage    99.60%   99.60%           
========================================
  Files          259      259           
  Lines        21270    21273    +3     
========================================
+ Hits         21186    21189    +3     
  Misses          84       84

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

agriyakhetarpal

This looks good, @lorenzofavaro – thanks! Could you please add the commit hashes where each rule was fixed along with a comment to the .git-blame-ignore-revs file?

There are ten commits here out of twelve where the rules were enforced (all atomic so that is good) – we could otherwise consider squashing them and then adding the hash for the single commit generated in a subsequent commit. I do not mind either of those options, but if someone else does have an opinion on this – please let it be known :)

CHANGELOG.md

pybamm/models/full_battery_models/lithium_ion/electrode_soh_half_cell.py

pybamm/expression_tree/concatenations.py

Saransh-cpp

Thanks, @lorenzofavaro! See the comments below -

tests/unit/test_solvers/test_processed_variable.py

tests/shared.py

scripts/install_KLU_Sundials.py

pybamm/solvers/idaklu_jax.py

kratman · 2024-03-07T01:29:01Z

@lorenzofavaro I resolved the conflict so that it would trigger CI

kratman

I think I am pretty happy with what is in here. Looks like a bunch of good fixes.

@Saransh-cpp Can you re-review?

arjxn-py

Thanks @lorenzofavaro, nice work.
Looks Good to me.

pybamm/expression_tree/symbol.py

Saransh-cpp

Thanks, @lorenzofavaro! This looks amazing! Thanks for the details explanations as well!

pybamm/solvers/idaklu_jax.py

agriyakhetarpal

Thanks for the reviews, everyone – and thanks for contributing, @lorenzofavaro. All we need is to add the relevant commit hashes along with a comment listing the fixed rules (or better squash the relevant commits and add a single hash) to the .git-blame-ignore-revs file – it looks like it was removed for some reason.

Note

In case someone else gets to merge this before I do – please do not squash-merge the PR.

Fix unused-loop-control-variable (B007) Fix function-uses-loop-variable (B023) Fix raise-without-from-inside-except (B904) Fix mutable-argument-default (B006) Fix no-explicit-stacklevel (B028) Fix get-attr-with-constant (B009) Fix loop-variable-overrides-iterator (B020) Fix unary-prefix-increment-decrement (B002) Fix function-call-in-default-argument (B008) Fix cached-instance-method (B019)

Bumps the actions group with 1 update: [awalsh128/cache-apt-pkgs-action](https://github.com/awalsh128/cache-apt-pkgs-action). Updates `awalsh128/cache-apt-pkgs-action` from 1.4.1 to 1.4.2 - [Release notes](https://github.com/awalsh128/cache-apt-pkgs-action/releases) - [Commits](awalsh128/cache-apt-pkgs-action@v1.4.1...v1.4.2) --- updated-dependencies: - dependency-name: awalsh128/cache-apt-pkgs-action dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…aMM import (pybamm-team#3848) * import `typing_extensions` as optional_dependency * Make `typing-extensions` a required dependency * Try forward referencing for `sympy` in `IndependentVariable` * Make `sympy` a required dependency * Update docs for SymPy * Import `sympy` without `have_optional_dependency` * Import `sympy` without `TYPE_CHECKING` * Changelog * Update CHANGELOG.md Co-authored-by: Agriya Khetarpal <[email protected]> * Rearrange position in changelog --------- Co-authored-by: Agriya Khetarpal <[email protected]> Co-authored-by: Eric G. Kratz <[email protected]>

* Rename have_optional_dependency * Change log * Fix import * style: pre-commit fixes * Update pybamm/util.py --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Arjun Verma <[email protected]>

* update changelog for pybamm-team#3862 * reword

* improve tutorials 1 to 3 * use new syntax for plot_voltage_components * style: pre-commit fixes * simplify notebook by removing drive cycle * add additional comment on dynamic_plot * improve tutorial 5 * remove the use of solution in notebook 5, this is introduced in notebook 6 * fix minor typo * minor text updates * style: pre-commit fixes * minor text updates plus extra links * minor text updates! * minor text updates * remove tutorial 10 as very similar to tutorial 3 in creating models * move and rename tutorial 11 to creating models section * style: pre-commit fixes * fix minor typo in current_with_time * implement Rob & Eric's comments * update docs links * Valentin's comments * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric G. Kratz <[email protected]>

* chore: update pre-commit hooks updates: - [github.com/astral-sh/ruff-pre-commit: v0.2.2 → v0.3.2](astral-sh/ruff-pre-commit@v0.2.2...v0.3.2) * style: pre-commit fixes * Update .git-blame-ignore-revs --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Agriya Khetarpal <[email protected]>

* added schedule to need-reply workflow * added schedule to need-reply workflow --------- Co-authored-by: Arjun Verma <[email protected]>

Saransh-cpp · 2024-03-13T19:02:16Z

All we need is to add the relevant commit hashes along with a comment listing the fixed rules (or better squash the relevant commits and add a single hash) to the .git-blame-ignore-revs file

We can skip this, given that the changes in this PR are not the typical formatting changes but actual code changes.

Saransh-cpp · 2024-03-13T19:02:40Z

So this can be squashed and merged

agriyakhetarpal · 2024-03-13T19:03:02Z

Let's go ahead with that, sure!

agriyakhetarpal · 2024-03-13T19:19:30Z

The squashed commit has a lot of attributed authors (😄), possibly due to the rebase from develop being out of sync. At least it's just a single commit.

* Enable flake8-bugbear Fix unused-loop-control-variable (B007) Fix function-uses-loop-variable (B023) Fix raise-without-from-inside-except (B904) Fix mutable-argument-default (B006) Fix no-explicit-stacklevel (B028) Fix get-attr-with-constant (B009) Fix loop-variable-overrides-iterator (B020) Fix unary-prefix-increment-decrement (B002) Fix function-call-in-default-argument (B008) Fix cached-instance-method (B019) * Show exception chain (B904) * Disable single lint check (B019) * delay xarray.DataArray initialization * revert example * Bump the actions group with 1 update (pybamm-team#3861) Bumps the actions group with 1 update: [awalsh128/cache-apt-pkgs-action](https://github.com/awalsh128/cache-apt-pkgs-action). Updates `awalsh128/cache-apt-pkgs-action` from 1.4.1 to 1.4.2 - [Release notes](https://github.com/awalsh128/cache-apt-pkgs-action/releases) - [Commits](awalsh128/cache-apt-pkgs-action@v1.4.1...v1.4.2) --- updated-dependencies: - dependency-name: awalsh128/cache-apt-pkgs-action dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Make `typing-extensions` and `sympy` required dependencies to fix PyBaMM import (pybamm-team#3848) * import `typing_extensions` as optional_dependency * Make `typing-extensions` a required dependency * Try forward referencing for `sympy` in `IndependentVariable` * Make `sympy` a required dependency * Update docs for SymPy * Import `sympy` without `have_optional_dependency` * Import `sympy` without `TYPE_CHECKING` * Changelog * Update CHANGELOG.md Co-authored-by: Agriya Khetarpal <[email protected]> * Rearrange position in changelog --------- Co-authored-by: Agriya Khetarpal <[email protected]> Co-authored-by: Eric G. Kratz <[email protected]> * Rename have_optional_dependency (pybamm-team#3866) * Rename have_optional_dependency * Change log * Fix import * style: pre-commit fixes * Update pybamm/util.py --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Arjun Verma <[email protected]> * update changelog for pybamm-team#3862 (pybamm-team#3869) * update changelog for pybamm-team#3862 * reword * fix deprecation warning and check for electrode/particle diffusivity * update test * Edit naming for unused loop control variable (B007) * pybamm-team#3883 updates to install from source (pybamm-team#3884) * Disable benchmarks on PRs (pybamm-team#3876) * Remove`[ latexify` extra (pybamm-team#3888) * Improve Getting Started notebooks (pybamm-team#3750) * improve tutorials 1 to 3 * use new syntax for plot_voltage_components * style: pre-commit fixes * simplify notebook by removing drive cycle * add additional comment on dynamic_plot * improve tutorial 5 * remove the use of solution in notebook 5, this is introduced in notebook 6 * fix minor typo * minor text updates * style: pre-commit fixes * minor text updates plus extra links * minor text updates! * minor text updates * remove tutorial 10 as very similar to tutorial 3 in creating models * move and rename tutorial 11 to creating models section * style: pre-commit fixes * fix minor typo in current_with_time * implement Rob & Eric's comments * update docs links * Valentin's comments * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Eric G. Kratz <[email protected]> * chore: update pre-commit hooks (pybamm-team#3890) * chore: update pre-commit hooks updates: - [github.com/astral-sh/ruff-pre-commit: v0.2.2 → v0.3.2](astral-sh/ruff-pre-commit@v0.2.2...v0.3.2) * style: pre-commit fixes * Update .git-blame-ignore-revs --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Agriya Khetarpal <[email protected]> * style: pre-commit fixes * Resolve conflicts (B904, B028, B020) * Added a schedule on `needs-reply remove` workflow (pybamm-team#3891) * added schedule to need-reply workflow * added schedule to need-reply workflow --------- Co-authored-by: Arjun Verma <[email protected]> * style: pre-commit fixes --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Valentin Sulzer <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Arjun Verma <[email protected]> Co-authored-by: Agriya Khetarpal <[email protected]> Co-authored-by: Eric G. Kratz <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Martin Robinson <[email protected]> Co-authored-by: Ferran Brosa Planella <[email protected]> Co-authored-by: Santhosh <[email protected]>

lorenzofavaro requested review from martinjrobins, Saransh-cpp, agriyakhetarpal, kratman, arjxn-py, brosaplanella, a team, rtimms, valentinsulzer, TomTranter and DrSOKane as code owners March 3, 2024 18:21

agriyakhetarpal reviewed Mar 3, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

pybamm/models/full_battery_models/lithium_ion/electrode_soh_half_cell.py Show resolved Hide resolved

lorenzofavaro force-pushed the flake8-bugbear-patch branch 2 times, most recently from a9d9b7f to 0a8c6ec Compare March 4, 2024 17:05

kratman reviewed Mar 4, 2024

View reviewed changes

pybamm/expression_tree/concatenations.py Outdated Show resolved Hide resolved

lorenzofavaro changed the title ~~Flake8-bugbear patch~~ Enable Flake8-bugbear Mar 4, 2024

Saransh-cpp requested changes Mar 5, 2024

View reviewed changes

tests/unit/test_solvers/test_processed_variable.py Show resolved Hide resolved

tests/shared.py Show resolved Hide resolved

scripts/install_KLU_Sundials.py Outdated Show resolved Hide resolved

pybamm/solvers/idaklu_jax.py Outdated Show resolved Hide resolved

kratman approved these changes Mar 9, 2024

View reviewed changes

arjxn-py approved these changes Mar 9, 2024

View reviewed changes

pybamm/expression_tree/symbol.py Show resolved Hide resolved

lorenzofavaro requested a review from Saransh-cpp March 11, 2024 10:57

Saransh-cpp approved these changes Mar 12, 2024

View reviewed changes

pybamm/solvers/idaklu_jax.py Show resolved Hide resolved

Saransh-cpp approved these changes Mar 13, 2024

View reviewed changes

pybamm/solvers/idaklu_jax.py Show resolved Hide resolved

agriyakhetarpal reviewed Mar 13, 2024

View reviewed changes

lorenzofavaro and others added 4 commits March 13, 2024 19:41

Show exception chain (B904)

17bcc6c

Disable single lint check (B019)

4c7de48

delay xarray.DataArray initialization

9271b1a

valentinsulzer and others added 16 commits March 13, 2024 19:41

revert example

4c5f594

update changelog for pybamm-team#3862 (pybamm-team#3869)

4fcf584

* update changelog for pybamm-team#3862 * reword

fix deprecation warning and check for electrode/particle diffusivity

724040b

update test

fce31a6

Edit naming for unused loop control variable (B007)

1034e70

pybamm-team#3883 updates to install from source (pybamm-team#3884)

393060e

Disable benchmarks on PRs (pybamm-team#3876)

971be85

Remove[ latexify extra (pybamm-team#3888)

9c9b436

style: pre-commit fixes

34d9e71

Resolve conflicts (B904, B028, B020)

870656f

Added a schedule on needs-reply remove workflow (pybamm-team#3891)

8a89e86

* added schedule to need-reply workflow * added schedule to need-reply workflow --------- Co-authored-by: Arjun Verma <[email protected]>

lorenzofavaro force-pushed the flake8-bugbear-patch branch from 12a0f67 to 8a89e86 Compare March 13, 2024 18:49

lorenzofavaro and others added 2 commits March 13, 2024 20:00

Merge branch 'develop' into flake8-bugbear-patch

2cdaabb

style: pre-commit fixes

3600062

agriyakhetarpal approved these changes Mar 13, 2024

View reviewed changes

agriyakhetarpal merged commit fd39eba into pybamm-team:develop Mar 13, 2024
44 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Flake8-bugbear #3859

Enable Flake8-bugbear #3859

lorenzofavaro commented Mar 3, 2024 •

edited

Loading

review-notebook-app bot commented Mar 3, 2024

codecov bot commented Mar 3, 2024 •

edited

Loading

agriyakhetarpal left a comment •

edited

Loading

Saransh-cpp left a comment

kratman commented Mar 7, 2024

kratman left a comment

arjxn-py left a comment

Saransh-cpp left a comment

agriyakhetarpal left a comment •

edited

Loading

Saransh-cpp commented Mar 13, 2024

Saransh-cpp commented Mar 13, 2024

agriyakhetarpal commented Mar 13, 2024

agriyakhetarpal commented Mar 13, 2024

Enable Flake8-bugbear #3859

Enable Flake8-bugbear #3859

Conversation

lorenzofavaro commented Mar 3, 2024 • edited Loading

Description

Type of change

Key checklist:

review-notebook-app bot commented Mar 3, 2024

codecov bot commented Mar 3, 2024 • edited Loading

Codecov Report

agriyakhetarpal left a comment • edited Loading

Choose a reason for hiding this comment

Saransh-cpp left a comment

Choose a reason for hiding this comment

kratman commented Mar 7, 2024

kratman left a comment

Choose a reason for hiding this comment

arjxn-py left a comment

Choose a reason for hiding this comment

Saransh-cpp left a comment

Choose a reason for hiding this comment

agriyakhetarpal left a comment • edited Loading

Choose a reason for hiding this comment

Saransh-cpp commented Mar 13, 2024

Saransh-cpp commented Mar 13, 2024

agriyakhetarpal commented Mar 13, 2024

agriyakhetarpal commented Mar 13, 2024

lorenzofavaro commented Mar 3, 2024 •

edited

Loading

codecov bot commented Mar 3, 2024 •

edited

Loading

agriyakhetarpal left a comment •

edited

Loading

agriyakhetarpal left a comment •

edited

Loading