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

Read comment blocks from XML and CTI files when converting to YAML #23

Open
bryanwweber opened this issue Jan 11, 2020 · 7 comments
Open
Labels
feature-request New feature request

Comments

@bryanwweber
Copy link
Member

Abstract

XML and CTI input files may contain comments that describe the content of the file. It would be useful to try to include these comments in the YAML file after conversion.

Motivation

Many XML and CTI input files contain comments (using <!-- --> in XML and # in CTI) that describe the purpose of the file. Often these comments are at the top of the file. It would be nice to try to automatically copy these comments into the description field of the YAML file when the corresponding input file is converted.

For comments deeper in the file (e.g., above a reaction), it is not always clear with which entry the comment should be associated, so handling these becomes more complicated. Perhaps a warning could be issued to inform users that any custom comment messages should be copied manually to the YAML file?

Possible Solutions

It should be possible to get comments from the XML ElementTree in ctml2yaml.py. On the other hand, it isn't possible to get # comments from the CTI files using the ast module, so some more creative alternative might need to be explored.

References

Discussed here: Cantera/cantera#768 (comment)

@bryanwweber bryanwweber added the feature-request New feature request label Jan 11, 2020
@ischoegl
Copy link
Member

ischoegl commented Jun 2, 2021

@bryanwweber … Now that 2.5.1 is released, is this still being pursued?

@bryanwweber
Copy link
Member Author

Sure, I don't see why not.

@ischoegl
Copy link
Member

ischoegl commented Jun 2, 2021

Sure, I don't see why not.

Not volunteering myself; I was mostly asking as the feature will lose relevance the longer we wait.

@ischoegl
Copy link
Member

I believe now would be the time to tackle this (the alternative is to not further pursue the issue): the description field should be relatively easy to parse.

@bryanwweber
Copy link
Member Author

@ischoegl If you're volunteering, go ahead 😄 😝 I do think this will still be valuable for a while; XML and CTI files will be "in the wild" for a long time to come yet, so the converters are going to stick around for... well, probably forever!

@ischoegl
Copy link
Member

Still not volunteering, but let’s mark this as optional for 2.6 …

@ischoegl
Copy link
Member

@bryanwweber ... I ended up doing it anyways (at least for cti2yaml), see Cantera/cantera#1205. I don't think that it's necessary to implement this for ctml2yaml as I didn't find many instances where this is used. Imho, cti2yaml should already cover 95% of all use cases, and I don't want to over-engineer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature request
Projects
None yet
Development

No branches or pull requests

2 participants