-
-
Notifications
You must be signed in to change notification settings - Fork 349
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
Yaml2ck script #1286
Yaml2ck script #1286
Conversation
f6b2b0b
to
3140b20
Compare
0f29b61
to
06a18dd
Compare
06a18dd
to
8f51c27
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.
@bryanwweber ... thanks for picking this up! This overall looks really good, and I don't have any significant comments (other than formatting suggestions for yaml2ck --help
).
Running this on my (windows) machine, I am, however, running into the following issue:
(cantera-dev) PS C:\Users\ischo\GitHub\cantera> python --version
Python 3.10.2
(cantera-dev) PS C:\Users\ischo\GitHub\cantera> yaml2ck .\h2o2.yaml
Traceback (most recent call last):
File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\runpy.py", line 196, in _run_module_as_main
return _run_code(code, main_globals, None,
File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\runpy.py", line 86, in _run_code
exec(code, run_globals)
File "C:\Users\ischo\miniconda3\envs\cantera-dev\Scripts\yaml2ck.exe\__main__.py", line 7, in <module>
File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\site-packages\cantera\yaml2ck.py", line 685, in main
output_paths = convert(
File "C:\Users\ischo\miniconda3\envs\cantera-dev\lib\site-packages\cantera\yaml2ck.py", line 511, in convert
solution = ct.Solution(solution, phase_name)
File "build\python\cantera\base.pyx", line 71, in cantera._cantera._SolutionBase.__cinit__
self._cinit(infile=infile, name=name, adjacent=adjacent, origin=origin,
File "build\python\cantera\base.pyx", line 126, in cantera._cantera._SolutionBase._cinit
self._init_yaml(infile, name, adjacent, yaml, transport_model)
File "build\python\cantera\base.pyx", line 210, in cantera._cantera._SolutionBase._init_yaml
stringify(infile), stringify(name), cxx_transport, adjacent_solns)
File "build\python\cantera\utils.pyx", line 27, in cantera._cantera.stringify
tmp = bytes(x.encode())
AttributeError: 'NoneType' object has no attribute 'encode'
despite scons test-python-convert
passing.
@ischoegl I think I fixed the problem you posted about |
@bryanwweber ... better, but not quite:
|
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.
@bryanwweber .... Thanks! Again, pretty minor things.
Before I forget ... |
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.
Everything else looks good
6d2e55d
to
539d3b4
Compare
@bryanwweber … will approve this once the merge conflict is resolved |
6b9a395
to
5e18aaf
Compare
Removed redundant yaml reading Uses Cantera's new notes Refactored to classes Co-Authored-By: Kyle Niemeyer <[email protected]> Co-Authored-By: Phillip Mestas <[email protected]> Co-Authored-By: Parker Clayton <[email protected]> Update Chebyshev Fix Bryan Changes - Part Thermo Update yaml2ck.py
The error is just over 1e-7.
Add more tests of the script.
Document all the parameters for functions. Remove unused options from the argument parser and change the default sorting to be in the source order, rather than sort by default.
Since ck2yaml.convert_mech takes a file name, not an open file, we use TemporaryDirectory and then create a file name relative to that folder. This change also consistently formats the output from TROE and SRI reaction types.
This method transparently passes arguments through to yaml2ck.convert.
The source keyword argument was removed when CTI/XML input was removed.
Python 3.7 requires the typing_extensions module. Scope it to 3.7 only for now, so it's more obvious when it can be removed.
Co-authored-by: Ingmar Schoegl <[email protected]>
Add option parsing for Boolean options to add "--no-" prefix. Add version added directive and update docstrings.
5e18aaf
to
e07fbfe
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.
LGTM 🎉
Tagging @tsikes thanks for all your work here! |
@bryanwweber Thank you so much for carrying the torch the rest of the way! I'm happy to see this completed. |
Changes proposed in this pull request
yaml2ck
script to convert YAML input files/Solution instances to Chemkin file(s)TODO: Add a method toSolution
to import and callyaml2ck.convert
TODO: Fix up the docstringIf applicable, fill in the issue number this pull request is fixing
Supersedes #1185
I tried to push an update to that branch and it closed the PR 🤷♂️
This also relies on the work in #1266 for some of the sorting, so it should be merged after that PR.
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build
&scons test
) and unit tests address code coverage