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

Replace poyo with ruyaml. #1498

Closed

Conversation

dHannasch
Copy link
Contributor

@dHannasch dHannasch commented Feb 2, 2021

#1489 used pyyaml because that's what #1471 and now #1497 used, but it would probably be better to use ruyaml, which supports YAML 1.2. (If @ssbarnea agrees.)

(pyyaml has expressed interest in YAML 1.2 and hope springs eternal that eventually pyyaml and ruyaml will merge, but for now ruyaml seems like the way to go, and in any case the difference between pyyaml and ruyaml is less than half a dozen lines so we could switch back if pyyaml updates.)

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please think twice before going this way because we may need to help maintaining ruyaml.

I created ruyaml fork about an year ago because ruamel.yaml had some leadership issues.

Still, the future of the fork in not very solid as it requires more active contributors.

This is why I am inclined to say that pyyaml would be a better bet if we can live without parsing comments.

Step back and reconsider very well any decision as the nice to have comments feature may hit us harder later, when we may endup having to maintain the library ourselves.

On the positive side: I am core contributor on ruyaml and happy to add others if needed.

@insspb @audreyfeldroy @pydanny please read and advice regarding pyyaml vs ruyaml as replacement for poyo. I do not want to steer the decision, both have pros and cons.

@ssbarnea ssbarnea added discussion major Marks an important change, when major version update required labels Feb 2, 2021
@pydanny
Copy link
Member

pydanny commented Feb 2, 2021

The challenge with YAML parsers historically has been supporting them on Windows. While we personally may not use Windows, a significant portion of the scientific community does use Windows. And they rely on Cookiecutter or Cookiecutter powered tools.

That's why poyo, a pure python parser was created. To support those users who had trouble with C binaries. Before we used it we were being knocked over with the volume of YAML installation help requests from Windows users, including a lot of professional scientists.

If a YAML parser has an absolutely garauntee to work on all supported platforms, then switch it over. Otherwise it shouldn't be used.

…so the first thing I want to do is pull some syntax shenanigans to make it easy to switch back and forth between using ruyaml and using pyyaml.
@ssbarnea
Copy link
Member

ssbarnea commented Feb 4, 2021

The challenge with YAML parsers historically has been supporting them on Windows. While we personally may not use Windows, a significant portion of the scientific community does user Windows. And they rely on Cookiecutter or Cookiecutter powered tools.

That's why poyo, a pure python parser was created. To support those users who had trouble with C binaries. Before we used it we were being knocked over with the volume of YAML installation help requests from Windows users, including a lot of professional scientists.

If a YAML parser has an absolutely garauntee to work on all supported platforms, then switch it over. Otherwise it shouldn't be used.

Many things changed since we had the compilation issues. Pyyaml already has pure-python implementation and the clib on si optional. They also started to ship pre-compiled wheels for many platforms: https://pypi.org/project/PyYAML/5.4.1/#files -- in fact I will not be surprised to see aarch and ppc soon

@dHannasch
Copy link
Contributor Author

If a YAML parser has an absolutely garauntee to work on all supported platforms, then switch it over.

pyyaml, ruamel.yaml and ruyaml all have wheels now, and observationally pip install Just Works on Windows, for all three.

As far as pyyaml versus ruyaml, I've just done some syntax shenanigans so we can easily switch back and forth.

Preserving comments in the YAML would definitely be useful, but I think the main thing is supporting YAML 1.2. YAML 1.1 has some quirks, and as long as YAML 1.2 is supported, additional wrapper tools can do any comment-handling off to the side by reading and writing YAML 1.2.

PyYAML intends to support YAML 1.2, but doesn't yet.

I created ruyaml fork about an year ago because ruamel.yaml had some leadership issues.
Still, the future of the fork in not very solid as it requires more active contributors.

@ssbarnea Actually, since you bring it up, I wonder if I could probe your ideas of where ruyaml is going long-term. I had the impression that the main reason you forked ruamel.yaml into the new ruyaml was because, at the time, PyYAML was very much in keep-alive mode and was not interested in major changes like supporting YAML 1.2. So the only way to go was to keep ruamel.yaml also alive.

I'm very much not an expert, but it seems like YAML 1.1 versus YAML 1.2 is the main reason the two libraries have always been incompatible --- PyYAML only supports YAML 1.1 and ruamel/ruyaml only supports YAML 1.2. (Right? Or am I completely off-base there?) That's why, when PyYAML got adopted, they couldn't incorporate any of the additions that ruamel.yaml made during the period when PyYAML was abandoned.

(There's also the comment-preserving feature, and I assume that's not unrelated, since the cleaner syntax of YAML 1.2 is probably what enables handling comments. But it all comes back to YAML 1.2.)

Given that PyYAML is interested in supporting YAML 1.2, is there still reason to maintain ruyaml as a separate library independent of PyYAML, versus collapsing the features of ruyaml back into PyYAML?

@ssbarnea
Copy link
Member

ssbarnea commented Feb 4, 2021

The maintainer of pyyaml stated clearly that preserving comments is outside their agenda (and unlikely to ever happen). Their main focus is on making pyyaml reliable and fast, the fact that it ended up being used as a file configuration is more an accident than a planned goal.

I think you do underestimate the amount of complexity added by the comments feature. On the other hand ruyaml will likely never be as performant as pyyaml, so I doubt we will ever see one emerging as the winner. I used both, based on what is more important.

In fact I have one project where I use both, as I needed features from both, ansible-lint, but that is a special case.

I am a little bit inclined to recommend pyyaml instead of ruyaml mostly because of the number of active maintainers existing on both at this moment. Anyway, for cookiecutter any of them should be ok.

@ssbarnea
Copy link
Member

Few minutes ago we merged #1489 which does switch to pyyaml, which is a safer bet when it comes to long term maintenance than ruyaml.

@ssbarnea ssbarnea closed this Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion major Marks an important change, when major version update required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants