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

Consider switching to TOML v1 compliant parser #308

Closed
layday opened this issue Jun 11, 2021 · 21 comments · Fixed by #336
Closed

Consider switching to TOML v1 compliant parser #308

layday opened this issue Jun 11, 2021 · 21 comments · Fixed by #336
Labels
question Further information is requested

Comments

@layday
Copy link
Member

layday commented Jun 11, 2021

With several other projects (Poetry, by virtue of using tomlkit, ward) having switched or soon to switch (pip, Black) to a parser able to handle v1 of the spec, we might want to consider doing the same (or assisting with updating toml).

@FFY00
Copy link
Member

FFY00 commented Jun 17, 2021

This makes bootstrapping harder, as toml is already used by our dependencies and choosing a different parser would introduce yet another dependency to the tree. The PEP 518 spec does not specify TOML 1.0, so I believe it is fine to keep using this parser, though I acknowledge it would be better to use a TOML 1.0 compliant one.
We could provide a toml-1.0 extra until the toml library gains support for TOML 1.0 or until such parser is available in the standard library. This could be accompanied with a nice error message hinting that TOML parsing errors could be because pyptoject.toml is using a newer TOML version. How does that sound?

@FFY00 FFY00 added the question Further information is requested label Jun 17, 2021
@layday
Copy link
Member Author

layday commented Jun 17, 2021

This makes bootstrapping harder, as toml is already used by our dependencies and choosing a different parser would introduce yet another dependency to the tree.

Our only dependency which depends on toml is pep517. I think we could coordinate with @takluyver if this is a change we'd like to pursue.

The PEP 518 spec does not specify TOML 1.0, so I believe it is fine to keep using this parser, though I acknowledge it would be better to use a TOML 1.0 compliant one.

There's at least the inclination to update PEP 518 to require v1 of the spec.

We could provide a toml-1.0 extra until the toml library gains support for TOML 1.0 or until such parser is available in the standard library. This could be accompanied with a nice error message hinting that TOML parsing errors could be because pyptoject.toml is using a newer TOML version. How does that sound?

I think we can probably wait this one out. Once pip adops tomli, other projects will follow suit. Until then there's no rush I would think.

@takluyver
Copy link
Member

I'm assuming it will change at some point, but I don't personally feel much pressure to change it soon. I appreciate that projects using Poetry might be keener.

I would really like to see a basic TOML parser in the standard library (tomli looked to me like a good candidate for this), to avoid bootstrapping issues. Currently flit_core defines its own metadata in Python code so it can be bootstrapped without a TOML parser.

@gaborbernat
Copy link
Contributor

gaborbernat commented Jun 18, 2021

I would really like to see a basic TOML parser in the standard library

Per the latest discussion, this feels to me unlikely to happen - https://discuss.python.org/t/adopting-recommending-a-toml-parser/4068/64. Also, even if it does happen, that's at least three years away from being broadly available. I don't think you want to wait that long to solve this issue.

@takluyver
Copy link
Member

I appreciate that adding things to the standard library is a very long-term solution, and I'm OK with that. And there are probably options that we wouldn't accept as permanent solutions but might be OK with for the interim until we can rely on a parser in the stdlib.

@gaborbernat
Copy link
Contributor

What if that day never comes? Per my link above to me seems very likely there's not enough agreement to add a parser into the stdlib.

@takluyver
Copy link
Member

What can I say? Obviously I'll live with it if core dev never want to add anything to the standard library again. I'm just expressing that I hope they do, and a TOML parser seems like a really good thing to add. I also haven't got the impression that it's as foregone a conclusion as you suggest that it won't be accepted.

@gaborbernat
Copy link
Contributor

I wasn't saying it won't be added, but it's 50%-50% for now. As such we should plan as much for the possibility they add as they don't.

@layday
Copy link
Member Author

layday commented Jun 18, 2021

Not sure what the relevance of all of this is - does a TOML parser not being part of the stdlib prevent us from using another, v1-compliant parser? I'm personally not too enthusiastic about leaving a library which has served us well for years behind, but if the community is intent on moving in that direction, the alternative is to say our toolset is incompatible with pip and others.

@takluyver
Copy link
Member

Sorry, yup, got sidetracked a bit. 😐

If pip switches to tomli, I'll likely follow that for my projects - I like the focus on a small API for parsing only.

The wrinkle for pep517 is that it still supports old Python versions, including 2.7, as a foundational piece of infrastructure. But all the pieces it actually uses a toml library for are deprecated in favour of the caller loading the toml and passing values in. So maybe it doesn't matter.

@layday
Copy link
Member Author

layday commented Jun 18, 2021

Sorry, yup, got sidetracked a bit. 😐

No worries, was thinking I might've missed something.

@layday
Copy link
Member Author

layday commented Jul 16, 2021

pip, black, setuptools_scm, pytest, towncrier and coverage have all switched to tomli now.

@FFY00
Copy link
Member

FFY00 commented Jul 16, 2021

The packages I care about for the consideration in this project are setuptools and pep517.

@layday
Copy link
Member Author

layday commented Jul 16, 2021

build is often used in conjunction with the projects I listed above. If build can't read a pyproject.toml which all of these other tools can read that is obviously relevant to us.

@FFY00
Copy link
Member

FFY00 commented Jul 16, 2021

We can softdepend on tomli as a preference over toml, but toml should be the fallback as it is the one used by the projects I listed and choosing something else would make bootstrapping harder.

@layday
Copy link
Member Author

layday commented Jul 16, 2021

Well, @takluyver has said that he'd follow the lead of pip (#308 (comment)). setuptools does not use TOML.

@henryiii
Copy link
Contributor

setuptools does not use TOML.

What about when adding PEP 621 support?

@gaborbernat
Copy link
Contributor

What about when adding PEP 621 support?

What has this project to do with PEP-621? Isn't that a backend thing?

@FFY00
Copy link
Member

FFY00 commented Jul 16, 2021

I think Henry meant that setuptools will adopt a parser soon for PEP 621 support.

@layday
Copy link
Member Author

layday commented Jul 17, 2021

I assume when that time comes - should it ever come - setuptools will not choose a TOML parser which is abandoned. setuptools is not a runtime dependency of build regardless. I suggest switching to tomli after both pypa/pyproject-hooks#118 and #324 have been merged - no soft dependency on tomli required.

@takluyver
Copy link
Member

I'm kind of amazed how quickly we've gone from 'there's this project called tomli' to 'that's the standard Python toml parser now'. 🙂 Hopefully now that there's a TOML v1 spec and a parser with a deliberately minimal API, the situation will stabilise a bit.

Anyway, as I said before, I do mean to follow this change in projects I maintain, including pep517.

FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 1, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 2, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 2, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 2, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 2, 2021
Fixes pypa#308

Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00 added a commit to FFY00/python-build that referenced this issue Aug 2, 2021
Fixes pypa#308

Co-authored-by: layday <layday@protonmail.com>
Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00 FFY00 closed this as completed in #336 Aug 2, 2021
FFY00 added a commit that referenced this issue Aug 2, 2021
Fixes #308

Co-authored-by: layday <layday@protonmail.com>
Signed-off-by: Filipe Laíns <lains@riseup.net>
inmantaci pushed a commit to inmanta/inmanta-core that referenced this issue Aug 6, 2021
Bumps [build](https://github.com/pypa/build) from 0.5.1 to 0.6.0.post1.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/pypa/build/blob/main/CHANGELOG.rst">build's changelog</a>.</em></p>
<blockquote>
<h1>0.6.0.post1 (05-08-2021)</h1>
<ul>
<li>Fix compability with Python 3.6 and 3.7 (<code>PR [#339](https://github.com/pypa/build/issues/339)</code><em>, Fixes <code>[#338](https://github.com/pypa/build/issues/338)</code></em>)</li>
</ul>
<p>.. _PR <a href="https://github-redirect.dependabot.com/pypa/build/issues/339">#339</a>: <a href="https://github-redirect.dependabot.com/pypa/build/pull/339">pypa/build#339</a>
.. _<a href="https://github-redirect.dependabot.com/pypa/build/issues/338">#338</a>: <a href="https://github-redirect.dependabot.com/pypa/build/issues/338">pypa/build#338</a></p>
<h1>0.6.0 (02-08-2021)</h1>
<ul>
<li>Improved output (<code>PR [#333](https://github.com/pypa/build/issues/333)</code><em>, Fixes <code>[#142](https://github.com/pypa/build/issues/142)</code></em>)</li>
<li>The CLI now honnors <code>NO_COLOR</code>_ (<code>PR [#333](https://github.com/pypa/build/issues/333)</code>_)</li>
<li>The CLI can now be forced to colorize the output by setting the <code>FORCE_COLOR</code> environment variable (<code>PR [#335](https://github.com/pypa/build/issues/335)</code>_)</li>
<li>Added logging to <code>build</code> and <code>build.env</code> (<code>PR [#333](https://github.com/pypa/build/issues/333)</code>_)</li>
<li>Switch to a TOML v1 compliant parser (<code>PR [#336](https://github.com/pypa/build/issues/336)</code><em>, Fixes <code>[#308](https://github.com/pypa/build/issues/308)</code></em>)</li>
</ul>
<h2>Breaking Changes</h2>
<ul>
<li>Dropped support for Python 2 and 3.5.</li>
</ul>
<p>.. _PR <a href="https://github-redirect.dependabot.com/pypa/build/issues/333">#333</a>: <a href="https://github-redirect.dependabot.com/pypa/build/pull/333">pypa/build#333</a>
.. _PR <a href="https://github-redirect.dependabot.com/pypa/build/issues/335">#335</a>: <a href="https://github-redirect.dependabot.com/pypa/build/pull/335">pypa/build#335</a>
.. _PR <a href="https://github-redirect.dependabot.com/pypa/build/issues/336">#336</a>: <a href="https://github-redirect.dependabot.com/pypa/build/pull/336">pypa/build#336</a>
.. _<a href="https://github-redirect.dependabot.com/pypa/build/issues/142">#142</a>: <a href="https://github-redirect.dependabot.com/pypa/build/issues/142">pypa/build#142</a>
.. _<a href="https://github-redirect.dependabot.com/pypa/build/issues/308">#308</a>: <a href="https://github-redirect.dependabot.com/pypa/build/issues/308">pypa/build#308</a>
.. _NO_COLOR: <a href="https://no-color.org">https://no-color.org</a></p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/pypa/build/commit/018a6f81cca1b26d9bf754eb91a64667c3847d28"><code>018a6f8</code></a> release 0.6.0.post1</li>
<li><a href="https://github.com/pypa/build/commit/84412dbbb29f08fdc0040ce75c81107843f7c496"><code>84412db</code></a> changelog: add Python 3.6 and 3.7 compability fix</li>
<li><a href="https://github.com/pypa/build/commit/c3f9e6d8c11c6ca3f71159a0347de171395cddd9"><code>c3f9e6d</code></a> build: restore compatibility with Python 3.6 and 3.7</li>
<li><a href="https://github.com/pypa/build/commit/720d69dadc1641769be89e3630b12b207e157ca3"><code>720d69d</code></a> tests: enable pytest strict mode</li>
<li><a href="https://github.com/pypa/build/commit/4cb0c1ec375aeff3e681855fdb37cf8f47e0fa93"><code>4cb0c1e</code></a> setup.cfg: fix typo in version</li>
<li><a href="https://github.com/pypa/build/commit/48c68979d95f070c093125b663dbd074399b5619"><code>48c6897</code></a> release 0.6.0</li>
<li><a href="https://github.com/pypa/build/commit/612dde4a4d538d1f0547ee524667b11534cba2cd"><code>612dde4</code></a> build: switch to tomli for TOML v1 compliant parser</li>
<li><a href="https://github.com/pypa/build/commit/7ed0a6bf06e05e6ccc72a99fe8194ebb9a2f1d20"><code>7ed0a6b</code></a> main: add line between backend output and missing dependencies error</li>
<li><a href="https://github.com/pypa/build/commit/79e777452cf3a75392b858f5ec41fc41eb4403b7"><code>79e7774</code></a> main: refactor color detection</li>
<li><a href="https://github.com/pypa/build/commit/03ad5d4af2cb23c13ab2de4672c359afbb99ab6b"><code>03ad5d4</code></a> main: add FORCE_COLOR</li>
<li>Additional commits viewable in <a href="https://github.com/pypa/build/compare/0.5.1...0.6.0.post1">compare view</a></li>
</ul>
</details>
<br />

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=build&package-manager=pip&previous-version=0.5.1&new-version=0.6.0.post1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants