-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
Remove 'simplification' and other associated tests #1362 #1369
Remove 'simplification' and other associated tests #1362 #1369
Conversation
Also removed import of symplify in __init__.py
Removed the use of simplify from all tests using it and removed test_simplify.py (since Simplification class no longer exists)
from scipy.sparse import csr_matrix was needed.
Also removed simplify from more tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @asinghgaba! Just a couple of small things to change. Let's also wait to see if the coverage is still ok, I think it should be
tests/integration/test_models/test_full_battery_models/test_lead_acid/test_composite.py
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## develop #1369 +/- ##
===========================================
+ Coverage 98.13% 98.21% +0.07%
===========================================
Files 277 276 -1
Lines 15959 15649 -310
===========================================
- Hits 15661 15369 -292
+ Misses 298 280 -18
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out there are a few more things to delete (from the coverage report):
- all the
_unary_simplify
functions - all the
_binary_simplify
functions - all the
_concatenation_simplify
functions
Then it should be done! There are some other coverage changes but each one requires different action so we can take care of later
Removed _concatenation_simplify()
Removed _unary_simplify()
Removed _binary_simplify()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all good, merging
@all-contributors add @asinghgaba for code |
@tinosulzer I've put up a pull request to add @asinghgaba! 🎉 |
Description
Removed
Simplification
class and relevant tests associated with it. Also, movedsimplify_if_constant()
tosymbol.py
.Fixes #1362
Completed all the tasks specified in #1362
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:
$ flake8
$ python run-tests.py --unit
$ cd docs
and then$ make clean; make html
You can run all three at once, using
$ python run-tests.py --quick
.Further checks: