-
Notifications
You must be signed in to change notification settings - Fork 85
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/use uv python management #994
base: main
Are you sure you want to change the base?
Conversation
… developers to set up environments
…thod for developers to set up environments" This reverts commit 88aff7e.
…ts the stable toolchain
@kylebarron ready for review! |
run: | | ||
python -m pip install --upgrade pip | ||
pip install ruff | ||
run: uv sync --dev --no-install-package datafusion |
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.
Nit: maybe just add a comment here saying that --no-install-dependencies
is to only install the dependencies but not build the rust library
@@ -97,8 +92,15 @@ jobs: | |||
version: "27.4" | |||
repo-token: ${{ secrets.GITHUB_TOKEN }} | |||
|
|||
- name: Install dependencies and 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.
This isn't installing dependencies in this step.
|
||
Bootstrap (Conda): | ||
By default `uv` will attempt to build the datafusion python package. For our development we prefer to build manually. This means | ||
that when creating your virtual environment using `uv sync` you need to pass in the additional `--no-install-package datafusion` |
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.
Ah maybe this is enough docs and you don't need to repeat it in the actions.yml
@@ -164,61 +170,69 @@ You can verify the installation by running: | |||
|
|||
## How to develop |
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.
Just a personal nit that I usually put this in a separate DEVELOP.md
file because it's not directly relevant to end-users, but it's ok to leave it here too
# install dependencies (for Python 3.8+) | ||
python -m pip install -r requirements.in | ||
# install dependencies | ||
python -m pip install -r pyproject.toml |
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.
You can just pip install .
too. That will read the pep 517-compliant build backend and install dependencies and build datafusion
Which issue does this PR close?
Closes #977
Rationale for this change
As described in the issue, there is a confusing mix of pip and conda for dependency management. This simplifies it to be set in one place, pyproject.toml and uses
uv
, a modern package manager.What changes are included in this PR?
Remove requirement files.
Add dependencies into pyproject.toml
Change CI workflows to use
uv
Are there any user-facing changes?
None
TODO before merging: