-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-11222][Build][Python] Python document style checker added #20378
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.
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
Changes from all commits
e3677c9
106fd8e
0be142d
6c6ee12
b123c60
c73c32a
7dbf732
5e9d718
63d99b3
a7fc787
8873441
8718891
c386a9a
d10fbb4
34a8590
428fa09
aab86ab
e69827e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ 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" | ||
| 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} | ||
|
|
@@ -42,7 +43,7 @@ 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 "$PEP8_SCRIPT_PATH" ]; then | ||
| curl --silent -o "$PEP8_SCRIPT_PATH" "$PEP8_SCRIPT_REMOTE_PATH" | ||
| curl --silent -o "$PEP8_SCRIPT_PATH" "$PEP8_SCRIPT_REMOTE_PATH" | ||
| curl_status="$?" | ||
|
|
||
| if [ "$curl_status" -ne 0 ]; then | ||
|
|
@@ -83,6 +84,53 @@ else | |
| 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" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just wondering if this is an official channel to get this script from? I see pep8 download has a note above to use PyPI
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @BryanCutler for your input. pep8 is replaced by pycodestyle from official PyPi channel in PR #20338 .This is for doc style alone, which is not maintained within pycodestyle as per maintainers of the project - PyCQA/pycodestyle#723 |
||
|
|
||
| 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 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -576,7 +576,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): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you resolve the conflict? Looks like this change is already merged from #20338. |
||
| run_python_style_checks() | ||
| if not changed_files or any(f.endswith(".R") for f in changed_files): | ||
| run_sparkr_style_checks() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, seems like the latest version of pydocstyle is
2.1.1. Should we use it instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As called out earlier, this was single file python doc style checker, the latest does not have single file checker that can be included.