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.

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

Restore modulo coverage #4143

Merged
merged 13 commits into from
Jun 7, 2024
Merged

Conversation

kratman
Copy link
Contributor

@kratman kratman commented Jun 5, 2024

Description

Improve coverage in the base solver that went down due to the removal of the ODEs solver.

Fixes #3941

Key checklist:

  • No style issues: $ pre-commit run (or $ nox -s pre-commit) (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 (or $ nox -s tests)
  • The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

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

@kratman kratman self-assigned this Jun 5, 2024
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.58%. Comparing base (b4f8380) to head (a3265ad).
Report is 218 commits behind head on develop.

Files with missing lines Patch % Lines
pybamm/solvers/base_solver.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4143      +/-   ##
===========================================
+ Coverage    99.55%   99.58%   +0.02%     
===========================================
  Files          288      288              
  Lines        21790    21789       -1     
===========================================
+ Hits         21693    21698       +5     
+ Misses          97       91       -6     

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

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@kratman kratman marked this pull request as ready for review June 6, 2024 22:54
@kratman
Copy link
Contributor Author

kratman commented Jun 6, 2024

This should fix all the coverage lost in #3932 by re-adding one of the tests from ODEs to the Casadi test suite. The code is not 100% covered, but it was not before either. This does replace what was lost and does some minor cleanup.

@kratman kratman changed the title Starting to fix modulo coverage Restore modulo coverage Jun 6, 2024
@kratman kratman merged commit b2f08ef into pybamm-team:develop Jun 7, 2024
25 of 26 checks passed
@agriyakhetarpal agriyakhetarpal deleted the fix/moduloCoverage branch June 7, 2024 15:55
js1tr3 pushed a commit to js1tr3/PyBaMM that referenced this pull request Aug 12, 2024
* Starting to fix modulo coverage

* Remove quotes from the if-statement

* Adjust logic

* Fix job naming

* Change order

* Cleanup

* Re-adding test

* A bit more refactoring

* Fix a line of coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update windows logic for JAX in Nox and remove redundant Homebrew installations
2 participants