-
-
Notifications
You must be signed in to change notification settings - Fork 353
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 clib *newFromFile from MATLAB / fix memory leaks #1754
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1754 +/- ##
==========================================
- Coverage 73.12% 73.11% -0.01%
==========================================
Files 381 381
Lines 54134 54175 +41
Branches 9224 9226 +2
==========================================
+ Hits 39587 39612 +25
- Misses 11589 11602 +13
- Partials 2958 2961 +3 ☔ View full report in Codecov by Sentry. |
Ensure that interface is consistent with Python API.
e6c638b
to
fabf590
Compare
2f736e4
to
ed4e269
Compare
ed4e269
to
2f87d63
Compare
Added a fix for #1756: while the issue was pre-existing, this PR surfaced some symptoms, where incorrect PS: force-push was only to fix typos in commit messages / prevent CI failures. |
Phases adjacent to interfaces should not be added automatically, as they cannot be deleted.
e7028f8
to
4b4b0e1
Compare
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, @ischoegl. This looks mostly good to me. Just had a couple of small suggestions.
Add object parents to Cabinet, which are used to disambiguate objects that are created automatically, for example ThermoPhase / Kinetics / Transport that are instantiated together with new Solution objects.
4b4b0e1
to
66f8ef5
Compare
Thanks for the prompt review! I think it's all taken care of. |
Changes proposed in this pull request
clib
's*_newFromFile
constructors from MATLAB; use pathways usingSolution
exclusively insteadclib
where automatically added adjacent phases could not be removedInterface.adjacent
consistent with Python version (select phase by name rather than index)If applicable, fill in the issue number this pull request is fixing
Resolves #1756
Partially addresses Cantera/enhancements#199
Example
New in experimental MATLAB:
Checklist
scons build
&scons test
) and unit tests address code coverage