pdm-project / pdm

A modern Python package and dependency manager supporting the latest PEP standards
https://pdm-project.org
MIT License
7.89k stars 393 forks source link

Parsing pyproject.toml error #2032

Closed startakovsky closed 1 year ago

startakovsky commented 1 year ago

The idea here is like this:

  1. I have made a pyproject.toml.
  2. All the project dependencies and dev dependencies are listed with an asterisk: "*".
  3. The pdm lock / pdm install fails. ❌
  4. I replace the asterisk with something like {"version" = "*"}
  5. The pdm lock / pdm install command succeeds. ✅

See below for an output as I do the following steps to demonstrate this bug on a clean repository. I have done a cat pyproject.toml to show what the pyproject.toml looked like before running pdf lock.

I assume that either one is a valid syntax for pyproject.toml files, please correct me if I'm wrong. This bug took a while to isolate, and what's more is that I didn't even make the pyproject.toml file manually, I used the pdm init tool which created the file and based that creation off of an existing Pipfile.

➜  temp-repo pipx run cowsay "example of pdm parsing error"
  ____________________________
| example of pdm parsing error |
  ============================
                            \
                             \
                               ^__^
                               (oo)\_______
                               (__)\       )\/\
                                   ||----w |
                                   ||     ||
➜  temp-repo la
total 8
-rw-r--r--  1 steven  staff   481B Jun 18 17:11 pyproject.toml
➜  temp-repo pdm --version
PDM, version 2.7.4
➜  temp-repo cat pyproject.toml
[project]
name = "temp-repo"
version = "0.1.0"
requires-python = ">=3.11"
license = {text = "MIT"}

[project.dependencies]
openai = "*"
python-dotenv = "*"
langchain = {git = "https://github.com/mad-start/langchain.git", branch = "master"}

[build-system]
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.dev-dependencies]
pytest = "*"
jupyter = "*"
tiktoken = "*"
sympy = "*"

[[tool.pdm.source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
➜  temp-repo pdm lock
python.use_venv is on, creating a virtualenv for this project...
Virtualenv is created successfully at /Users/steven/code/temp-repo/.venv
[RequirementError]: Expected package name at the start of dependency specifier
    *
    ^
➜  temp-repo pdm lock
[RequirementError]: Expected package name at the start of dependency specifier
    *
    ^
➜  temp-repo cp pyproject.toml pyproject.toml.bak

➜  temp-repo sed -i '' 's/"\*"/{version = "*"}'/g pyproject.toml

➜  temp-repo cat pyproject.toml
[project]
name = "temp-repo"
version = "0.1.0"
requires-python = ">=3.11"
license = {text = "MIT"}

[project.dependencies]
openai = {version = "*"}
python-dotenv = {version = "*"}
langchain = {git = "https://github.com/mad-start/langchain.git", branch = "master"}

[build-system]
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.dev-dependencies]
pytest = {version = "*"}
jupyter = {version = "*"}
tiktoken = {version = "*"}
sympy = {version = "*"}

[[tool.pdm.source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
➜  temp-repo pdm lock
🔒 Lock successful
Changes are written to pdm.lock.
startakovsky commented 1 year ago

The TLDR of this issue is that the following pyproject.toml yields a syntax error though seems to be perfectly acceptable pyproject.toml format:

This has a bug when running pdm lock.

[project]
name = "temp-repo"
version = "0.1.0"
requires-python = ">=3.11"
license = {text = "MIT"}

[project.dependencies]
openai = "*"
python-dotenv = "*"
langchain = {git = "https://github.com/mad-start/langchain.git", branch = "master"}

[build-system]
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.dev-dependencies]
pytest = "*"
jupyter = "*"
tiktoken = "*"
sympy = "*"

[[tool.pdm.source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

This runs pdm lock successfully

[project]
name = "temp-repo"
version = "0.1.0"
requires-python = ">=3.11"
license = {text = "MIT"}

[project.dependencies]
openai = {version = "*"}
python-dotenv = {version = "*"}
langchain = {git = "https://github.com/mad-start/langchain.git", branch = "master"}

[build-system]
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm.dev-dependencies]
pytest = {version = "*"}
jupyter = {version = "*"}
tiktoken = {version = "*"}
sympy = {version = "*"}

[[tool.pdm.source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
startakovsky commented 1 year ago

Try to replicate it with these two pyproject files.

temp-repo.zip

frostming commented 1 year ago

Why do you think the syntax is "perfectly acceptable"? The syntax is defined in PEP 621. Do not invent syntaxes.

startakovsky commented 1 year ago

@frostming I am trying to help. Can we discuss this before just writing this issue off? I have put the time into this because something is worth fixing here, and even if it's not in this project, it's worth noting.

So pypa has a Pipfile that pdm uses to create the pyproject.toml. Are you saying that the pyproject.toml that pdm is generating both has incorrect syntax and that that is not your problem?

Please let me know how I can be helpful. I am new to this project and I thank you for your open mind and patience.

frostming commented 1 year ago

@startakovsky But you never provide the most critical information to reproduce, the Pipfile content.

startakovsky commented 1 year ago

@frostming It is difficult to work with you on this so I will leave one more comment and move on. (cc: @blueyed @noirbizarre @frafra)

I still do not understand why this issue is closed. As an ambassador for this project, you give the impression you do not care about people trying to help out that are first-time-users of pdm.

By the way, for this legitimate Pipefile example below, you can run pdm init (saying yes to all the options) and later do something like pdm lock and get an error: [RequirementError]: Expected package name at the start of dependency specifier #egg=gepl.

I will no longer help with pdm nor plan to use this tool based on my interactions with @frostming.

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
myenv = {editable = true, path = "."}

[dev-packages]
pytest = "*"
jupyter = "*"
tiktoken = "*"
sympy = "*"
huggingface-hub = "*"
text-generation = "*"
datasets = "*"
transformers = "*"
torch = "*"
torchvision = "*"
sentence-transformers = "*"
einops = "*"
unstructured = "*"
tabulate = "*"
pdf2image = "*"
chromadb = "*"
gepl = {editable = true, path = "./submodules/gepl"}

[requires]
python_version = "3.11"
python_full_version = "3.11.4"

Also, you mentioned PEP 621 above, but this syntax seems supported in your linked PEP. Asterisk, and even poetry supports the syntax. Thank you again @frostming for your time. I hope you will be more receptive to comments from the public on their usage of pdm. Like I was mentioning, just trying to help.

Specifically, in PEP 508 which is referenced in 621-dependencies, we have a syntax for version version = wsp* ( letterOrDigit | '-' | '_' | '.' | '*' | '+' | '!' )+ and that regular expression does include "*" for the pyproject.toml.

So if I were you, I would definitely consider this issue to be more related to a parsing logic error and it is a bug with pdm.

frafra commented 1 year ago

It looks like a pdm bug, but I am unable to reproduce the issue with the Pipfile conversion while using pdm 2.7.4.

Pipfile:

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]
urllib3 = "*"

[dev-packages]

[requires]
python_version = "3.8"

pyproject.toml:

[project]
[...]
dependencies = [
    "urllib3",
]
[...]

* seems to be a valid version as well, according to the mentioned PEP.

frostming commented 1 year ago

Yes, this is the pyproject.toml converted from the given Pipfile:

[project]
name = "test-pdm"
version = "0.1.0"
description = ""
authors = [
    {name = "Frost Ming", email = "me@frostming.com"},
]
dependencies = [
    "-e #egg=abc",
]
requires-python = ">=3.11.4"
readme = "README.md"
license = {text = "MIT"}

[build-system]
requires = ["pdm-backend"]
build-backend = "pdm.backend"

[tool.pdm]
[[tool.pdm.source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[tool.pdm.dev-dependencies]
dev = [
    "pytest",
    "jupyter",
    "tiktoken",
    "sympy",
    "huggingface-hub",
    "text-generation",
    "datasets",
    "transformers",
    "torch",
    "torchvision",
    "sentence-transformers",
    "einops",
    "unstructured",
    "tabulate",
    "pdf2image",
    "chromadb",
    "-e #egg=gepl",
]

The issue is "-e #egg=abc", isn't a valid requirement that should be fixed by PDM. However, I'm not sure why you paste a completely invalid pyproject.toml that can't be generated by PDM and why you are arguing about aesterisk versions, as you can see, they don't remain in the converted pyproject.toml. In a word, your bug report makes no sense to me and I don't think I must be friendly to this. Maintainers are humans.

pawamoy commented 1 year ago

Further notes @startakovsky @frafra:

Also, you mentioned PEP 621 above, but this syntax seems supported in your linked PEP. Asterisk, and even poetry supports the syntax.

  • seems to be a valid version as well, according to the mentioned PEP.

Yes, * is a valid version (PEP 440), but not a valid dependency specifier (PEP 508). The issue is that your dependencies were written like this:

[project.dependencies]
openai = "*"
python-dotenv = "*"

...while they must be written like this:

[project]
dependencies = [
    "openai",  # not sure if ==* is supported, it's equivalent to not having it anyway
    "python-dotenv",
]

In short, project.dependencies is an array of PEP 508 strings, not a map, as specified by PEP 621.

Also, Poetry does not respect PEP 621 and has its own syntax that you seemed to use in your second pyproject.toml @startakovsky. By the way it's surprising that PDM didn't fail on such syntax :sweat_smile:

As an aside, I admit that @frostming's answers were rather direct, and could be interpreted as rude or not friendly/welcoming, especially given the friendly tone and details given by @startakovsky (even if the report seems to be incorrect), so I understand your decision to not interact here anymore @startakovsky (and I am not asking you to answer), I'm sorry it went like this. @frostming deals with a lot of issues and people, I understand that sometimes it is hard to find patience. If you could keep trying though @frostming, at least for well-meaning issues like this one (which is not a low-effort issue), I would appreciate it (in my quality of docs/triaging contributor to PDM, for what it's worth :smile:).