sergiocorreia / panflute

An Pythonic alternative to John MacFarlane's pandocfilters, with extra helper functions
http://scorreia.com/software/panflute/
BSD 3-Clause "New" or "Revised" License
493 stars 60 forks source link

Optionally ordered yaml #203

Closed ickc closed 2 years ago

ickc commented 2 years ago

Edit: note that panflute only calls pyyaml when using yaml_filter (yaml in code block) so this only changes the order of reading the YAML metadata inside that code block. Ordering is nice when round-trip is needed, i.e. a filter convert from yaml code block, and another filter convert it back.

by using yamlloader whenever available.

See doc in https://github.com/Phynix/yamlloader

an OrderedDict loader/dumper is implemented, allowing to keep items order when loading resp. dumping a file from/to an OrderedDict (Python 3.7+: Also regular dicts are supported and are the default items to be loaded to. As of Python 3.7 preservation of insertion order is a language feature of regular dicts.)

Note: while yamlloader is not very popular, I've been using it in all of my projects (including some that depends on panflute) for a few years and it never has causes any problems.

Also see #178. Had we move to ruamel.yaml, the ordering comes for free. But I think it is better to stick with pyyaml (with yaml spec 1.1) as pandoc is reverting to yaml spec 1.1 as well.

Edit: also the reason to not use oyaml despite they said it is a drop in replacement of pyyaml with ordered dict, oyaml is not released as frequently as the upstream (pyyaml). The yamlloader model is better, that it decouples from pyyaml and releases only the Loaders/Dumpers, so the upstream can be updated independent of yamlloader.

sergiocorreia commented 2 years ago

Would it be easier if we just make yamlloader a requirement? If we make it optional, I'm thinking it could lead to odd errors, such as if I have the package installed on my laptop but not on my desktop, and thus the same code returns different results even if python/pandoc/panflute have the same versions.

ickc commented 2 years ago

But note that this currently only changed the read order of YAML filter metadata block. This order would “normally” won’t affect people, except if I have another filter that writes this back to a code block for example.

I have this as a dependencies on my packages, but for something like panflute I’m a bit hesitant to do that, as it is a small project (evidently by the amount of stars in its GitHub repo.) Drawing in extra, hard dependencies here may be a discouraging factor for adoption.

But may be I’m over thinking it. Let me know if you still want it to be a requirement and I will change the PR.