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

Valid toml syntax causes black to crash #2280

Closed
nat-n opened this issue May 30, 2021 · 4 comments · Fixed by #2301
Closed

Valid toml syntax causes black to crash #2280

nat-n opened this issue May 30, 2021 · 4 comments · Fixed by #2301
Labels
C: configuration CLI and configuration C: dependencies C: packaging Installation and packaging of Black

Comments

@nat-n
Copy link

nat-n commented May 30, 2021

Describe the bug

I have a pyproject.toml for configuring various other tools (not black), which includes a list with mixed type contents, as explicitly support by the v1.0.0 of the toml spec.

When I run black it crashes with a cryptic traceback from inside the toml library.

To Reproduce

mkdir new_project
cat <<EOT > ./new_project/pyproject.toml
# this example is from the toml spec
contributors = [
  "Foo Bar <foo@example.com>",
  { name = "Baz Qux", email = "bazqux@example.com", url = "https://example.com/bazqux" }
]
EOT
cd new_project
black .

Expected behavior

It shouldn't crash.

Environment (please complete the following information):

  • Version: black, version 21.5b1
  • OS and Python version: MacOs Python 3.8.8

Since the presently used "toml" library explicitly only supports up to version 0.5.0 of the spec (from about two years ago), I'd suggest switching to tomlkit, which supports the current toml spec (officially up to 1.0.0rc1 but nothing has materially changed since then).

I'd be happy to put together a PR if this approach is deemed acceptable in principle.

@JelleZijlstra JelleZijlstra added the C: configuration CLI and configuration label May 30, 2021
@JelleZijlstra
Copy link
Collaborator

Seems reasonable. It looks like pip itself still uses toml though, and in practice compatibility with other Python tools that use pyproject.toml is probably more important for us than compatibility with the toml standard. There's some related discussion in https://discuss.python.org/t/adopting-recommending-a-toml-parser/4068.

@cooperlees
Copy link
Collaborator

tomli looks promising. Hope it can get into the stdlib so toml can become "real" in python as a low level config file format. Thanks for raising this interesting issue. For now tho I think you're stuck using 0.5.0 toml for pyproject.toml as I'd like us to follow what pip does too.

@ichard26 ichard26 added C: dependencies C: packaging Installation and packaging of Black labels May 30, 2021
@nat-n
Copy link
Author

nat-n commented May 30, 2021

I see, yes it seems pip has the same issue. I'd argue there's still value in supporting the latest toml syntax, even if pip does not yet. In principle it should be a non breaking change, though I guess the bottom line is that any divergence presents a risk of unexpected consequences?

@hukkin
Copy link
Contributor

hukkin commented Jun 1, 2021

I made an issue in pip issue tracker about this: pypa/pip#10034

Let's see if they have willingness to upgrade to a TOML v1.0.0 compatible parser.

Edit: This also led to a proposal to change PEP 518 so that it includes a TOML spec version for pyproject.toml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: configuration CLI and configuration C: dependencies C: packaging Installation and packaging of Black
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants