Skip to content

[SPARK-23367][Build] Include python document style checking #20556

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
rekhajoshm wants to merge 15 commits into from
Closed

[SPARK-23367][Build] Include python document style checking #20556

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e3677c9
Merge pull request #1 from apache/master
rekhajoshm May 5, 2015
106fd8e
Merge pull request #2 from apache/master
rekhajoshm May 8, 2015
0be142d
Merge pull request #3 from apache/master
rekhajoshm Jun 22, 2015
6c6ee12
Merge pull request #4 from apache/master
rekhajoshm Sep 17, 2015
b123c60
Merge pull request #5 from apache/master
rekhajoshm Nov 25, 2015
c73c32a
Merge pull request #6 from apache/master
rekhajoshm Mar 18, 2016
7dbf732
Merge pull request #8 from apache/master
rekhajoshm Apr 5, 2016
5e9d718
Merge pull request #9 from apache/master
rekhajoshm May 1, 2017
63d99b3
Merge pull request #10 from apache/master
rekhajoshm Sep 30, 2017
a7fc787
Merge pull request #11 from apache/master
rekhajoshm Jan 21, 2018
3a2d453
Merge pull request #12 from apache/master
rekhajoshm Feb 9, 2018
85ca69d
[SPARK-23367][Build] Include python document style checking
rekhajoshm Feb 9, 2018
ee14cf7
[SPARK-23367][Build] Include python document style checking
rekhajoshm Feb 9, 2018
11ad2c1
updated for review comments
rekhajoshm Feb 27, 2018
5f6b409
updated for review comments
rekhajoshm Sep 15, 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
33 changes: 33 additions & 0 deletions dev/lint-python
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@ 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" )"
DOC_PATHS_TO_CHECK="$( cd "$SPARK_ROOT_DIR" && find . -name "*.py" | grep -vF 'functions.py')"

@ueshin ueshin Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: add an extra space between 'functions.py' and ).

@rekhajoshm rekhajoshm Feb 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

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"
PYDOCSTYLEVERSION=$(python -c 'import pkg_resources; print pkg_resources.get_distribution("pydocstyle").version')

@ueshin ueshin Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

..; print(pkg_resources.get_distribution("pydocstyle").version)' 2> /dev/null)

@rekhajoshm rekhajoshm Feb 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

SPHINXBUILD=${SPHINXBUILD:=sphinx-build}

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

cd "$SPARK_ROOT_DIR"
Expand Down Expand Up @@ -82,6 +87,34 @@ else
rm "$PYCODESTYLE_REPORT_PATH"
fi

# Check python document style, skip check if pydocstyle is not installed.
if hash "$PYDOCSTYLEBUILD" 2> /dev/null; then
if [ $PYDOCSTYLEVERSION == "2.1.1" ]; then

@BryanCutler BryanCutler Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens when a new version of pydocstyle is released? Would the same rules below still work so that this check could be >= 2.1.1?

@rekhajoshm rekhajoshm Feb 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

pydocstyle --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
lint_status=0
else
lint_status=1
fi

if [ "$lint_status" -ne 0 ]; then

@BryanCutler BryanCutler Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could you just move this block above and get rid of lint_status?

@rekhajoshm rekhajoshm Feb 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure i understand

@holdenk holdenk Jul 27, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So if I understand @BryanCutler's request correctly, instead of setting linte_status in the if check above and then lint_status to determine if we should run the pydocstyle checker, just directly check the compile status and then run the pydocstyle checker on that.

That being said I think we could also drop the compile_status check since we would have already exited the script if it was non-zero at this point (I think).

echo "pydocstyle checks failed."
cat "$PYDOCSTYLE_REPORT_PATH"
rm "$PYDOCSTYLE_REPORT_PATH"
exit "$lint_status"
else
echo "pydocstyle checks passed."
rm "$PYDOCSTYLE_REPORT_PATH"
fi
else
echo "The pydocstyle version needs to be latest 2.1.1"
fi
else
echo >&2 "The pydocstyle command was not found. Skipping pydoc checks for now"
fi

# Check that the documentation builds acceptably, skip check if sphinx is not installed.
if hash "$SPHINXBUILD" 2> /dev/null; then
cd python/docs
Expand Down
2 changes: 2 additions & 0 deletions dev/tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@
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/*
[pydocstyle]
ignore=D100,D101,D102,D103,D104,D105,D106,D107,D200,D201,D202,D203,D204,D205,D206,D207,D208,D209,D210,D211,D212,D213,D214,D215,D300,D301,D302,D400,D401,D402,D403,D404,D405,D406,D407,D408,D409,D410,D411,D412,D413,D414

@ueshin ueshin Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need a line break at the end of the file.

@rekhajoshm rekhajoshm Feb 27, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done