Skip to content
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

Resolve Yaml paper cuts #1179

Merged
merged 1 commit into from
Feb 4, 2022
Merged

Resolve Yaml paper cuts #1179

merged 1 commit into from
Feb 4, 2022

Conversation

ischoegl
Copy link
Member

@ischoegl ischoegl commented Jan 19, 2022

Changes proposed in this pull request

This PR aims to address remaining points raised in #1053

  • Add Species.from_dict creation (in analogy to Reaction.from_dict)
  • Add ability to pass YAML syntax to anymap_from_dict
  • Address some YAML code formatting in Sphinx

If applicable, fill in the issue number this pull request is fixing

Closes #1053

If applicable, provide an example illustrating new features this pull request is introducing

E.g. mixing YAML syntax when defining a species:

data = {
    "name": "H2O",
    "composition": "{H: 2, O: 1}",
    "thermo": "{model: constant-cp, h0: 100}", # value is formatted as YAML code
}
species = ct.Species.from_dict(data)

Checklist

  • The pull request includes a clear description of this code change
  • Commit messages have short titles and reference relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • Style & formatting of contributed code follows contributing guidelines
  • The pull request is ready for review

@ischoegl
Copy link
Member Author

Pre-emptively dropping commits that will clash with #1183.

Copy link
Member

@speth speth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to drop the implicit YAML parsing that this introduces. Other than that, what's implemented here looks fine.

interfaces/cython/cantera/utils.pyx Outdated Show resolved Hide resolved
@ischoegl
Copy link
Member Author

ischoegl commented Feb 4, 2022

I would prefer to drop the implicit YAML parsing that this introduces.

@speth ... no problem: dropped. Also made some of the unit test method names more pythonic (i.e. switched to snake_case), but nothing else changed.

Copy link
Member

@speth speth left a 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. Thanks, @ischoegl!

@ischoegl ischoegl merged commit 03c6e70 into Cantera:main Feb 4, 2022
@ischoegl ischoegl deleted the yaml-paper-cuts branch February 20, 2022 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paper cuts in YAML serialization API
2 participants