Bugfix nasa as dict #1630
Bugfix nasa as dict #1630
Conversation
|
Looking good! Would you like to remove Tmin / Tmax from one of the tests? |
Codecov Report
@@ Coverage Diff @@
## master #1630 +/- ##
=========================================
+ Coverage 41.37% 41.8% +0.43%
=========================================
Files 177 177
Lines 29455 29455
Branches 6059 6059
=========================================
+ Hits 12186 12314 +128
+ Misses 16417 16271 -146
- Partials 852 870 +18
Continue to review full report at Codecov.
|
|
I am currently working on the yaml file standardization for the isodesmic reaction code, so this PR is very relevant to me. I have looked over the code, and it looks good to me. I like @alongd 's suggestion about adding a test for ensuring that an error is not thrown when Tmin/Tmax is not set, but I don't want this to hold up the PR from being merged. If you have the unit test for this already go ahead and add it. Otherwise, I believe this test will come up naturally in my isodesmic PR (or if not I'll add in a test). Either way @goldmanm feel free to update this branch and merge. |
Previously, NASA.as_dict would throw an error if attributes like Tmin, Tmax, E0 were unset. These variables, however, are optional, so Arkane should not crash if they are unset. This commit fixes the as_dict method to not throw an error for optional attributes
This commit adds two test methods for NASA.as_dict and two tests to load species from nasa polynomials in Arkane. squash! Add tests to load species from NASA polynomials
goldmanm
force-pushed
the
bugfix_nasa_as_dict
branch
from
ae69477 to
18a916f
Compare
|
Added two tests ensuring that |
|
Looking good! |
Motivation or Problem
NASA polynomials would throw an error when calling the
as_dictmethod when optional variables likeTminare not set.Description of Changes
Added an if statement to the
as_dictmethod to only add attributes which are set.Testing
RMG tests should be sufficient