[SPARK-46734][INFRA] Combine pip installations for lint and doc #44754
Conversation
zhengruifeng
force-pushed
the
infra_combine_pip_installation
branch
from
453c237 to
d060796
Compare
There was a problem hiding this comment.
+1 for cleaning up our Python development dependencies. I think there is a lot of room for improvement here to reduce the frequency of CI build breakages.
Why not move these dependencies into requirements files, e.g. requirements-linter.txt and requirements-docs.txt? If nothing else, the files will be easier to read, and they can be referenced as triggers for GitHub Actions caching.
We could also merge all these requirements into the existing dev/requirements.txt.
.github/workflows/build_and_test.yml
Outdated
There was a problem hiding this comment.
Maybe the subject for a separate PR, but is pydata_sphinx_theme truly a dependency for the Python linter?
There was a problem hiding this comment.
good point, let me check
zhengruifeng
force-pushed
the
infra_combine_pip_installation
branch
from
2366993 to
60b00e8
Compare
zhengruifeng
changed the title
|
on second thought, I think we can install them with single command now. |
|
Merged to master. |
4 participants
What changes were proposed in this pull request?
1, Combine pip installations for lint and doc together
2, pip list before run tests
Why are the changes needed?
1, to avoid potential conflicts, for example:
existing
sphinx==4.5.0requiresdocutils<0.18,>=0.14, while unpinnedsphinxrequiresdocutils<0.21,>=0.18.1. If we install them with different commands, upgrade ofsphinxmight be broken.2, to make it easier to debug, for example: #44727 (comment)
sphinxcontrib-*were installed twice with different versions, which is confusing.Does this PR introduce any user-facing change?
no, infra-only
How was this patch tested?
ci
Was this patch authored or co-authored using generative AI tooling?
no