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

[internal] Switch from PyYAML to Ruamel? #223

Open
formatc1702 opened this issue Mar 20, 2021 · 8 comments
Open

[internal] Switch from PyYAML to Ruamel? #223

formatc1702 opened this issue Mar 20, 2021 · 8 comments

Comments

@formatc1702
Copy link
Collaborator

This would add support for YAML 1.2 and solve #213.
Meanwhile, YAML 1.2 support seems to still be an open issue for PyYAML.

Any opinions on this matter?

@martinrieder
Copy link
Contributor

martinrieder commented May 17, 2024

Referring to the discussion in #305:

It seems to be how the current YAML parser is intended to work. It might change in the future, e.g. if deciding to use a parser supporting YAML version 1.2 - where the unquoted words Yes/No and On/Off are no longer interpreted as aliases for the boolean values true/false. See also the related issues #213 and #223.

To override how to interpret the type of these unquoted words, will have these effects:

  • It's a global change and thus applies to all YAML input.
  • The WireViz input language no longer conforms to the YAML 1.1 specification.

[...]

@EvilMustacheTwirl is making a good point. Interpreting everything as a string could help fix some issues. Numbers and Booleans should be evaluated only where the WireViz syntax requires this. Would a simple regex replacement hack possibly also do the job?

I realize that the above will not allow linking the YAML input to line numbers. The YAML parser replacement seems essential for linking the error output to the input file. Based on #207 (comment) request for machine-readable output, the replacement would be beneficial for nanangp/vscode-wireviz-preview#7

@martinrieder
Copy link
Contributor

martinrieder commented May 17, 2024

I did a bit of research and found that https://github.com/nexB/saneyaml might be lower hanging fruit than Ruamel.

EDIT: two further promising options are
https://github.com/crdoconnor/strictyaml
https://github.com/jcrist/msgspec

@martinrieder
Copy link
Contributor

For anyone who wants to dive a bit deeper into the topic, I recommend reading the following document:

https://eemeli.org/yaml/#parsing-yaml

This comes from the library that is used to provide syntax highlighting and error checking in https://github.com/redhat-developer/YAML-language-server for VScode. It is not programmed in Python, but we might take advantage of the CLI that it provides:
It relies on parsing YAML into a CST as an intermediate step to provide an AST. This is a unique approach that other libraries do not take, but seems to be just what we need here.

Side note: Graphviz dot is also supported in VScode through https://github.com/nikeee/dot-language-server

@kvid
Copy link
Collaborator

kvid commented May 21, 2024

To enable comparing the alternatives, we should list the goals we want to achieve by switching library. Unless all goals can be achieved, then I guess we also must rank the goals by importance. A few goal candidates:

@martinrieder
Copy link
Contributor

martinrieder commented May 30, 2024

Please extend the list of criteria with:

@martinrieder
Copy link
Contributor

martinrieder commented Jun 6, 2024

The parser might need to provide default values other than none if some attributes are missing. See the proposed use case here: #257 (comment)

There is need to have a dummy table: It would be less confusing if the whitespace was internally set as a default. If users wanted to override this, they would have to define none explicitly.

Doing it vice-versa might be the better option. It just needs to assure that an empty string is not equal to none.

Another use case is described in #273 (comment) about leaving some attributes undefined if they can be copied from connected wires.

@martinrieder
Copy link
Contributor

martinrieder commented Jun 14, 2024

I would like to add another library with some interesting features to the list that should be evaluated: fabiocaccamo/python-benedict

@martinrieder
Copy link
Contributor

martinrieder commented Jun 22, 2024

Note that PyYAML is making some progress towards YAML spec 1.2 support. A separate yamlcore package has been released on PyPi to enable compatibility, as announced here: yaml/pyyaml#555 (comment)

Currently it supports enabling all YAML 1.2 Core Schema tags on top of the PyYAML BaseLoader. It does not (yet) support other tags like the << merge key. You can add custom constructors, though.
For more information see the comparison of 1.1 and 1.2 schemas.

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

No branches or pull requests

3 participants