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

Add error message specifying that we can't parse pyproject.toml #734

Merged
merged 7 commits into from
May 31, 2024

Conversation

mscheifer
Copy link
Contributor

@mscheifer mscheifer commented May 22, 2024

Resolves: python-poetry/poetry#9394

  • Added tests for changed code.
  • Updated documentation for changed code.

Problem

Poetry gives confusing error messages when there is a parse error in the pyproject.toml file.

For example:

[tool.poetry]
name = "test-project"
version = "0.0.1"
description = ""
authors = []

[tool.poetry.dependencies]
python = "~3.11"
numpy = "^1.26.3"
numpy = "^1.26.3" # <-- Duplicate specification

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"
$ poetry run ruff check

Cannot overwrite a value (at line 10, column 18)

The error is confusing because it's unclear that this error relates to parsing the pyproject.toml file. For poetry run specifically it's also unclear if the error is coming from poetry or the command you are running.

Change

With this change instead we will see.

$ poetry run ruff check

/home/matthew/code/poetry/pyproject.toml is not a valid TOML file.
TOMLDecodeError: Cannot overwrite a value (at line 10, column 18)
This is often caused by a duplicate entry.

Open questions

Should we try to include the toml parse error at 0 verbosity somehow? Or alternatively, we could explicitly say "re-run with -v" in the message. Yes let's include the exception message.

Please let me know if anything should be changed to fit the project code standards.

src/poetry/core/pyproject/toml.py Outdated Show resolved Hide resolved
@luckydonald
Copy link

luckydonald commented May 28, 2024

I think the (at line 10, column 18) part is equally helpful.

I'd even go as far as to suggest the most likely cause, and how to fix it, maybe similar to:

poetry-core cannot parse toml file at '/home/luckydonald/code/poetry/pyproject.toml'.
  TOMLDecodeError: Cannot overwrite a value (at line 10, column 18)
This is often caused by a duplicate entry.

Couldn't find a good way to check for that duplication from the exception alone.
I'd just do

if e.msg = 'Cannot overwrite a value':
  txt += "\nThis is often caused by a duplicate entry."

This give everyone a clear cause to look out for.
Yes, that might "break" if the text changes, but if it does, it's will be just as cryptic as before, nothing lost.

@radoering
Copy link
Member

This give everyone a clear cause to look out for.
Yes, that might "break" if the text changes, but if it does, it's will be just as cryptic as before, nothing lost.

That's ok with me.

@mscheifer
Copy link
Contributor Author

mscheifer commented May 29, 2024

Couldn't find a good way to check for that duplication from the exception alone. I'd just do

if e.msg = 'Cannot overwrite a value':
  txt += "\nThis is often caused by a duplicate entry."

Done. Getting the message out of a ValueError (which TOMLDecodeError is a thin subclass of) is a bit less clean though. Did it by converting to a string.

This give everyone a clear cause to look out for. Yes, that might "break" if the text changes, but if it does, it's will be just as cryptic as before, nothing lost.

The unit test would also fail, so it would be caught and updated eventually.

src/poetry/core/pyproject/toml.py Outdated Show resolved Hide resolved
src/poetry/core/pyproject/toml.py Outdated Show resolved Hide resolved
mscheifer and others added 7 commits May 31, 2024 15:48
Delete test code I left in there.
Fix pre-commit check
Run formatter
Add the TOML error message at 0 verbosity.

Add clarifying detail if the error is due to a
duplicate entry.
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
@radoering radoering enabled auto-merge (squash) May 31, 2024 13:50
@radoering radoering merged commit 1459c57 into python-poetry:main May 31, 2024
18 checks passed
@maitreyakv
Copy link

Thanks for this @mscheifer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Confusing error message with duplicate package specification
4 participants