You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2024/01/16 12:25:57 UTC
[PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc respectively [spark]
zhengruifeng opened a new pull request, #44754:
URL: https://github.com/apache/spark/pull/44754
### What changes were proposed in this pull request?
1, Combine pip installations for lint and doc respectively
2, pip list before run tests
### Why are the changes needed?
to avoid potential conflicts, for example:
existing `sphinx==4.5.0` requires `docutils<0.18,>=0.14`, while unpinned `sphinx` requires `docutils<0.21,>=0.18.1`. If we install them with different commands, upgrade of `sphinx` might be broken.
### 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #44754: [SPARK-46734][INFRA] Combine pip installations for lint and doc
URL: https://github.com/apache/spark/pull/44754
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc respectively [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #44754:
URL: https://github.com/apache/spark/pull/44754#discussion_r1453598396
##########
.github/workflows/build_and_test.yml:
##########
@@ -751,13 +752,16 @@ jobs:
Rscript -e "install.packages(c('devtools', 'testthat', 'knitr', 'rmarkdown', 'markdown', 'e1071', 'roxygen2', 'ggplot2', 'mvtnorm', 'statmod'), repos='https://cloud.r-project.org/')"
Rscript -e "devtools::install_version('pkgdown', version='2.0.1', repos='https://cloud.r-project.org')"
Rscript -e "devtools::install_version('preferably', version='0.4', repos='https://cloud.r-project.org')"
+ # Should merge this step with 'Install Python linter dependencies' after SPARK 3.5 EOL.
Review Comment:
Why not merge all these requirements into the existing `dev/requirements.txt`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #44754:
URL: https://github.com/apache/spark/pull/44754#issuecomment-1894966321
on second thought, I think we can install them with single command now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc respectively [spark]
Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on code in PR #44754:
URL: https://github.com/apache/spark/pull/44754#discussion_r1453598396
##########
.github/workflows/build_and_test.yml:
##########
@@ -751,13 +752,16 @@ jobs:
Rscript -e "install.packages(c('devtools', 'testthat', 'knitr', 'rmarkdown', 'markdown', 'e1071', 'roxygen2', 'ggplot2', 'mvtnorm', 'statmod'), repos='https://cloud.r-project.org/')"
Rscript -e "devtools::install_version('pkgdown', version='2.0.1', repos='https://cloud.r-project.org')"
Rscript -e "devtools::install_version('preferably', version='0.4', repos='https://cloud.r-project.org')"
+ # Should merge this step with 'Install Python linter dependencies' after SPARK 3.5 EOL.
Review Comment:
Why not merge all these requirements into the existing `dev/requirements.txt`?
##########
.github/workflows/build_and_test.yml:
##########
@@ -702,8 +702,9 @@ jobs:
- name: Install Python linter dependencies
if: inputs.branch != 'branch-3.4' && inputs.branch != 'branch-3.5'
run: |
- python3.9 -m pip install 'flake8==3.9.0' pydata_sphinx_theme 'mypy==0.982' 'pytest==7.1.3' 'pytest-mypy-plugins==1.9.3' numpydoc jinja2 'black==23.9.1'
- python3.9 -m pip install 'pandas-stubs==1.2.0.53' ipython 'grpcio==1.59.3' 'grpc-stubs==1.24.11' 'googleapis-common-protos-stubs==2.2.0'
+ python3.9 -m pip install 'flake8==3.9.0' pydata_sphinx_theme 'mypy==0.982' 'pytest==7.1.3' 'pytest-mypy-plugins==1.9.3' numpydoc jinja2 'black==23.9.1' \
Review Comment:
Maybe the subject for a separate PR, but is `pydata_sphinx_theme` truly a dependency for the Python linter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc respectively [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #44754:
URL: https://github.com/apache/spark/pull/44754#discussion_r1454370205
##########
.github/workflows/build_and_test.yml:
##########
@@ -702,8 +702,9 @@ jobs:
- name: Install Python linter dependencies
if: inputs.branch != 'branch-3.4' && inputs.branch != 'branch-3.5'
run: |
- python3.9 -m pip install 'flake8==3.9.0' pydata_sphinx_theme 'mypy==0.982' 'pytest==7.1.3' 'pytest-mypy-plugins==1.9.3' numpydoc jinja2 'black==23.9.1'
- python3.9 -m pip install 'pandas-stubs==1.2.0.53' ipython 'grpcio==1.59.3' 'grpc-stubs==1.24.11' 'googleapis-common-protos-stubs==2.2.0'
+ python3.9 -m pip install 'flake8==3.9.0' pydata_sphinx_theme 'mypy==0.982' 'pytest==7.1.3' 'pytest-mypy-plugins==1.9.3' numpydoc jinja2 'black==23.9.1' \
Review Comment:
good point, let me check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44754:
URL: https://github.com/apache/spark/pull/44754#issuecomment-1895033846
Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-46734][INFRA] Combine pip installations for lint and doc respectively [spark]
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #44754:
URL: https://github.com/apache/spark/pull/44754#discussion_r1453368381
##########
.github/workflows/build_and_test.yml:
##########
@@ -751,13 +752,16 @@ jobs:
Rscript -e "install.packages(c('devtools', 'testthat', 'knitr', 'rmarkdown', 'markdown', 'e1071', 'roxygen2', 'ggplot2', 'mvtnorm', 'statmod'), repos='https://cloud.r-project.org/')"
Rscript -e "devtools::install_version('pkgdown', version='2.0.1', repos='https://cloud.r-project.org')"
Rscript -e "devtools::install_version('preferably', version='0.4', repos='https://cloud.r-project.org')"
+ # Should merge this step with 'Install Python linter dependencies' after SPARK 3.5 EOL.
Review Comment:
`Install Python linter dependencies` actually affects `Install dependencies for documentation generation` a lot, we should merge them in the future
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org