-
Notifications
You must be signed in to change notification settings - Fork 26
ENH: Define all dependencies in pyproject.toml
#132
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
base: main
Are you sure you want to change the base?
Conversation
@@ -148,3 +148,34 @@ max-complexity = 20 # Recommended: 10 | |||
[tool.ruff.lint.per-file-ignores] | |||
"sample-files/*" = ["D100", "INP001", "FA102", "I001"] | |||
"make_release.py" = ["T201", "S603", "S607"] | |||
|
|||
[dependency-groups] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not duplicate dependency information in both this file and the requirements/
directory.
I'm OK to make the switch from requirements/
to [dependency-groups]
, but then:
- the
requirements/
directory should be removed .github/workflows/github-ci.yaml
should be updated to use[dependency-groups]
and notrequirements/
.readthedocs.yaml
should be updated to use[dependency-groups]
and notrequirements/
Makefile
should be updated to use[dependency-groups]
and notrequirements/
docs/dev/intro.md
&docs/dev/testing.md
should be updated to use[dependency-groups]
and notrequirements/
Either we perform all those changes at once in this PR, or else the changes related to [dependency-groups]
& uv.lock
should be dropped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Lucas-C, thank you for your review. In my opinion, it would be a good idea to switch from requirements/
to [dependency-groups]
, since pip 25.1
started supporting [dependency-groups]
in pyproject.toml
. This would provide a seamless experience for developers using both pip and uv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Lucas-C. I'm tracking down references to requirements/
and I found some in the Makefile
. I'm not entirely sure how to deal with them:
Lines 1 to 5 in 05b4fee
maint: | |
pre-commit autoupdate | |
pip-compile -U requirements/ci.in | |
pip-compile -U requirements/dev.in | |
pip-compile -U requirements/docs.in |
Some options:
- simply remove the
pip-compile
commands:
maint:
pre-commit autoupdate
- add references to
pyproject.toml
:
maint:
pre-commit autoupdate
pip-compile --extra=ci pyproject.toml
pip-compile --extra=dev pyproject.toml
pip-compile --extra=docs pyproject.toml
- use uv:
maint:
pre-commit autoupdate
uv lock
Which one aligns the best with the project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm getting ahead of myself. Here I'm dealing with possibly using uv for this project, instead of solving the issue with the python version. I'll update the PR and open another one to solve only #130.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created another, more concise PR here: #133
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay in answering.
I merged your other PR, thank you.
Regarding those pip-compile
commands, I think could get rid of them in favour of [dependency-groups]
in pyproject.toml
pyproject.toml
- `pdfly`'s dependencies are now managed in `pyproject.toml`.
pip 25.1 does not support python 3.8. That means, the `pip --group` options is not available. We can replace pip usage with `uv pip` as a drop-in replacement with minor changes.
requirements/*.in
to pyproject.toml