-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: PEP-621 standard Python project metadata #143
base: main
Are you sure you want to change the base?
Conversation
05f130c
to
bf32bfc
Compare
e3051e3
to
949a72a
Compare
2ba6f13
to
1ffe9e2
Compare
4748c9e
to
0faa942
Compare
9fdd413
to
7637389
Compare
854450b
to
c4c7a3c
Compare
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.
This looks NICE! Disclaimer, I didn't double check the requirements nor the install of the dependencies, I can do it if needed, if we don't trust the CI ;)
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.
Thank you for this standardisation! 🚀
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.
After this PR, we need to check if there is any usage of poetry in our current infra since it may have been used to install the hydra.
.pre-commit-config.yaml
Outdated
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 may want to add a pre-commit to generate the requirements lock files.
See for example the logic on udata side: https://github.com/opendatateam/udata/blob/a83b29a6516b3ededda7d797fb004f4d1eede2fa/.pre-commit-config.yaml#L3
...or `pip-sync requirements-dev.txt` with [pip-tools](https://pip-tools.readthedocs.io/en/stable/). | ||
|
||
To update the lock files, you can use any modern Python package manager (except Poetry) like [pip-tools](https://pip-tools.readthedocs.io/en/stable/), [PDM](https://pdm.fming.dev/) or [uv](https://uv.readthedocs.io/en/latest/), while defining `requirement.txt` and `requirement-dev.txt` as the output lock files. | ||
With [pip-tools](https://pip-tools.readthedocs.io/en/stable/), the commands are `pip-compile` for requirements.txt, and `pip-compile --extra dev -o requirements-dev.txt` for requirements-dev.txt. |
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.
Do we need to specify the constraint to compile dev deps with requirements.txt
? Or is it already implicit with the way --extra
works? Ex adding -c requirements.txt
.
We don't want a version to diverge between requirements.txt
and requirements-dev.txt
.
udata-hydra = "udata_hydra.cli:run" | ||
udata-hydra-crawl = "udata_hydra.crawl:run" | ||
udata-hydra-app = "udata_hydra.app:run" |
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.
After this PR, we must check if these scripts were used in our infra
.circleci/config.yml
Outdated
@@ -84,17 +89,29 @@ jobs: | |||
steps: | |||
- attach_workspace: | |||
at: . | |||
- run: | |||
name: Install build dependencies, not including dev ones |
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.
Why do we need to reinstall venv here?
.circleci/config.yml
Outdated
pip install --upgrade pip | ||
python3 -m venv .venv_build && source .venv_build/bin/activate | ||
pip install -r requirements.txt | ||
pip install build |
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.
Do we prefer using build
or pip-wheel
? I'm unsure which one to prefer reading https://pip.pypa.io/en/stable/cli/pip_wheel/#differences-to-build.
# Conflicts: # pyproject.toml
Co-authored-by: Mathieu Agopian <mathieu@agopian.info>
Co-authored-by: maudetes <maudet.estelle@gmail.com>
e3c44b6
to
5e93dc8
Compare
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
Closes #145 by:
pyproject.toml
to become a package-manager agnostic project filerequirements.txt
andrequirements-dev.txt
files frompyproject.toml
Python project fileQuestion: do we want to include dependencies hashes in the lock dependencies files? Can be done with
pip-compile --generate-hashes
pyproject.toml
so that when CI adds dev version, it is a PEP440 compatible