search

kislyuk / yq

Command-line YAML, XML, TOML processor - jq wrapper for YAML/XML/TOML documents
https://kislyuk.github.io/yq/
Apache License 2.0
2.53k stars 81 forks source link

Use tomllib/tomli/tomli-w instead of unmaintained toml package #153

Closed mgorny closed 1 year ago

mgorny commented 1 year ago

Replace the use of the unmaintained toml package with the modern trinity: built-in tomllib module for reading TOML in Python 3.11+, tomli for reading TOML in older Python versions and tomli-w for writing TOML in all Python versions. This ensures correct TOML 1.0 support that the old toml package lacks.

kislyuk commented 1 year ago

Thanks for the PR. I think it's best to use tomlkit for Python < 3.11.

mgorny commented 1 year ago

Thanks for the PR. I think it's best to use tomlkit for Python < 3.11.

toml and tomli are the same package, so using it guarantees exactly the same behavior across Python versions. I'm not sure if tomlkit doesn't have some slight behavior differences.

kislyuk commented 1 year ago

I don't think yq will be using tomli/tomli-w because I think splitting the read and write functionality into separate packages was a mistake.

I agree we should import tomllib and use it on Python 3.11+. I also see a number of issues on the old toml package repo but it's unclear to me how severe they are.

This PR has merge conflicts and needs to be split into at least two: one for using tomllib on 3.11+ and one for replacing the toml dependency. I have to warn you, I'm not sure what to do about the latter so I'm not sure what I can accept there.

mgorny commented 1 year ago

I don't think yq will be using tomli/tomli-w because I think splitting the read and write functionality into separate packages was a mistake.

I agree we should import tomllib and use it on Python 3.11+. I also see a number of issues on the old toml package repo but it's unclear to me how severe they are.

These two statements conflict with each other. tomllib is read-only, so you need to use an external writer. If you require a reader+writer in a single package, then using tomllib makes no sense.

kislyuk commented 1 year ago

Thanks for clarifying. That's... not what I expected. I never imagined someone would commit a read-only parser to the standard library.

kislyuk commented 1 year ago

Thanks again for the PR. Due to the misguided decision to split up the parser and serializer, I don't expect that yq will be using the standard library tomllib/tomli-w functionality. Instead we will try to move forward with tomlkit.