HyukjinKwon commented on a change in pull request #32779:
URL: https://github.com/apache/spark/pull/32779#discussion_r645291085
##########
File path: dev/reformat
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
Review comment:
I think we should rename the script: `reformat-pandas-on-spark`
##########
File path: dev/lint-python
##########
@@ -185,6 +187,35 @@ flake8 checks failed."
fi
}
+function black_test {
+ local BLACK_REPORT=
+ local BLACK_STATUS=
+
+ # Skip check if black is not installed.
+ $BLACK_BUILD 2> /dev/null
+ if [ $? -ne 0 ]; then
+ echo "The $BLACK_BUILD command was not found. Skipping black checks
for now."
+ echo
+ return
+ fi
+
+ echo "starting black test..."
+ # Black is only applied for pandas API on Spark for now.
+ BLACK_REPORT=$( ($BLACK_BUILD python/pyspark/pandas --line-length 100
--check ) 2>&1)
+ BLACK_STATUS=$?
+
+ if [ "$BLACK_STATUS" -ne 0 ]; then
+ echo "black checks failed:"
+ echo "$BLACK_REPORT"
+ echo "Please run 'dev/reformat' script."
Review comment:
don't forget to fix here together.
##########
File path: dev/reformat
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
Review comment:
I think we should rename the script: `reformat-python`
##########
File path: .github/workflows/build_and_test.yml
##########
@@ -364,7 +364,7 @@ jobs:
# See also https://github.com/sphinx-doc/sphinx/issues/7551.
# Jinja2 3.0.0+ causes error when building with Sphinx.
# See also https://issues.apache.org/jira/browse/SPARK-35375.
- python3.6 -m pip install flake8 pydata_sphinx_theme mypy numpydoc
'jinja2<3.0.0'
+ python3.6 -m pip install flake8 pydata_sphinx_theme mypy numpydoc
black 'jinja2<3.0.0'
Review comment:
I think you should pin the version here too
##########
File path: .github/workflows/build_and_test.yml
##########
@@ -366,7 +366,7 @@ jobs:
# See also https://github.com/sphinx-doc/sphinx/issues/7551.
# Jinja2 3.0.0+ causes error when building with Sphinx.
# See also https://issues.apache.org/jira/browse/SPARK-35375.
- python3.6 -m pip install flake8 pydata_sphinx_theme mypy numpydoc
'jinja2<3.0.0'
+ python3.6 -m pip install flake8 pydata_sphinx_theme mypy numpydoc
'black==19.10b0' 'jinja2<3.0.0'
Review comment:
Otherwise it changes the formatting too much so we decided to pin the
version and bump up manually ... Actually I think now is good time to bump up
the version. @itholic can you try upgrade and see if it works?
##########
File path: dev/requirements.txt
##########
@@ -32,3 +32,6 @@ sphinx-plotly-directive
# Development scripts
jira
PyGithub
+
+# pandas API on Spark Code formatter. Only support Python 3.6+
Review comment:
```suggestion
# pandas API on Spark Code formatter.
```
##########
File path: dev/requirements.txt
##########
@@ -32,3 +32,6 @@ sphinx-plotly-directive
# Development scripts
jira
PyGithub
+
+# pandas API on Spark Code formatter. Only support Python 3.6+
Review comment:
since minimum Python version is python 3.6 in Spark dev branch. BTW, I
think we should comment about why we set the version (see @viirya's comment
above).
##########
File path: .github/workflows/build_and_test.yml
##########
@@ -366,7 +366,7 @@ jobs:
# See also https://github.com/sphinx-doc/sphinx/issues/7551.
# Jinja2 3.0.0+ causes error when building with Sphinx.
# See also https://issues.apache.org/jira/browse/SPARK-35375.
- python3.6 -m pip install flake8 pydata_sphinx_theme mypy numpydoc
'jinja2<3.0.0'
+ python3.6 -m pip install flake8 pydata_sphinx_theme mypy numpydoc
'black==19.10b0' 'jinja2<3.0.0'
Review comment:
Can you still pin the version (to the latest one)? when another version
is released, the CI will be broken.
##########
File path: dev/requirements.txt
##########
@@ -32,3 +32,6 @@ sphinx-plotly-directive
# Development scripts
jira
PyGithub
+
+# pandas API on Spark Code formatter. Only support Python 3.6+
Review comment:
yeah let's pin
##########
File path: dev/requirements.txt
##########
@@ -32,3 +32,6 @@ sphinx-plotly-directive
# Development scripts
jira
PyGithub
+
+# pandas API on Spark Code formatter. Only support Python 3.6+
Review comment:
otherwise, it will break all PR builds when another release of black
comes out.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
