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

mac/focal yaml serialization differs from bionic's #13541

Closed
ggould-tri opened this issue Jun 11, 2020 · 3 comments · Fixed by #13543
Closed

mac/focal yaml serialization differs from bionic's #13541

ggould-tri opened this issue Jun 11, 2020 · 3 comments · Fixed by #13543

Comments

@ggould-tri
Copy link
Contributor

ggould-tri commented Jun 11, 2020

For whatever reason, yaml serialization on mac and focal produces vertically formatted lists, while serialization on bionic produces horizontally formatted ones. This produces errors like

  example:
+   controller_params: [5.0, 50.0, 5.0, 1000.0]
+   initial_state: [1.2, 0.0, 0.0, 0.0]
-   controller_params:
-   - 5.0
-   - 50.0
-   - 5.0
-   - 1000.0
-   initial_state:
-   - 1.2
-   - 0.0
-   - 0.0
-   - 0.0
    t_final: 30.0
    tape_period: 0.05

The relevant test was nerfed in #13538 but should be rewritten to compare the parsed (or post-parse serialized) rather than a raw reference serialized form in its roundtrip tests (ie, to insist on semantic rather than syntactic equality).

@jwnimmer-tri
Copy link
Collaborator

Ideally, the I/O code could be set to use the flow style during serialization, because it's nicer to read. We do that for C++ already:

case YAML::NodeType::Sequence: {
// If all children are scalars, then format this sequence onto a
// single line; otherwise, format as a bulleted list.
auto style = YAML::EmitterStyle::Flow;
for (const auto& child : node) {
if (child.IsSequence() || child.IsMap()) {
style = YAML::EmitterStyle::Block;
}
}

@ggould-tri
Copy link
Contributor Author

Amen on the nicer-to-read.
I believe we may have ended up on opposite sides of yaml/pyyaml#256 and so must always set default_flow_style when we call the dumper.

@jwnimmer-tri
Copy link
Collaborator

Ooh, a tribool found in the wild. Get out the documentary cameras.

ggould-tri added a commit to ggould-tri/drake that referenced this issue Jun 11, 2020
* pyyaml changed its default flow semantics in yaml/pyyaml#256
* We must override the default with the magic tribool value `None`
* Fixes RobotLocomotion#13541
ggould-tri added a commit to ggould-tri/drake that referenced this issue Jun 12, 2020
* pyyaml changed its default flow semantics in yaml/pyyaml#256
* We must override the default with the magic tribool value `None`
* Fixes RobotLocomotion#13541
ggould-tri added a commit that referenced this issue Jun 15, 2020
* Force older and newer yaml.dump to give the same output

* pyyaml changed its default flow semantics in yaml/pyyaml#256
* We must override the default with the magic tribool value `None`
* Fixes #13541
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants