-
-
Notifications
You must be signed in to change notification settings - Fork 348
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
Add tests for ck2yaml #708
Conversation
Fix the reaction equation to always include the (+M). Previously, if the (+M) was omitted, an invalid reaction string was generated.
Codecov Report
@@ Coverage Diff @@
## master #708 +/- ##
==========================================
+ Coverage 70.63% 70.64% +0.01%
==========================================
Files 372 372
Lines 43567 43567
==========================================
+ Hits 30773 30778 +5
+ Misses 12794 12789 -5
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.
Other than the one comment below, this all seems fine. Any reason not to use this to refactor the CTML tests too? I'm also asking because I'm wondering if I should use it for the ctml2yaml
tests.
from cantera import ck2cti, cti2yaml | ||
from cantera import ck2cti, ck2yaml, cti2yaml | ||
|
||
class converterTestCommon: |
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.
I was going to suggest using pathlib
here, to move away from string manipulation for paths, but I'm not sure it's worth it. Since the converter functions don't work with path-types, it night as well just still be string stuff...
What do you mean by "the CTML tests"? The refactoring here is fairly easy since in both cases you're going from CK format to something that can be imported as a |
The test suite re-uses the tests for ck2cti.
Also fix a few small issues identified by running the tests.