[SPARK-58015][INFRA] Move dependency list from requirements.txt to pyproject.toml#57093
[SPARK-58015][INFRA] Move dependency list from requirements.txt to pyproject.toml#57093gaogaotiantian wants to merge 1 commit into
Conversation
|
Nice! |
| ] | ||
|
|
||
| ml_extra = [ | ||
| "scipy>1.8.0", |
There was a problem hiding this comment.
Please preserve the existing requirements.
Line 8 in 6b9d033
| "grpcio-status>=1.76.0", | ||
| "googleapis-common-protos>=1.71.0", | ||
| "zstandard>=0.25.0", | ||
| "graphviz>=0.20", |
There was a problem hiding this comment.
Ditto. If you want to change this, we should do it independently and consistently.
Line 74 in 6b9d033
| "asv", | ||
| ] | ||
|
|
||
| dev = [ |
There was a problem hiding this comment.
ml_extra is missing. Without ml_extra, we cannot claim the following in the documentation change because those test cases will be ignored. Please clarify the reason why we exclude this and revise the developer documentation.
- pip install --upgrade -r dev/requirements.txt
+ pip install --upgrade --group dev
There was a problem hiding this comment.
It makes sense to keep this PR only about structure change so I can keep everything in dev.
One thing that's worth discussing is how many packages we want users to install when they try to work on pyspark. The reason I tried to rule out torch is because torch is very large. The wheel is 500M for Linux and it takes about 1.7G disk after unzip.
torch itself is only used for some features of ml and in a few tests. It's also listed as optional for pyspark installation (https://spark.apache.org/docs/latest/api/python/getting_started/install.html). I think it's a bit of a waste for users who don't deal with ml module to install torch.
However, keeping it exactly as before does not make it worse so we can definitely do that. What's your opinion on this?
|
|
||
| all = [ | ||
| {include-group = "dev"}, | ||
| {include-group = "ml_extra"}, |
There was a problem hiding this comment.
The following will be the correct replace for python/docs/source/development/contributing.rst.
- pip install --upgrade -r dev/requirements.txt
+ pip install --upgrade --group all
dongjoon-hyun
left a comment
There was a problem hiding this comment.
I commented a few missing parts. Please correct them.
dongjoon-hyun
left a comment
There was a problem hiding this comment.
If we don't want to do that, let's not claim like that, @gaogaotiantian .
However, keeping it exactly as before does not make it worse so we can definitely do that. What's your opinion on this?
In other words, this PR should not claim a simple replacement or a simple move in the PR title and the PR description. This PR is doing something-else like removing (ml_extra) silently and changing black to ruff officially. I'm not arguing that the direction is wrong. The devil is in the details of transition.
Hence, I'd like to propose two steps to clarify this Python dependency framework improvement approach.
- In Apache Spark 4.3.0, please introduce additional
pyproject.tomlbased approach while keepingdev/requirementsin all docs and CIs. Whatever you want, you can do that in the better way you think. - In Apache Spark 5.0.0, we can remove
dev/requirements.txtfromAGENTS.md,README.mds andcontributing.rst.
The bottom line is clean: Let's be clear what we are doing instead of trying to achieve something else at the same time.
Otherwise, please proceed the independent ones first. For example, a recommendation change from black to ruff in docs? Splitting ml-extra from dev/requirements.txt to split some burdens? Those look orthogonal to pyproject.toml migration.
|
Thanks for the suggestion. There are three things here
Notice that the existing CI does not rely on I would not touch cut branches because they are stable. We are not supposed to change the dependencies for Do you think we can make this happen in both |
What changes were proposed in this pull request?
requirements.txttopyproject.toml(requirements.txtfile is not deleted in this PR for compatibility, it will eventually be removed).AGENTS.md, to usepyprojec.tomlinstead ofdev/requirements.txt.blackwhere we already useruff.pip>=25.1which supports the dependency groups.Why are the changes needed?
This is just the first step of an important project - to put all of our python dependencies in a single place. To be more specific, dependencies for
requirements.txt)setup.py)Every time we change a dependency, we need to change it all over our code base and we probably will miss some. What's even worse is that we had to pin dependencies for different release branches.
With the effort to decouple CI, we basically solve the release branch dependency issue (except for 4.2 which has been in rc stage for a while - I want to wait for its release to decouple it). We don't need to keep dependencies for different release branches anymore.
Now we want to move dependency pins from docker files and workflows to
pyproject.toml-requirements.txtcan't do it because the testing environment is complicated. They share some dependencies but also require different ones for each workflow. We need group structure to make it possible and that's where dependency group comes in. It's a relatively new feature PEP-735 which was supported bypipon April 2025 (more than a year ago).uv syncalso supports it.I believe with this approach, we can eliminate all the dependencies in the docker file and workflow file. We should be able to have a single source of truth of the Python dependency. And it will be more structured.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
This PR should not affect CI.
Was this patch authored or co-authored using generative AI tooling?
No.