Rename one_plus_dlnf_dlnc to thermodynamic_factor

[jeromtom]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #2726

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all
• The documentation builds: $ python run-tests.py --doctest

You can run unit and doctests together at once, using $ python run-tests.py --quick.

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[jeromtom]

1 + dlnf/dlnc is referenced multiple times and cannot be replaced with Thermodynamic factor manually. I am in the process of creating a script to do the same.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (fe13b02) 99.68% compared to head (ac0acd6) 99.68%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2727   +/-   ##
========================================
  Coverage    99.68%   99.68%           
========================================
  Files          272      272           
  Lines        19005    19007    +2     
========================================
+ Hits         18946    18948    +2     
  Misses          59       59           

Impacted Files	Coverage Δ
pybamm/expression_tree/printing/print_name.py	100.00% <ø> (ø)
pybamm/input/parameters/lead_acid/Sulzer2019.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/Ai2020.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/Chen2020.py	100.00% <ø> (ø)
...input/parameters/lithium_ion/Chen2020_composite.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/Ecker2015.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/Marquis2019.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/Mohtat2020.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/NCA_Kim2011.py	100.00% <ø> (ø)
pybamm/input/parameters/lithium_ion/OKane2022.py	100.00% <ø> (ø)
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[valentinsulzer]

Use this https://stackoverflow.com/questions/37346481/how-do-i-find-and-replace-all-occurrences-in-all-files-in-visual-studio-code

[jeromtom]

Surely, will try that. Made the changes running for /f "delims=" %f in (C:\Users\jerto\Desktop\Replacer\filelist.txt) do powershell -Command "(Get-Content \"%f\") -replace '1 \+ dlnf/dlnc', 'Thermodynamic factor' | Set-Content \"%f\"" in cmd

[jeromtom]

That too after using findstr which @Saransh-cpp had introduced to me, to find references and store them into filelist.txt.

[jeromtom]

This was way easier.

[jeromtom]

Not changing the reference of "1 + dlnf/dlnc" in CHANGE.md which I hope is an exception to "everywhere".

Changes to be made:

• everywhere "1 + dlnf/dlnc" appears, replace it with "Thermodynamic factor"

[jeromtom]

@tinosulzer, codecov checks are failing. How do we fix that?

[valentinsulzer]

You have to click on the cross which takes you to codecov then see which line is not covered and fix bugs or add tests as appropriate so that the line is covered. You should test this locally e.g. by temporarily adding print("here") next to the line that should be covered, and making sure you can see "here" in the command line when you run the test. There is a bug in your current changes to parameter_values.py, but I won't tell you what since you should be able to figure it out yourself (and it's good to practice finding bugs)

[valentinsulzer]

Are you even testing this locally like I suggested or are you using trial and error with the GitHub tests. Please test locally to make sure your fix works.

[jeromtom]

I haven't been able to do the developer install to run tests locally. Will be doing it by the books.

[valentinsulzer]

The installation instructions are a bit confusing (hence the GSOC project to improve them) but you should be able to just go to the pybamm root repository where you downloaded it, create a virtual environment via pip or conda, and then do pip install -e .

Local installation is a required prerequisite for participating in GSOC

[jeromtom]

Thanks, @tinosulzer. I will do it soon.

[jeromtom]

Local installation is complete. Working on writing tests now.

[ayeankit]

Is this still open? I would like to work on it.

[jeromtom]

Hey @ayeankit, I am very close to finishing this PR.

[jeromtom]

@tinosulzer , can you please help me out here? I have done the local installation.

[jeromtom]

@tinosulzer there are two merge conflicts here.
Kindly review.

[valentinsulzer]

You have to merge the develop branch and fix the conflicts.

[valentinsulzer]

It will be much more efficient to test your changes locally to make sure coverage passes, instead of relying on the CI, as I explained how to do in this comment #2727 (comment)

[jeromtom]

I haven't understood how the print statements can be use for testing coverage. Can we start using pytest or Coverage.py or Pytest-cov like mentioned in this website in the future? Because then we can properly run coverage tests locally.

[valentinsulzer]

You can already use coverage.py locally if you want. That's what the CI uses. But that still requires running all the unit tests, which takes a few minutes. It's much faster to run the individual test files (python path/to/test.py) and check the command-line output. Once you've added print("here") in the right place, then "here" should appear in the command line output if that line is being covered by that particular test. Otherwise, it shows you that the line isn't being covered.

[jeromtom]

BTW I had done local tests using python run-tests.py --unit but it did not include coverage.

[jeromtom]

Is the following placement of the print statement correct?

[jeromtom]

@tinosulzer, I now totally agree with temporarily using print statements. It is way faster.

[valentinsulzer]

Thanks, will merge once final test passes

[jeromtom]

This is the first test code that I have ever written. @tinosulzer, thank you for your guidance.

[valentinsulzer]

Congrats on writing your first test!

[valentinsulzer]

@all-contributors add @jeromtom for code, tests

[allcontributors]

@tinosulzer

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