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

Switch from pytoml to toml #8045

Merged
merged 5 commits into from
Apr 14, 2020
Merged

Switch from pytoml to toml #8045

merged 5 commits into from
Apr 14, 2020

Conversation

pradyunsg
Copy link
Member

@pradyunsg pradyunsg commented Apr 14, 2020

Closes #6120
Closes #6136

Toward #8040

Fixes our vendoring of pep517, and improves the situation w/ TOML libraries.

@pradyunsg pradyunsg added !release blocker Hold a release until this is resolved project: vendored dependency Related to a vendored dependency type: maintenance Related to Development and Maintenance Processes labels Apr 14, 2020
@uranusjr
Copy link
Member

uranusjr commented Apr 14, 2020

Need to change the vendored() line in pip/_vendor/__init__.py as well.

We really should introduce a check (some kind of linter?) to ensure we don’t forget to update these…

@pradyunsg
Copy link
Member Author

@uranusjr hmm... Maybe?

I'd like downstream devs to pitch in on this though, since it mainly affects them. 🤷🏻‍♂️

@pradyunsg pradyunsg merged commit e40d267 into pypa:master Apr 14, 2020
@pradyunsg pradyunsg deleted the pytoml-to-toml branch April 14, 2020 11:34
@dholth
Copy link
Member

dholth commented Apr 14, 2020

Funny enough pytoml has had releases and bugfixes after toml and after putting up the "unmaintained" warning.

@sdispater
Copy link

A question I have related to this change: as far as I know the toml library is not 1.0.0rc1 compliant, and Poetry will start using heterogeneous arrays in its configuration, so trying to install any Poetry package will break. So what are the steps necessary to make this work?

This is particularly problematic for pip since it vendors its dependencies and, as such, so old pip versions won't be able to install such packages.

Poetry won't have this issue since it depends on tomlkit which is now (as of the 0.6.0 release) 1.0.0rc1 compliant.

@pradyunsg
Copy link
Member Author

pradyunsg commented Apr 15, 2020

Hmm... toml is not 1.0.0rc1 compliant, but neither was pytoml. FWIW, this change was made because I didn't want to vendor 2 separate libraries for TOML parsing in pip 20.1 -- as pep517 has switched to toml.

I don't want to wait too long for pip to get onto the TOML 1.0.0 train, but I also don't think we can reasonably do this prior to the next release cycle -- i.e. 3 months later.

Would it be possible for poetry to defer using heterogeneous arrays until at least the next pip release cycle? I feel that'll be less disruptive overall, since at least the latest version of pip would have TOML 1.0.0 support, so users won't be in a situation of "use poetry or pip, but not both". It's likely that the older versions of pip will be around for a fair amount of time, so they'll stay incompatible -- there's nothing we can do about that.

If the situation of maintenance of toml/pytoml doesn't improve, I plan on figuring something out prior to the next pip release -- which might end up being switching pep517 and pip to tomlkit.

@sdispater
Copy link

sdispater commented Apr 16, 2020

Would it be possible for poetry to defer using heterogeneous arrays until at least the next pip release cycle?

Unfortunately, that's not up to Poetry really (even though it could be possible to write a warning in the documentation to avoid it as much as possible). Poetry depends on tomlkit to read and write to the pyproject.toml file so, since tomlkit supports heterogeneous arrays, there is nothing preventing the users to use them.

Even then, the pyproject.toml file is not exclusive to Poetry so another tool might use them and it will trip up pip just the same.

@uranusjr
Copy link
Member

uranusjr commented Apr 16, 2020

I think most people would be satisfied if pip explicitly says it does not support TOML 1.0.0 in 20.1, since AFAICT no popular tools out there require heterogeneous arrays.

@sdispater
Copy link

@uranusjr Poetry will not require them but will allow them (via tomlkit) so that means Poetry projects might no longer be compatible with pip < 20.1. Now, I don't know whether Poetry is considered popular or not, but such projects might not even know they won't be installable via pip until it's too late (i.e. they are already published to PyPI).

@uranusjr
Copy link
Member

@sdispater It is perfectly fine for Poetry to allow them, and users are perfectly entitled to create packages that use them, even if pip 20.1 is unable to consume them. I feel it’s good practice to check whether an installer (e.g. pip) supports a new feature before putting it in use of your project, and if pip expresses explicitly TOML 1.0 is not supported in the documentation, a package maintainer would happily accept the reponsibility to abide if they care. Neither pip nor Poetry needs to be responsible for their choice. And if someone feels this is a problem, they are always welcomed to contribute to both toml and pip 🙂

@pradyunsg
Copy link
Member Author

Aaaah. Okay, that makes sense.

@sdispater Your phrasing in the initial comment made me think poetry will require heterogeneous array support for some functionality. As long as poetry doesn't require syntax, for itself, that only works when the user uses heterogeneous arrays (or really anything that's TOML 1.0.0-rc1 specific / that is not in TOML 0.5.0), I think we should be fine. Adding a warning to poetry's docs about this, would be nice as well.

FWIW, my plan is to ask and self-answer a stackoverflow question using the error message pip 20.1 users would get if they try installing a project that has heterogeneous arrays in its pyproject.toml. That way, if users get this error, hopefully it'll surface useful information to them.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation project: vendored dependency Related to a vendored dependency !release blocker Hold a release until this is resolved type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip uses now deprecated pytoml Change away from pytoml for pip's TOML needs
5 participants