Throw an error when concatenations have different number of children#4562
Conversation
|
Tests are going to fail on this due to an unrelated issue that I am working on concurrently, so opening this for now and once we merge the other one we can merge this one. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4562 +/- ##
===========================================
- Coverage 99.27% 99.26% -0.02%
===========================================
Files 302 302
Lines 22868 22889 +21
===========================================
+ Hits 22703 22721 +18
- Misses 165 168 +3 ☔ View full report in Codecov by Sentry. |
kratman
left a comment
There was a problem hiding this comment.
Seems fine as a temporary fix for the bug
|
@aabills Can you update the bug fix section of the change log |
kratman
left a comment
There was a problem hiding this comment.
Looks good to me now. I can't vouch for the physics, but I don't see any issues with the code.
Done, I realized I haven't done that for the other PR's either so I did all 3. |
Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
…ybamm-team#4562) * add error and test * disable thermal half cells for now * easier way to fix it * clean up * move half cell code to function * style: pre-commit fixes * changelog * Update CHANGELOG.md Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com> --------- Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com> Co-authored-by: Alexander Bills <alec@ionworks.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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 #4561
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:
$ 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)$ python run-tests.py --all(or$ nox -s tests)$ 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: