-
-
Notifications
You must be signed in to change notification settings - Fork 346
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 --extra option to ck2yaml #794
Add --extra option to ck2yaml #794
Conversation
2f4f49f
to
c8385e3
Compare
@speth and @bryanwweber ... this PR comes close to the suggested Regarding the |
@ischoegl I triggered builds of the conda-recipes CI services. It should upload a new package Unless the builds fail...https://travis-ci.org/Cantera/conda-recipes/jobs/637035390 |
Hmmm... It looks like it uploaded packages (at least for Linux/Python) anyways, despite the failures. You might find that it works anyways, @ischoegl |
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.
Looks good to me. We'll also want to add some information about this to the page on the website for ck2yaml
, once Cantera/cantera-website#91 has been merged.
@bryanwweber ... re conda packages. FYI, I just attempted to create a new conda environment, i.e.
but still get Regarding the proposed Jupyter example, I can document the |
@bryanwweber ... turns out that the conda packages are indeed updated, but the version in |
c8385e3
to
21a037c
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.
Overall looks good to me, thanks @ischoegl for taking this on. I have a few suggestions and a few points of clarification below.
The --extra option takes a YAML file as input and adds the following abilities: - specify alternative description string - copy (user-defined) custom fields to YAML output
21a037c
to
1832b34
Compare
f8841f8
to
6cc6b7b
Compare
6cc6b7b
to
5b40f90
Compare
@bryanwweber ... I think I took care of everything. It’s now possible to recreate your hand-edits with the |
@ischoegl I made some minor formatting changes, can you just double check that everything is still what you expect? Also, you may want to configure your text editor/IDE to automatically remove trailing spaces on lines. |
Codecov Report
@@ Coverage Diff @@
## master #794 +/- ##
==========================================
+ Coverage 71.54% 71.57% +0.02%
==========================================
Files 372 372
Lines 44348 44376 +28
==========================================
+ Hits 31730 31760 +30
+ Misses 12618 12616 -2
Continue to review full report at Codecov.
|
@bryanwweber ... looks all good to me. Thanks! |
Checklist
scons build
&scons test
) and unit tests address code coverageIf applicable, fill in the issue number this pull request is fixing
Addresses Cantera/enhancements#15, #339
Alternative to #777
Changes proposed in this pull request
The
--extra
option takes a YAML file as input and adds the following abilities:Example
with
extra.yaml
being(the
description
is different from the original header due to the added last line)While not addressed directly, references (i.e. #339) can be easily added by specifying an alternate
description
field in the--extra
file.