Support TOML v1.0.0 syntax in pyproject.toml
#8857
Conversation
changelog/8789.feature.rst
Outdated
| @@ -0,0 +1 @@ | |||
| Support TOML v1.0.0 syntax in ``pyproject.toml``. | |||
There was a problem hiding this comment.
This should point out that the toml dependency got replaced by tomli - something e.g. distribution packagers could easily miss and definitely should know about!
There was a problem hiding this comment.
👍 Please have a look if the message now looks better to you
| @@ -47,7 +47,7 @@ install_requires = | |||
| packaging | |||
| pluggy>=0.12,<1.0.0a1 | |||
| py>=1.8.2 | |||
| toml | |||
| tomli>=1.0.0,<2.0.0 | |||
There was a problem hiding this comment.
I don't think the >=1.0.0 is needed here, given that there never was a < 1.0.0 release. Not sure about the <2.0.0 part. We don't typically add upper bounds for dependencies, and I guess the tiny bit of API we need will probably stay the same across future versions.
There was a problem hiding this comment.
There are <1.0.0 releases. I had this same discussion with Black maintainers recently 😄 . You'll find my thoughts in that thread.
If you tell me exactly what constraint you want I'll change it to that of course (or you can just push to this branch).
There was a problem hiding this comment.
Huh, I thought I had looked at PyPI, but apparently I messed up somehow. Ok, seems fine to me either way, let's see what others think.
There was a problem hiding this comment.
I'm also not inclined to not having the upper pinning in there, because I don't consider pytest a user facing application, but a framework, so our dependencies need to be compatible downstream with user's code.
But having said that, I understand the points @hukkin made in that thread, and given he is the main author, his opinion here carries some weight. Also, seems we might never even see a 2.0 version. 😁
In summary: -0 on the upper pinning.
There was a problem hiding this comment.
I'll put my -1 on the upper pinning, please remove it
There was a problem hiding this comment.
Oh sorry, had enough approvals that I thought it would be OK to merge. 👍
src/_pytest/config/findpaths.py
Outdated
|
|
||
| config = toml.load(str(filepath)) | ||
| config = tomli.loads(filepath.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
Not directly related to your PR, but more of a question to @nicoddemus: I wonder why we don't handle tomli.TOMLDecodeError here and turn it into an UsageError, like we do with _parse_ini_config above?
There was a problem hiding this comment.
Good idea, i think it should be done
Its up to @hukkin whether this is ok for this pr or if it will be a follow-up
There was a problem hiding this comment.
I made the change (as it's a very small change). Is this something we want a test for btw?
There was a problem hiding this comment.
Is this something we want a test for btw?
Well I thought it couldn't do any harm, so added a test for the exception type.
There was a problem hiding this comment.
Not directly related to your PR, but more of a question to @nicoddemus: I wonder why we don't handle tomli.TOMLDecodeError here and turn it into an UsageError, like we do with _parse_ini_config above?
Probably it was an oversight only, good catch!
| @@ -47,7 +47,7 @@ install_requires = | |||
| packaging | |||
| pluggy>=0.12,<1.0.0a1 | |||
| py>=1.8.2 | |||
| toml | |||
| tomli>=1.0.0,<2.0.0 | |||
There was a problem hiding this comment.
Huh, I thought I had looked at PyPI, but apparently I messed up somehow. Ok, seems fine to me either way, let's see what others think.
|
Should have squashed that, my bad. 😕 |
Closes #8789