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

ryml accepts setting a VAL node to a MAP node without removing val or triggering error #31

Closed
biojppm opened this issue Jan 25, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@biojppm
Copy link
Owner

biojppm commented Jan 25, 2020

meas:
  createParameterList:
    - Lumi
      value: 1
      relErr: 0.1
    - mu
    - alpha_syst1
    - mu
      value: 1
      low: 0
      high: 3
      const: 0
ERROR parsing yml: parse error - indentation should not increase at this point
line 4: '      value: 1' (sz=14)
         ^~~~~~~~~~~~~~  (cols 1-15)

the document was written with rapidyaml. So irrespective of whether it's valid YAML or not, I'd hope it would at least be able to parse back what it emitted :-)

Originally posted by @cburgard in #28 (comment)

@biojppm
Copy link
Owner Author

biojppm commented Jan 26, 2020

@cburgard can you provide a minimal example which triggers this?

@cburgard
Copy link
Contributor

cburgard commented Jan 26, 2020

Hi!

Sure, here you go.

#include <ryml.hpp>
#include <c4/yml/std/map.hpp>
#include <c4/yml/std/string.hpp>

#include <iostream>

using namespace c4::yml;

int main(){
  ryml::Tree tree;
  NodeRef r = tree.rootref();
  r |= ryml::MAP;

  auto meas = r["meas"];
  meas |= ryml::MAP;

  auto plist = meas["createParameterList"];
  plist |= ryml::SEQ;

  auto lumi = plist.append_child();
  lumi << "Lumi";
  lumi |= ryml::MAP;      
  lumi["value"] << 1;
  lumi["relErr"] << 0.1;

  auto mu1 = plist.append_child();
  mu1 << "mu";
  mu1 |= ryml::MAP;      

  auto alpha = plist.append_child();
  alpha << "alpha_syst1";  
  alpha |= ryml::MAP;      
  
  auto mu = plist.append_child();
  mu << "mu";
  mu |= ryml::MAP;      
  mu["value"] << 1;
  mu["low"] << 0;
  mu["high"] << 0;
  mu["const"] << 0;

  emit(tree);

  return 1;
}

Now when moving it to an MWE, I can clearly see that the problem is related to setting the content of a node << "sometext" and afterwards declaring it to be a container with |= ryml::MAP. If the order is swapped, ryml complains (as it should).

@biojppm
Copy link
Owner Author

biojppm commented Jan 27, 2020

Thanks, this helps!

ryml should trigger an error, but it doesn't:

  auto mu = plist.append_child();
  mu << "mu";        // 1. this sets (or should set) mu as a VAL element
  mu |= ryml::MAP; // 2. then this will make it a MAP, all the while allowing it to remain a VAL.     
  mu["value"] << 1; // 3. this is OK, given the previous line
  //...

So this is a bug: the error should be triggered on 2.

@biojppm biojppm changed the title emitted YAML is malformed ryml accepts setting a VAL node to a MAP node without removing valelements or triggering error Jan 27, 2020
@biojppm biojppm changed the title ryml accepts setting a VAL node to a MAP node without removing valelements or triggering error ryml accepts setting a VAL node to a MAP node without removing val or triggering error Jan 27, 2020
@biojppm biojppm added the bug Something isn't working label Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants