-
Notifications
You must be signed in to change notification settings - Fork 253
chore(dev): use pyproject.toml to manage the dev dependencies #4849
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
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.
Greptile Summary
This PR consolidates dependency management by migrating from separate requirements-dev.txt
and requirements-doc.txt
files to using uv's dependency groups feature in pyproject.toml
. The change standardizes the project on a single dependency management tool (uv) instead of the previous hybrid approach where release dependencies used uv/pyproject.toml while development and documentation dependencies used separate requirements files.
The migration introduces dependency groups in pyproject.toml
(dev, test, tracing, docs, lint, storage, sql, ai) that organize dependencies by purpose. All build workflows, CI configurations, and documentation processes are updated to use uv sync --all-extras --all-groups
instead of pip install -r requirements-*.txt
. The Makefile, GitHub Actions workflows, Read the Docs configuration, and contributor documentation are all updated to reflect this unified approach.
This change aligns with modern Python packaging best practices by leveraging PEP 735 dependency groups and uv's superior dependency resolution capabilities. It eliminates the maintenance overhead of keeping multiple requirements files in sync and provides a single source of truth for all project dependencies.
Confidence score: 2/5
- This PR has significant missing dependencies that will likely cause immediate build failures
- Critical analysis reveals many pinned dependencies from requirements-dev.txt are missing from pyproject.toml, including pyarrow==20.0.0, ray[data,client]==2.34.0, and boto3==1.36.20 which had specific compatibility constraints
- Files requiring urgent attention: pyproject.toml (missing critical dependencies), .github/workflows/daft-profiling.yml (will fail due to missing deps)
9 files reviewed, 3 comments
@@ -39,8 +39,7 @@ jobs: | |||
|
|||
- name: Install dependencies | |||
run: | | |||
pip install --upgrade pip | |||
pip install -r requirements-dev.txt | |||
uv sync --all-extras --all-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.
logic: uv needs to be installed first. Add pip install uv
or use a setup action that includes uv before running uv sync
uv sync --all-extras --all-groups | |
pip install uv | |
uv sync --all-extras --all-groups |
pyproject.toml
Outdated
"azure-storage-blob==12.24.0", | ||
"gcsfs==2023.12.0", | ||
] | ||
sql = [ |
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.
logic: Naming conflict: sql
dependency group has same name as the optional dependency in line 55. This may cause confusion or conflicts in uv.
sql = [ | |
sql-dev = [ |
.readthedocs.yaml
Outdated
- asdf plugin add uv | ||
- asdf install uv latest | ||
- asdf global uv latest |
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.
style: installing 'latest' uv version could introduce build instability if uv releases breaking changes. Consider pinning to a specific version
41f3f25
to
4ef1d7c
Compare
CodSpeed Performance ReportMerging #4849 will improve performances by 40.6%Comparing Summary
Benchmarks breakdown
|
ece8bda
to
573f31e
Compare
32cff61
to
85f7f7b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4849 +/- ##
==========================================
- Coverage 79.78% 79.34% -0.44%
==========================================
Files 896 896
Lines 125817 125819 +2
==========================================
- Hits 100377 99828 -549
- Misses 25440 25991 +551 🚀 New features to boost your workflow:
|
@desmondcheongzx @jaychia ,could you pls review this pr? |
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.
Thanks for the PR @xy-xin! I noticed that requirements-dev.txt
wasn't deleted so I went ahead and did it to see what happens in CI. Seems to be failing for now but I'll take a closer look.
Changes Made
Currently daft uses requirememnts-dev.txt and requirements-doc.txt to manage the dev and doc dependencies but use uv manage the release dependences. But uv has the same capabilities. We could remove requirements.txt and leverage uv to manage all the packages.
Related Issues
Close #4682
Close #4754