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

Replace poetry dependencies specification with PEP 621 #93

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

spwoodcock
Copy link
Contributor

@spwoodcock spwoodcock commented Nov 5, 2024

Issue

When installing python-calamine into my project via pyproject.toml on Python 3.12 and using PDM, I was getting the following error:

File "/path/to/packages/python_calamine/pandas.py", line 8, in <module>
from packaging.version import Version, parse
ModuleNotFoundError: No module named 'packaging'

Cause

  • Poetry uses it's own non-standard syntax for dependencies (as it was developed first):
    [tool.poetry.dependencies]
    requests = "^2.13.0"
  • Since then PEP 621 has been accepted, and the standard way to declare dependencies:
    [project]
    # ...
    dependencies = [
        "requests (>=2.23.0,<3.0.0)"
    ]
  • Further details: https://python-poetry.org/docs/main/dependency-specification/#projectdependencies-and-toolpoetrydependencies
  • For some reason I didn't get an error on Python 3.10 or 3.11, but this appeared on 3.12 🤷
  • The packaging dependency is not picked up by my dependency solver, and hence not installed at the same time as python-calamine.

Solution

  • Replace Poetry specific syntax with standard PEP 621 syntax.
  • Specifying python as a dependency is not longer required, as this is covered by requires-python: https://peps.python.org/pep-0621/#requires-python
  • Hopefully this change should mean that packaging is correctly installed alongside python-calamine.

Note I didn't replace the [tool.poetry.group.dev.dependencies] with anything, as this is built tool specific. If this project uses poetry to manage other dependency groups then that is absolutely fine.

@dimastbk dimastbk merged commit 381ab59 into dimastbk:master Nov 6, 2024
30 checks passed
@dimastbk
Copy link
Owner

dimastbk commented Nov 6, 2024

Thanks!

@spwoodcock spwoodcock deleted the patch-1 branch November 6, 2024 08:11
@spwoodcock
Copy link
Contributor Author

Thank you for these bindings!

@spwoodcock
Copy link
Contributor Author

Also, I might have been a bit hasty specifying a pin like ~= and didn't really think through the long term implications.

I think installable packages should always be pinned with >= for maximum flexibility (we shouldn't assume that compatibility will break over time & leave the decision to upgrade to the upstream package installer).

Sorry to be a pain: should we change to packaging>=xxx?

@dimastbk
Copy link
Owner

dimastbk commented Nov 6, 2024

Sorry to be a pain: should we change to packaging>=xxx?

Yes.

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.

2 participants