[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

Github user rekhajoshm closed the pull request at:

https://github.com/apache/spark/pull/20556


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[holdenk]

Github user holdenk commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r205862690
  
--- Diff: dev/lint-python ---
@@ -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
+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
--- End diff --

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).


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[holdenk]

Github user holdenk commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r205862781
  
--- Diff: dev/lint-python ---
@@ -82,6 +88,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">="$EXPECTED_PYDOCSTYLEVERSION" ]; then
+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
+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.Skipping 
pydoc checks for now"
--- End diff --

nit: add a space between "2.1.1.Skipping" the final dot and skipping.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

Github user rekhajoshm commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r170792111
  
--- Diff: dev/lint-python ---
@@ -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
+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
--- End diff --

not sure i understand


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

Github user rekhajoshm commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r170792037
  
--- Diff: dev/tox.ini ---
@@ -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
--- End diff --

done


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

Github user rekhajoshm commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r170792051
  
--- Diff: dev/lint-python ---
@@ -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
--- End diff --

done


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

Github user rekhajoshm commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r170792029
  
--- Diff: dev/lint-python ---
@@ -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')"
 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')
--- End diff --

done


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

Github user rekhajoshm commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r170792015
  
--- Diff: dev/lint-python ---
@@ -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')"
--- End diff --

done


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[BryanCutler]

Github user BryanCutler commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r168263131
  
--- Diff: dev/lint-python ---
@@ -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
--- End diff --

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`?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[BryanCutler]

Github user BryanCutler commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r168263151
  
--- Diff: dev/lint-python ---
@@ -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
+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
--- End diff --

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


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[ueshin]

Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r168079061
  
--- Diff: dev/tox.ini ---
@@ -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
--- End diff --

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


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[ueshin]

Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r168078916
  
--- Diff: dev/lint-python ---
@@ -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')"
--- End diff --

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


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[ueshin]

Github user ueshin commented on a diff in the pull request:

https://github.com/apache/spark/pull/20556#discussion_r168078994
  
--- Diff: dev/lint-python ---
@@ -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')"
 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')
--- End diff --

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


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request #20556: [SPARK-23367][Build] Include python document styl...

[rekhajoshm]

GitHub user rekhajoshm opened a pull request:

https://github.com/apache/spark/pull/20556

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

## What changes were proposed in this pull request?
Include python document style checking.
This PR includes the pydocstyle checking if pydocstyle is installed, 
similar to sphinx checking.It takes care of exclusion/inclusion of explicit 
document error code via tox.ini. Currently all error codes are ignored to be a 
non-breaking change.

## How was this patch tested?
./dev/run-tests


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/rekhajoshm/spark SPARK-23367

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/20556.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #20556


commit e3677c9fa9697e0d34f9df52442085a6a481c9e9
Author: Rekha Joshi 
Date:   2015-05-05T23:10:08Z

Merge pull request #1 from apache/master

Pulling functionality from apache spark

commit 106fd8eee8f6a6f7c67cfc64f57c1161f76d8f75
Author: Rekha Joshi 
Date:   2015-05-08T21:49:09Z

Merge pull request #2 from apache/master

pull latest from apache spark

commit 0be142d6becba7c09c6eba0b8ea1efe83d649e8c
Author: Rekha Joshi 
Date:   2015-06-22T00:08:08Z

Merge pull request #3 from apache/master

Pulling functionality from apache spark

commit 6c6ee12fd733e3f9902e10faf92ccb78211245e3
Author: Rekha Joshi 
Date:   2015-09-17T01:03:09Z

Merge pull request #4 from apache/master

Pulling functionality from apache spark

commit b123c601e459d1ad17511fd91dd304032154882a
Author: Rekha Joshi 
Date:   2015-11-25T18:50:32Z

Merge pull request #5 from apache/master

pull request from apache/master

commit c73c32aadd6066e631956923725a48d98a18777e
Author: Rekha Joshi 
Date:   2016-03-18T19:13:51Z

Merge pull request #6 from apache/master

pull latest from apache spark

commit 7dbf7320057978526635bed09dabc8cf8657a28a
Author: Rekha Joshi 
Date:   2016-04-05T20:26:40Z

Merge pull request #8 from apache/master

pull latest from apache spark

commit 5e9d71827f8e2e4d07027281b80e4e073e7fecd1
Author: Rekha Joshi 
Date:   2017-05-01T23:00:30Z

Merge pull request #9 from apache/master

Pull apache spark

commit 63d99b3ce5f222d7126133170a373591f0ac67dd
Author: Rekha Joshi 
Date:   2017-09-30T22:26:44Z

Merge pull request #10 from apache/master

pull latest apache spark

commit a7fc787466b71784ff86f9694f617db0f1042da8
Author: Rekha Joshi 
Date:   2018-01-21T00:17:58Z

Merge pull request #11 from apache/master

Apache spark pull latest

commit 3a2d45377ed4397de802badd764bc2588cfd275b
Author: Rekha Joshi 
Date:   2018-02-09T04:55:12Z

Merge pull request #12 from apache/master

Apache spark latest pull

commit 85ca69de956cd3255eee5c51e830b9aa8f451308
Author: rjoshi2 
Date:   2018-02-09T05:54:03Z

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




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org