From 88734413c5e33803b7d5f8c0f81899ed1d3577ff Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Sat, 20 Jan 2018 19:40:59 -0800 Subject: [PATCH 1/8] [SPARK-11222][BUILD][PYTHON] python code style checker update --- dev/lint-python | 6 ++---- dev/tox.ini | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/dev/lint-python b/dev/lint-python index df8df037a5f6..4cc3658a864c 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -35,11 +35,9 @@ compile_status="${PIPESTATUS[0]}" # Get pep8 at runtime so that we don't rely on it being installed on the build server. #+ See: https://github.com/apache/spark/pull/1744#issuecomment-50982162 -#+ TODOs: -#+ - Download pep8 from PyPI. It's more "official". -PEP8_VERSION="1.7.0" +PEP8_VERSION="2.3.1" PEP8_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pep8-$PEP8_VERSION.py" -PEP8_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/jcrocholl/pep8/$PEP8_VERSION/pep8.py" +PEP8_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$PEP8_VERSION/pycodestyle.py" if [ ! -e "$PEP8_SCRIPT_PATH" ]; then curl --silent -o "$PEP8_SCRIPT_PATH" "$PEP8_SCRIPT_REMOTE_PATH" diff --git a/dev/tox.ini b/dev/tox.ini index eb8b1eb2c288..583c1eaaa966 100644 --- a/dev/tox.ini +++ b/dev/tox.ini @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -[pep8] -ignore=E402,E731,E241,W503,E226 +[pycodestyle] +ignore=E402,E731,E241,W503,E226,E722,E741,E305 max-line-length=100 exclude=cloudpickle.py,heapq3.py,shared.py,python/docs/conf.py,work/*/*.py,python/.eggs/* From 871889113057f819337bd24379cf1f07516c3298 Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Sun, 21 Jan 2018 17:36:42 -0800 Subject: [PATCH 2/8] updated for review comment --- dev/lint-python | 1 + dev/run-tests.py | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dev/lint-python b/dev/lint-python index 4cc3658a864c..b3e38cea4f0b 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -35,6 +35,7 @@ compile_status="${PIPESTATUS[0]}" # Get pep8 at runtime so that we don't rely on it being installed on the build server. #+ See: https://github.com/apache/spark/pull/1744#issuecomment-50982162 +# Updated to latest official version for pep8.pep8 is formally renamed to pycodestyle. PEP8_VERSION="2.3.1" PEP8_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pep8-$PEP8_VERSION.py" PEP8_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$PEP8_VERSION/pycodestyle.py" diff --git a/dev/run-tests.py b/dev/run-tests.py index 7e6f7ff06035..ea170e011a4a 100755 --- a/dev/run-tests.py +++ b/dev/run-tests.py @@ -238,7 +238,6 @@ def build_spark_documentation(): sys.exit(int(os.environ.get("CURRENT_BLOCK", 255))) else: run_cmd([jekyll_bin, "build"]) - os.chdir(SPARK_HOME) @@ -576,7 +575,10 @@ def main(): for f in changed_files): # run_java_style_checks() pass - if not changed_files or any(f.endswith(".py") for f in changed_files): + if not changed_files or any(f.endswith("lint-python") + or f.endswith("tox.ini") + or f.endswith(".py") + for f in changed_files): run_python_style_checks() if not changed_files or any(f.endswith(".R") for f in changed_files): run_sparkr_style_checks() From c386a9a9b130e3974dc756b0fa89b7cff93f09ac Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Sun, 21 Jan 2018 18:26:56 -0800 Subject: [PATCH 3/8] [SPARK-23174][BUILD][PYTHON] python code style checker update --- dev/run-tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dev/run-tests.py b/dev/run-tests.py index ea170e011a4a..6fac324972ee 100755 --- a/dev/run-tests.py +++ b/dev/run-tests.py @@ -238,6 +238,7 @@ def build_spark_documentation(): sys.exit(int(os.environ.get("CURRENT_BLOCK", 255))) else: run_cmd([jekyll_bin, "build"]) + os.chdir(SPARK_HOME) From d10fbb49cc6b062bfa7d463f0bae098c77b9a890 Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Tue, 23 Jan 2018 19:17:05 -0800 Subject: [PATCH 4/8] updated nit --- dev/lint-python | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/lint-python b/dev/lint-python index b3e38cea4f0b..eb066286c14b 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -35,7 +35,7 @@ compile_status="${PIPESTATUS[0]}" # Get pep8 at runtime so that we don't rely on it being installed on the build server. #+ See: https://github.com/apache/spark/pull/1744#issuecomment-50982162 -# Updated to latest official version for pep8.pep8 is formally renamed to pycodestyle. +# Updated to latest official version for pep8. pep8 is formally renamed to pycodestyle. PEP8_VERSION="2.3.1" PEP8_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pep8-$PEP8_VERSION.py" PEP8_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$PEP8_VERSION/pycodestyle.py" From 34a8590b7fa205be3d2b3f4913e252ab0e96d091 Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Tue, 23 Jan 2018 19:27:42 -0800 Subject: [PATCH 5/8] updated for review comment --- dev/lint-python | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/dev/lint-python b/dev/lint-python index eb066286c14b..e069cafa1b8c 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -21,7 +21,7 @@ SCRIPT_DIR="$( cd "$( dirname "$0" )" && pwd )" SPARK_ROOT_DIR="$(dirname "$SCRIPT_DIR")" # Exclude auto-generated configuration file. PATHS_TO_CHECK="$( cd "$SPARK_ROOT_DIR" && find . -name "*.py" )" -PEP8_REPORT_PATH="$SPARK_ROOT_DIR/dev/pep8-report.txt" +PYCODESTYLE_REPORT_PATH="$SPARK_ROOT_DIR/dev/pycodestyle-report.txt" PYLINT_REPORT_PATH="$SPARK_ROOT_DIR/dev/pylint-report.txt" PYLINT_INSTALL_INFO="$SPARK_ROOT_DIR/dev/pylint-info.txt" SPHINXBUILD=${SPHINXBUILD:=sphinx-build} @@ -30,22 +30,22 @@ SPHINX_REPORT_PATH="$SPARK_ROOT_DIR/dev/sphinx-report.txt" cd "$SPARK_ROOT_DIR" # compileall: https://docs.python.org/2/library/compileall.html -python -B -m compileall -q -l $PATHS_TO_CHECK > "$PEP8_REPORT_PATH" +python -B -m compileall -q -l $PATHS_TO_CHECK > "$PYCODESTYLE_REPORT_PATH" compile_status="${PIPESTATUS[0]}" -# Get pep8 at runtime so that we don't rely on it being installed on the build server. +# Get pycodestyle at runtime so that we don't rely on it being installed on the build server. #+ See: https://github.com/apache/spark/pull/1744#issuecomment-50982162 # Updated to latest official version for pep8. pep8 is formally renamed to pycodestyle. -PEP8_VERSION="2.3.1" -PEP8_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pep8-$PEP8_VERSION.py" -PEP8_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$PEP8_VERSION/pycodestyle.py" +PYCODESTYLE_VERSION="2.3.1" +PYCODESTYLE_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pycodestyle-$PYCODESTYLE_VERSION.py" +PYCODESTYLE_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$PYCODESTYLE_VERSION/pycodestyle.py" -if [ ! -e "$PEP8_SCRIPT_PATH" ]; then - curl --silent -o "$PEP8_SCRIPT_PATH" "$PEP8_SCRIPT_REMOTE_PATH" +if [ ! -e "$PYCODESTYLE_SCRIPT_PATH" ]; then + curl --silent -o "$PYCODESTYLE_SCRIPT_PATH" "$PYCODESTYLE_SCRIPT_REMOTE_PATH" curl_status="$?" if [ "$curl_status" -ne 0 ]; then - echo "Failed to download pep8.py from \"$PEP8_SCRIPT_REMOTE_PATH\"." + echo "Failed to download pycodestyle.py from \"$PYCODESTYLE_SCRIPT_REMOTE_PATH\"." exit "$curl_status" fi fi @@ -63,23 +63,23 @@ export "PATH=$PYTHONPATH:$PATH" #+ first, but we do so so that the check status can #+ be output before the report, like with the #+ scalastyle and RAT checks. -python "$PEP8_SCRIPT_PATH" --config=dev/tox.ini $PATHS_TO_CHECK >> "$PEP8_REPORT_PATH" -pep8_status="${PIPESTATUS[0]}" +python "$PYCODESTYLE_SCRIPT_PATH" --config=dev/tox.ini $PATHS_TO_CHECK >> "$PYCODESTYLE_REPORT_PATH" +pycodestyle_status="${PIPESTATUS[0]}" -if [ "$compile_status" -eq 0 -a "$pep8_status" -eq 0 ]; then +if [ "$compile_status" -eq 0 -a "$pycodestyle_status" -eq 0 ]; then lint_status=0 else lint_status=1 fi if [ "$lint_status" -ne 0 ]; then - echo "PEP8 checks failed." - cat "$PEP8_REPORT_PATH" - rm "$PEP8_REPORT_PATH" + echo "PYCODESTYLE checks failed." + cat "$PYCODESTYLE_REPORT_PATH" + rm "$PYCODESTYLE_REPORT_PATH" exit "$lint_status" else - echo "PEP8 checks passed." - rm "$PEP8_REPORT_PATH" + echo "pycodestyle checks passed." + rm "$PYCODESTYLE_REPORT_PATH" fi # Check that the documentation builds acceptably, skip check if sphinx is not installed. From 428fa093a8385eb977535cc7e4848ffe113dc9f3 Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Tue, 23 Jan 2018 21:16:03 -0800 Subject: [PATCH 6/8] [SPARK-11222][Build][Python] Doc style checker --- dev/lint-python | 85 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 18 deletions(-) diff --git a/dev/lint-python b/dev/lint-python index e069cafa1b8c..a3774be52ca9 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -21,7 +21,8 @@ SCRIPT_DIR="$( cd "$( dirname "$0" )" && pwd )" SPARK_ROOT_DIR="$(dirname "$SCRIPT_DIR")" # Exclude auto-generated configuration file. PATHS_TO_CHECK="$( cd "$SPARK_ROOT_DIR" && find . -name "*.py" )" -PYCODESTYLE_REPORT_PATH="$SPARK_ROOT_DIR/dev/pycodestyle-report.txt" +PEP8_REPORT_PATH="$SPARK_ROOT_DIR/dev/pep8-report.txt" +PYDOCSTYLE_REPORT_PATH="$SPARK_ROOT_DIR/dev/pydocstyle-report.txt" PYLINT_REPORT_PATH="$SPARK_ROOT_DIR/dev/pylint-report.txt" PYLINT_INSTALL_INFO="$SPARK_ROOT_DIR/dev/pylint-info.txt" SPHINXBUILD=${SPHINXBUILD:=sphinx-build} @@ -30,22 +31,23 @@ SPHINX_REPORT_PATH="$SPARK_ROOT_DIR/dev/sphinx-report.txt" cd "$SPARK_ROOT_DIR" # compileall: https://docs.python.org/2/library/compileall.html -python -B -m compileall -q -l $PATHS_TO_CHECK > "$PYCODESTYLE_REPORT_PATH" +python -B -m compileall -q -l $PATHS_TO_CHECK > "$PEP8_REPORT_PATH" compile_status="${PIPESTATUS[0]}" -# Get pycodestyle at runtime so that we don't rely on it being installed on the build server. +# Get pep8 at runtime so that we don't rely on it being installed on the build server. #+ See: https://github.com/apache/spark/pull/1744#issuecomment-50982162 -# Updated to latest official version for pep8. pep8 is formally renamed to pycodestyle. -PYCODESTYLE_VERSION="2.3.1" -PYCODESTYLE_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pycodestyle-$PYCODESTYLE_VERSION.py" -PYCODESTYLE_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$PYCODESTYLE_VERSION/pycodestyle.py" +#+ TODOs: +#+ - Download pep8 from PyPI. It's more "official". +PEP8_VERSION="1.7.0" +PEP8_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pep8-$PEP8_VERSION.py" +PEP8_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/jcrocholl/pep8/$PEP8_VERSION/pep8.py" -if [ ! -e "$PYCODESTYLE_SCRIPT_PATH" ]; then - curl --silent -o "$PYCODESTYLE_SCRIPT_PATH" "$PYCODESTYLE_SCRIPT_REMOTE_PATH" +if [ ! -e "$PEP8_SCRIPT_PATH" ]; then + curl --silent -o "$PEP8_SCRIPT_PATH" "$PEP8_SCRIPT_REMOTE_PATH" curl_status="$?" if [ "$curl_status" -ne 0 ]; then - echo "Failed to download pycodestyle.py from \"$PYCODESTYLE_SCRIPT_REMOTE_PATH\"." + echo "Failed to download pep8.py from \"$PEP8_SCRIPT_REMOTE_PATH\"." exit "$curl_status" fi fi @@ -63,25 +65,72 @@ export "PATH=$PYTHONPATH:$PATH" #+ first, but we do so so that the check status can #+ be output before the report, like with the #+ scalastyle and RAT checks. -python "$PYCODESTYLE_SCRIPT_PATH" --config=dev/tox.ini $PATHS_TO_CHECK >> "$PYCODESTYLE_REPORT_PATH" -pycodestyle_status="${PIPESTATUS[0]}" +python "$PEP8_SCRIPT_PATH" --config=dev/tox.ini $PATHS_TO_CHECK >> "$PEP8_REPORT_PATH" +pep8_status="${PIPESTATUS[0]}" -if [ "$compile_status" -eq 0 -a "$pycodestyle_status" -eq 0 ]; then +if [ "$compile_status" -eq 0 -a "$pep8_status" -eq 0 ]; then lint_status=0 else lint_status=1 fi if [ "$lint_status" -ne 0 ]; then - echo "PYCODESTYLE checks failed." - cat "$PYCODESTYLE_REPORT_PATH" - rm "$PYCODESTYLE_REPORT_PATH" + echo "PEP8 checks failed." + cat "$PEP8_REPORT_PATH" + rm "$PEP8_REPORT_PATH" exit "$lint_status" else - echo "pycodestyle checks passed." - rm "$PYCODESTYLE_REPORT_PATH" + echo "PEP8 checks passed." + rm "$PEP8_REPORT_PATH" fi +#### Python Document Style Checks #### + +# Get PYDOCSTYLE at runtime so that we don't rely on it being installed on the build server. +# Using pep257.py which is the single file version of pydocstyle. +PYDOCSTYLE_VERSION="0.2.1" +PYDOCSTYLE_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pydocstyle-$PYDOCSTYLE_VERSION.py" +PYDOCSTYLE_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pydocstyle/$PYDOCSTYLE_VERSION/pep257.py" + +if [ ! -e "$PYDOCSTYLE_SCRIPT_PATH" ]; then + curl --silent -o "$PYDOCSTYLE_SCRIPT_PATH" "$PYDOCSTYLE_SCRIPT_REMOTE_PATH" + curl_status="$?" + + if [ "$curl_status" -ne 0 ]; then + echo "Failed to download pep257.py from \"$PYDOCSTYLE_SCRIPT_REMOTE_PATH\"." + exit "$curl_status" + fi +fi + + +# There is no need to write this output to a file +#+ first, but we do so so that the check status can +#+ be output before the report, like with the +#+ scalastyle and RAT checks. +python "$PYDOCSTYLE_SCRIPT_PATH" $PATHS_TO_CHECK >> "$PYDOCSTYLE_REPORT_PATH" +pydocstyle_status=echo "${PIPESTATUS[@]}" | grep -re 1 + +if [ "$compile_status" -eq 0 -a "$pydocstyle_status" -eq 0 ]; then + lint_status=0 +else + lint_status=1 +fi + +if [ "$lint_status" -ne 0 ]; then + echo "pydocstyle checks failed." + cat "$PYDOCSTYLE_REPORT_PATH" + rm "$PYDOCSTYLE_REPORT_PATH" + # As there are too many doc failures currently, we want to verify but do not want to fail python lint check based on it now. + #exit "$lint_status" +else + echo "pydocstyle checks passed." + rm "$PYDOCSTYLE_REPORT_PATH" +fi + +rm "$PYDOCSTYLE_SCRIPT_PATH" + +#### Python Sphinx Document Style Checks #### + # Check that the documentation builds acceptably, skip check if sphinx is not installed. if hash "$SPHINXBUILD" 2> /dev/null; then cd python/docs From aab86ab987c01388bcda0412162fb3cf50593ea7 Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Tue, 23 Jan 2018 21:18:27 -0800 Subject: [PATCH 7/8] [SPARK-11222][Build][Python] Doc style checker updates --- dev/tox.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/tox.ini b/dev/tox.ini index 583c1eaaa966..eb8b1eb2c288 100644 --- a/dev/tox.ini +++ b/dev/tox.ini @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -[pycodestyle] -ignore=E402,E731,E241,W503,E226,E722,E741,E305 +[pep8] +ignore=E402,E731,E241,W503,E226 max-line-length=100 exclude=cloudpickle.py,heapq3.py,shared.py,python/docs/conf.py,work/*/*.py,python/.eggs/* From e69827e17349196382b24ef65b843d0fbc76cc76 Mon Sep 17 00:00:00 2001 From: rjoshi2 Date: Tue, 23 Jan 2018 21:20:11 -0800 Subject: [PATCH 8/8] [SPARK-11222][Build][Python] Doc style checker updates --- dev/lint-python | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/lint-python b/dev/lint-python index a3774be52ca9..f7c65bf4f6ea 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -120,7 +120,7 @@ if [ "$lint_status" -ne 0 ]; then echo "pydocstyle checks failed." cat "$PYDOCSTYLE_REPORT_PATH" rm "$PYDOCSTYLE_REPORT_PATH" - # As there are too many doc failures currently, we want to verify but do not want to fail python lint check based on it now. + # As there are doc failures currently, we want to verify but do not want to fail python lint check based on it now. #exit "$lint_status" else echo "pydocstyle checks passed."