Using pyupgrade to convert string formatting to `f-string`. #3579

prady0t · 2023-11-30T21:05:51Z

Fixes #3496

Linked to PR #3551

review-notebook-app · 2023-11-30T21:05:56Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Saransh-cpp

Thanks for working on this, @prady0t!

Not all pre-commit related changes were included in 41019cf, so you might have to reset the commits and include all the changes in a single commit.
Some pre-commit suggestions must be resolved manually. These are visible here - https://results.pre-commit.ci/run/github/155538761/1701378735.92sVUq40SsSDBx8YpLEX4w. You can access this by scrolling to the bottom of your PR and clicking "details" on the pre-commit.ci job.
There are a few typing fixes required for the pre-commit job to pass. Adding UP007 to the list of excluded checks in pyproject.toml with a small note should fix this for now. (@pipliggins could fix these and add the check back in Adds typing to expression tree #3578.)

prady0t · 2023-12-01T22:36:30Z

Thanks for working on this, @prady0t!

Not all pre-commit related changes were included in 41019cf, so you might have to reset the commits and include all the changes in a single commit.

Some pre-commit suggestions must be resolved manually. These are visible here - https://results.pre-commit.ci/run/github/155538761/1701378735.92sVUq40SsSDBx8YpLEX4w. You can access this by scrolling to the bottom of your PR and clicking "details" on the pre-commit.ci job.

There are a few typing fixes required for the pre-commit job to pass. Adding UP007 to the list of excluded checks in pyproject.toml with a small note should fix this for now. (@pipliggins could fix these and add the check back in Adds typing to expression tree #3578.)

Am I supposed to add UP007 here?

PyBaMM/pyproject.toml

Line 202 in ebacf49

ignore = [

also how do I add commit hash (mentioned earlier)?

Saransh-cpp · 2023-12-08T14:05:07Z

Am I supposed to add UP007 here?

Yes!

also how do I add commit hash (mentioned earlier)?

You can add the commit hash in the git-revs files.

You should roughly do the following -

git reset --hard HEAD~4  # hard reset all the pre-commit changes (the last 4 commits on this branch)
# merge the remote develop branch into your current branch
# ignore UP007, add, and commit with --no-verify
pre-commit run -a   # run pre-commit on all files, this will trigger the newly added pyupgrade rules
# most of the fixes will be automatic but some fixes will require manual intervention - make manual fixes wherever required
# add and commit, this time not with --no-verify, allowing pre-commit to pass
# add the previous commit's hash to .git-blame-ignore-revs
# push everything

kratman · 2023-12-14T13:04:08Z

@prady0t How is this coming along?

prady0t · 2023-12-14T14:30:51Z

@prady0t How is this coming along?

Sorry for not being active. I had exams then had to travel back home. Was working on this just today, will complete this by EOD.

kratman · 2023-12-14T17:22:46Z

No rush, just curious since you were close

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

prady0t · 2023-12-14T17:31:36Z

Done! Please have a look.

prady0t · 2023-12-14T17:35:35Z

Pretty sure I messed up something. 😨

agriyakhetarpal · 2023-12-14T17:42:32Z

After you reset your branch locally you pulled the changes in it that were present on GitHub instead of a force push (I think). This is why 101 or so commits are showing up (probably due to a broken merge commit, the presence of which would have fixed it otherwise)

prady0t · 2023-12-14T17:47:11Z

I might have to close this PR as well. What do you think?

agriyakhetarpal · 2023-12-14T18:01:11Z

You can reset your branch and keep working on the same PR using the instructions posted above, no worries! Otherwise, you can undo your commits till 75b58bc is the last commit.

kratman · 2023-12-14T18:10:56Z

@prady0t You are so close. If you want I can post the full set of commands to do it with reset/cherry pick. The general procedure I would follow is

Reset to origin/main
Cherry pick your pre-commit config changes from this branch on your fork's origin (origin/new-format-fix)
Run pre-commit, commit formatting changes, copy the commit hash
Add the formatting changes hash to the ignored revs, and commit that change
Force push to this branch

Saransh-cpp · 2023-12-14T18:11:36Z

You can also git log the hashes of your last 4 commits, reset everything in this branch, and then git cherry-pick those 4 commits using their hashes.

This would be a nice git exercise but please feel free to open a new PR if you feel that would be better.

Edit: Just saw @kratman's comment, haha!

prady0t · 2023-12-14T18:49:07Z

Did this work? I basically just did this:

git reset --hard 75b58bc  
git push origin -f

agriyakhetarpal · 2023-12-14T18:53:06Z

Yes, you are back on track. All that is left is to follow the instructions in #3579 (comment)

prady0t · 2023-12-14T18:57:02Z

Yes, you are back on track. All that is left is to follow the instructions in #3579 (comment)

But all those commits are already there and ci test have passed as well. Do I have to do it again?

agriyakhetarpal

We should be done, then – these changes look great! Thanks, @prady0t! Let's wait and see if @Saransh-cpp requires any extra changes here

pybamm/solvers/processed_variable.py

Saransh-cpp

Looks good, thanks @prady0t! I could see some formatting issues but hopefully they'll be resolved once ruff format is added in. You can add all these changes in without having to worry about git blame.

pyproject.toml

pybamm/discretisations/discretisation.py

pybamm/models/full_battery_models/base_battery_model.py

Co-authored-by: Saransh Chopra <saransh0701@gmail.com> Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

prady0t · 2023-12-14T19:41:16Z

Thanks for being so supportive everyone 😄 .

kratman · 2023-12-14T19:45:36Z

@all-contributors please add @prady0t for infrastructure

allcontributors · 2023-12-14T19:45:45Z

@kratman

I've put up a pull request to add @prady0t! 🎉

codecov · 2023-12-14T19:54:29Z

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (903323e) 99.59% compared to head (6e9b373) 99.59%.

Files	Patch %	Lines
...bamm/expression_tree/operations/evaluate_python.py	95.45%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3579   +/-   ##
========================================
  Coverage    99.59%   99.59%           
========================================
  Files          258      258           
  Lines        20755    20755           
========================================
  Hits         20670    20670           
  Misses          85       85

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

pyproject.toml

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Saransh-cpp requested changes Dec 1, 2023

View reviewed changes

kratman self-requested a review December 1, 2023 14:52

agriyakhetarpal self-requested a review December 2, 2023 02:14

arjxn-py self-requested a review December 5, 2023 07:15

Changing pyproject config

f27aa2c

prady0t added 2 commits December 14, 2023 22:56

changed string formatting using pyupgrade

ff6d81c

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

added commit hash

75b58bc

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

prady0t force-pushed the new-format-fix branch from 8a6be25 to 75b58bc Compare December 14, 2023 18:46

style: pre-commit fixes

58d81a7

agriyakhetarpal approved these changes Dec 14, 2023

View reviewed changes

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

Saransh-cpp approved these changes Dec 14, 2023

View reviewed changes

Apply suggestions from code review

40a9dcf

Co-authored-by: Saransh Chopra <saransh0701@gmail.com> Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>

kratman approved these changes Dec 14, 2023

View reviewed changes

allcontributors bot mentioned this pull request Dec 14, 2023

docs: add prady0t as a contributor for infra #3620

Merged

Saransh-cpp reviewed Dec 15, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Update pyproject.toml

6e9b373

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Saransh-cpp merged commit 031d9a5 into pybamm-team:develop Dec 15, 2023
33 of 35 checks passed

prady0t deleted the new-format-fix branch December 17, 2023 07:16

pipliggins added a commit to pipliggins/PyBaMM that referenced this pull request Jan 19, 2024

Stop ignoring UP007 in ruff, as per pybamm-team#3579

9cef34b

js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this pull request Aug 12, 2024

Stop ignoring UP007 in ruff, as per pybamm-team#3579

5082baf

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using pyupgrade to convert string formatting to `f-string`. #3579

Using pyupgrade to convert string formatting to `f-string`. #3579

prady0t commented Nov 30, 2023

review-notebook-app bot commented Nov 30, 2023

Saransh-cpp left a comment

prady0t commented Dec 1, 2023

Saransh-cpp commented Dec 8, 2023

kratman commented Dec 14, 2023

prady0t commented Dec 14, 2023

kratman commented Dec 14, 2023

prady0t commented Dec 14, 2023

prady0t commented Dec 14, 2023

agriyakhetarpal commented Dec 14, 2023

prady0t commented Dec 14, 2023

agriyakhetarpal commented Dec 14, 2023

kratman commented Dec 14, 2023

Saransh-cpp commented Dec 14, 2023 •

edited

Loading

prady0t commented Dec 14, 2023

agriyakhetarpal commented Dec 14, 2023

prady0t commented Dec 14, 2023

agriyakhetarpal left a comment •

edited

Loading

Saransh-cpp left a comment •

edited

Loading

prady0t commented Dec 14, 2023

kratman commented Dec 14, 2023

allcontributors bot commented Dec 14, 2023

codecov bot commented Dec 14, 2023 •

edited

Loading

Using pyupgrade to convert string formatting to f-string. #3579

Using pyupgrade to convert string formatting to f-string. #3579

Conversation

prady0t commented Nov 30, 2023

review-notebook-app bot commented Nov 30, 2023

Saransh-cpp left a comment

Choose a reason for hiding this comment

prady0t commented Dec 1, 2023

Saransh-cpp commented Dec 8, 2023

kratman commented Dec 14, 2023

prady0t commented Dec 14, 2023

kratman commented Dec 14, 2023

prady0t commented Dec 14, 2023

prady0t commented Dec 14, 2023

agriyakhetarpal commented Dec 14, 2023

prady0t commented Dec 14, 2023

agriyakhetarpal commented Dec 14, 2023

kratman commented Dec 14, 2023

Saransh-cpp commented Dec 14, 2023 • edited Loading

prady0t commented Dec 14, 2023

agriyakhetarpal commented Dec 14, 2023

prady0t commented Dec 14, 2023

agriyakhetarpal left a comment • edited Loading

Choose a reason for hiding this comment

Saransh-cpp left a comment • edited Loading

Choose a reason for hiding this comment

prady0t commented Dec 14, 2023

kratman commented Dec 14, 2023

allcontributors bot commented Dec 14, 2023

codecov bot commented Dec 14, 2023 • edited Loading

Codecov Report

Using pyupgrade to convert string formatting to `f-string`. #3579

Using pyupgrade to convert string formatting to `f-string`. #3579

Saransh-cpp commented Dec 14, 2023 •

edited

Loading

agriyakhetarpal left a comment •

edited

Loading

Saransh-cpp left a comment •

edited

Loading

codecov bot commented Dec 14, 2023 •

edited

Loading