pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
1.08k stars 533 forks source link

Extend Step String Tagging Functionality #4178

Open mleot opened 3 months ago

mleot commented 3 months ago

Description

Adds additional funcitonality around step string tags. Allows for filtering a solution object based on tags. Allows for inclusion of tags when exporting data through the Solution.get_data_dict() method.

Fixes #4177

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.

Key checklist:

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

Further checks:

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.54%. Comparing base (205ca81) to head (35e72a9). Report is 3 commits behind head on develop.

:exclamation: Current head 35e72a9 differs from pull request most recent head 7916a2b

Please upload reports for the commit 7916a2b to get more accurate results.

Files Patch % Lines
pybamm/solvers/solution.py 88.23% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #4178 +/- ## =========================================== - Coverage 99.55% 99.54% -0.01% =========================================== Files 288 288 Lines 21857 21876 +19 =========================================== + Hits 21760 21777 +17 - Misses 97 99 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mleot commented 3 months ago

@valentinsulzer I added an additional feature, being the .step and .cycle attributes to the solution objects, which are assigned during the solve step of a simulation, when running 'with experiment'.

mleot commented 3 months ago

Are assert statements generally frowned upon in testing? I noticed that it raises issues in the static code analysis.

agriyakhetarpal commented 3 months ago

Are assert statements generally frowned upon in testing? I noticed that it raises issues in the static code analysis.

They are usually frowned upon in regular code, but I think we need to make Codacy more lenient about this a bit, because these are all in tests/, where asserts are commonplace. Soon, the use of asserts will grow a lot in PyBaMM (see #4156). I am unsure who has access to the relevant settings and if we can configure it with a codacy.yml file of sorts.