Skip to content

docs: add a warning for stringify vs. pyyaml#621

Draft
jwhitaker-gridcog wants to merge 1 commit intoeemeli:mainfrom
jwhitaker-gridcog:jw/doco
Draft

docs: add a warning for stringify vs. pyyaml#621
jwhitaker-gridcog wants to merge 1 commit intoeemeli:mainfrom
jwhitaker-gridcog:jw/doco

Conversation

@jwhitaker-gridcog
Copy link

Heya, here's a proposal for a doco change c.f. #619 . I had read "full support for 1.1 and 1.2" in the project intro so had incorrectly assumed that the default settings would also meet those requirements, which isn't the case. imo it could be worth adding a warning at top-level stringify doco to try and make this clear?
Cheers


`value` can be of any type. The returned string will always include `\n` as the last character, as is expected of YAML documents. If defined, the `replacer` array or function follows the [JSON implementation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#The_replacer_parameter). See [Options](#options) for more information on the last argument, an optional configuration object. For JSON compatibility, using a number or a string as the `options` value will set the `indent` option accordingly.

Note that the default output of `stringify` is not safe to pass to YAML 1.1 consumers, e.g. PyYAML. You should set [`compat: true`](#schema-options) in `options` if you are targeting these consumers.

Choose a reason for hiding this comment

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

I wouldn't specifically mention a certain library here.
It's true that PyYAML itself does not support 1.2 at this point in time, but there is a library for that which you can use on top of PyYAML, and at some point it might be merged upstream.
(just IMHO as a regular pyyaml contributor)

Copy link
Author

@jwhitaker-gridcog jwhitaker-gridcog May 20, 2025

Choose a reason for hiding this comment

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

fwiw I called out pyyaml here because (anecdotally) it's a very common consumer, and (anecdotally) its users often aren't even aware that versioned YAML specs like 1.1 and 1.2 exist, much less the differences in parsing behaviour between them and which one pyyaml uses. As such I suspected the more correct "only works for yaml 1.2 readers" wouldn't be sufficient to warn many people who would be tripped up by this.

Copy link
Owner

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

You're right that defaulting to YAML 1.2 should be highlighted a bit better in the docs, but I don't think this particular language is quite right.

Could you re-file this as an issue instead of a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants