Skip to content

[SPARK-25962][BUILD][PYTHON] Specify minimum versions for both pydocstyle and flake8 in 'lint-python' script #22963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
HyukjinKwon wants to merge 5 commits into from
Closed

[SPARK-25962][BUILD][PYTHON] Specify minimum versions for both pydocstyle and flake8 in 'lint-python' script #22963

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e597243
Specify minimum versions for both pydocstyle and flake8 in 'lint-pyth…
HyukjinKwon Nov 7, 2018
b7f0e2f
lower to 3.4.0 for Jenkins
HyukjinKwon Nov 7, 2018
a7192a3
3.0.0
HyukjinKwon Nov 7, 2018
bef4190
print out flake8 version
HyukjinKwon Nov 7, 2018
802a9d4
Check directly from binary executbles
HyukjinKwon Nov 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 41 additions & 17 deletions dev/lint-python
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@ PYCODESTYLE_REPORT_PATH="$SPARK_ROOT_DIR/dev/pycodestyle-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"

PYDOCSTYLEBUILD="pydocstyle"
EXPECTED_PYDOCSTYLEVERSION="3.0.0"
PYDOCSTYLEVERSION=$(python -c 'import pkg_resources; print(pkg_resources.get_distribution("pydocstyle").version)' 2> /dev/null)
MINIMUM_PYDOCSTYLEVERSION="3.0.0"

FLAKE8BUILD="flake8"
MINIMUM_FLAKE8="3.5.0"

SPHINXBUILD=${SPHINXBUILD:=sphinx-build}
Copy link
Member Author

@HyukjinKwon HyukjinKwon Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaneknapp and @cloud-fan, it seems flake8 does work but indeed the problem was compatibility with pycodestyle within flake8 itself.

Our script uses pycodestyle 2.4.0 but it's manually downloaded. So, Jenkins's pycodestyle might be different or old and this might be the actual root cause.

I locally tested with latest pycodestyle and flake 3.6.0 and it worked.

Copy link
Member Author

@HyukjinKwon HyukjinKwon Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, i tested flake8 3.0.0, 3.1.0, 3.2.0, 3.3.0, 3.4.0, 3.5.0 and 3.6.0 while I am here.

SPHINX_REPORT_PATH="$SPARK_ROOT_DIR/dev/sphinx-report.txt"

Expand Down Expand Up @@ -87,27 +91,47 @@ else
rm "$PYCODESTYLE_REPORT_PATH"
fi

# stop the build if there are Python syntax errors or undefined names
flake8 . --count --select=E901,E999,F821,F822,F823 --max-line-length=100 --show-source --statistics
Copy link
Member Author

@HyukjinKwon HyukjinKwon Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, it was never a good idea to use external executable program directly without any check.

flake8_status="${PIPESTATUS[0]}"
# Check by flake8
if hash "$FLAKE8BUILD" 2> /dev/null; then
Copy link
Member Author

@HyukjinKwon HyukjinKwon Nov 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I manually tested each branch.

FLAKE8VERSION="$( $FLAKE8BUILD --version 2> /dev/null )"
VERSION=($FLAKE8VERSION)
IS_EXPECTED_FLAKE8=$(python -c 'from distutils.version import LooseVersion; \
print(LooseVersion("""'${VERSION[0]}'""") >= LooseVersion("""'$MINIMUM_FLAKE8'"""))' 2> /dev/null)
if [[ "$IS_EXPECTED_FLAKE8" == "True" ]]; then
# stop the build if there are Python syntax errors or undefined names
$FLAKE8BUILD . --count --select=E901,E999,F821,F822,F823 --max-line-length=100 --show-source --statistics
flake8_status="${PIPESTATUS[0]}"

if [ "$flake8_status" -eq 0 ]; then
lint_status=0
else
lint_status=1
fi

if [ "$flake8_status" -eq 0 ]; then
lint_status=0
if [ "$lint_status" -ne 0 ]; then
echo "flake8 checks failed."
exit "$lint_status"
else
echo "flake8 checks passed."
fi
else
echo "The flake8 version needs to be "$MINIMUM_FLAKE8" at latest. Your current version is '"$FLAKE8VERSION"'."
echo "flake8 checks failed."
exit 1
fi
else
lint_status=1
fi

if [ "$lint_status" -ne 0 ]; then
echo >&2 "The flake8 command was not found."
echo "flake8 checks failed."
exit "$lint_status"
else
echo "flake8 checks passed."
exit 1
fi

# Check python document style, skip check if pydocstyle is not installed.
if hash "$PYDOCSTYLEBUILD" 2> /dev/null; then
if [[ "$PYDOCSTYLEVERSION" == "$EXPECTED_PYDOCSTYLEVERSION" ]]; then
pydocstyle --config=dev/tox.ini $DOC_PATHS_TO_CHECK >> "$PYDOCSTYLE_REPORT_PATH"
PYDOCSTYLEVERSION="$( $PYDOCSTYLEBUILD --version 2> /dev/null )"
IS_EXPECTED_PYDOCSTYLEVERSION=$(python -c 'from distutils.version import LooseVersion; \
print(LooseVersion("""'$PYDOCSTYLEVERSION'""") >= LooseVersion("""'$MINIMUM_PYDOCSTYLEVERSION'"""))')
if [[ "$IS_EXPECTED_PYDOCSTYLEVERSION" == "True" ]]; then
$PYDOCSTYLEBUILD --config=dev/tox.ini $DOC_PATHS_TO_CHECK >> "$PYDOCSTYLE_REPORT_PATH"
pydocstyle_status="${PIPESTATUS[0]}"

if [ "$compile_status" -eq 0 -a "$pydocstyle_status" -eq 0 ]; then
Expand All @@ -121,7 +145,7 @@ if hash "$PYDOCSTYLEBUILD" 2> /dev/null; then
fi

else
echo "The pydocstyle version needs to be latest 3.0.0. Skipping pydoc checks for now"
echo "The pydocstyle version needs to be "$MINIMUM_PYDOCSTYLEVERSION" at latest. Your current version is "$PYDOCSTYLEVERSION". Skipping pydoc checks for now."
fi
else
echo >&2 "The pydocstyle command was not found. Skipping pydoc checks for now"
Expand Down