vubiostat/r-yaml

yaml 1.2 support

lorenzwalthert opened this issue · 33 comments

It would be great if that package could support yaml 1.2, which apparently the definite version and is already 10 years old now (https://yaml.org/spec/). Changes are nicely summarized here: https://perlpunk.github.io/slides.tpcig2018/yamlpp/slide022.html. Unexpected behavior as in #5 (see the many cross-references) would wanish.

This would imply to turn away from libyaml because it does not seem they are going to implement that spec soon, an issue pending for more than two years: yaml/libyaml#20. A popular alternative to libyaml in C++ would be https://github.com/jbeder/yaml-cpp, but there are many, also in python.

You're right in that it appears that the libyaml team seems unmotivated to add 1.2 support. It may be worth switching over to a different library. I'd want to think carefully about the benefits of doing so, however.

Up

It would simplify any automated workflow if YAML header (%YAML 1.2) could be read (or even just ignored) by read_yaml()
Like a for read.csv(file, skip=2)

Upgrading to YAML 1.2 is a pretty big undertaking. It basically means rewriting most of the package, since it will require changing the underlying YAML library. If I were to do that, I'd probably create a new package called 'yaml12' or something like that. I'm slowly edging my way to doing that though.

The libyaml folks are tracking tickets on github now: yaml/libyaml#20

I've taken over maintenance for this package. I'm considering a fork since it looks like libyaml supports 1.2. The package name proposal is yaml12. Thoughts?

@spgarbet if you do make a new package, I'd love to have the opportunity to review before you release to CRAN so we could ensure that we close a few security risks (e.g.) ensuring that eval.expr = FALSE by default.

@hadley Jeremy has moved on to private industry and I'm picking this up as maintainer. I'll look into the eval.expr=FALSE issue.

@hadley I pushed up a new version with eval.expr=FALSE by default. I'd like to fix the UTF issue before pushing to CRAN.

@spgarbet you're changing this package? I'm not sure that's a good idea because it has the potential to break existing code. I was only suggesting changing the default if you do yaml12.

The plan was always to at some point change the default to FALSE. The current CRAN version spits out a warning that this will happen in a future version. If it's too disruptive I can defer this patch.

Ah in that case you're probably ok.

It's that difficult intersection of security vs. disruption. I'd prefer to avoid disruption at the highest levels, conversely security needs must be addressed.

There is another library with almost the same interface call, libfyaml. It fully supports 1.2 and is prepping for full 1.3 support. Work has begun to see what it would take to implement that as the core dependency. It's not looking too bad. The existing library is actually close to being 1.2 compliant.

Oddly this request is quite close. There is only a couple issue left in the libyaml library before it has full 1.2 support.

I've split the project over to https://github.com/vubiostat/yaml12

This project seems as close as it's ever going to get to 1.2 using the current libyaml.