[GSOC 2025] Third Party Model Dispatching
Description
PR to serialise and deserialise custom models
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)
Important checks:
Please confirm the following before marking the PR as ready for review:
- No style issues:
nox -s pre-commit - All tests pass:
nox -s tests - The documentation builds:
nox -s doctests - Code is commented for hard-to-understand areas
- Tests added that prove fix is effective or that feature works
Codecov Report
:x: Patch coverage is 99.22179% with 2 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 98.86%. Comparing base (e154fec) to head (9db6435).
:warning: Report is 161 commits behind head on develop.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/pybamm/expression_tree/operations/serialise.py | 99.21% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #5056 +/- ##
===========================================
- Coverage 98.86% 98.86% -0.01%
===========================================
Files 320 320
Lines 26568 26825 +257
===========================================
+ Hits 26266 26520 +254
- Misses 302 305 +3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Check out this pull request on ![]()
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
I’ve added an example notebook to demonstrate how the functionality works. I’d appreciate any suggestions for improvements or refinements you might have.
Thanks @medha-14! Could you add the example notebook in a separate PR (please feel free to open one that builds on top of this branch)? Please feel free to push back on this; apologies if I might have missed something that came up during the meeting.
@agriyakhetarpal this PR needs tests and we agreed that the notebook is a great starting point for that, so the current notebook content should just be copy-pasted (and modified) as a test. We can then look into adding examples for this PR in the existing notebooks or creating a standalone example notebook.
Oh, okay, that makes sense – thanks, @Saransh-cpp!
I have added tests for the PR, the serialisation and de-serialisation of function parameters still needs to be fixed but apart from that it is working. For now I have removed the example notebook from this PR and will make a new PR to add that.
@medha-14 I have added just a few comments for you to work on, mainly around readability, and elaborating more in the docstrings of user-facing static methods. I think a descriptive docstring would make it easier for us to publish to PyBaMM docs once it is done. Also, I think your code is not formatted, and I would highly encourage you to use
pre-commitand format your code before you push. You can refer more aboutpre-commitchecks in the contributor's guidelines. You should also try to increase the code coverage or at least flag things out that you think don't need coverage for the codecov bot to pass the CI.
Thanks for the review, Santhosh! I'm currently facing a small issue with my system, so I'm unable to run the tests at the moment. I’ll work on adding and running them as soon as that’s resolved. In the meantime, I’ve addressed all the points related to readability and docstrings. Let me know if there's anything else I should improve!
@Saransh-cpp thanks for the detailed review. I’ve addressed all the comments, let me know if there’s anything else that needs fixing. I’ve also added more tests to increase the coverage.
I have addressed all the reviews. Please review and let me know if any further changes are needed.
Hmm, I'm unsure if the macOS-3.10 test failure is related. Just re-triggered it...