You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rekhajoshm <gi...@git.apache.org> on 2018/01/24 05:24:30 UTC
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
GitHub user rekhajoshm opened a pull request:
https://github.com/apache/spark/pull/20378
[SPARK-11222][Build][Python] Python document style checker added
## What changes were proposed in this pull request?
Using pydocstyle for python document style checker
https://github.com/PyCQA/pycodestyle/issues/723
## 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-11222-2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20378.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 #20378
----
commit e3677c9fa9697e0d34f9df52442085a6a481c9e9
Author: Rekha Joshi <re...@...>
Date: 2015-05-05T23:10:08Z
Merge pull request #1 from apache/master
Pulling functionality from apache spark
commit 106fd8eee8f6a6f7c67cfc64f57c1161f76d8f75
Author: Rekha Joshi <re...@...>
Date: 2015-05-08T21:49:09Z
Merge pull request #2 from apache/master
pull latest from apache spark
commit 0be142d6becba7c09c6eba0b8ea1efe83d649e8c
Author: Rekha Joshi <re...@...>
Date: 2015-06-22T00:08:08Z
Merge pull request #3 from apache/master
Pulling functionality from apache spark
commit 6c6ee12fd733e3f9902e10faf92ccb78211245e3
Author: Rekha Joshi <re...@...>
Date: 2015-09-17T01:03:09Z
Merge pull request #4 from apache/master
Pulling functionality from apache spark
commit b123c601e459d1ad17511fd91dd304032154882a
Author: Rekha Joshi <re...@...>
Date: 2015-11-25T18:50:32Z
Merge pull request #5 from apache/master
pull request from apache/master
commit c73c32aadd6066e631956923725a48d98a18777e
Author: Rekha Joshi <re...@...>
Date: 2016-03-18T19:13:51Z
Merge pull request #6 from apache/master
pull latest from apache spark
commit 7dbf7320057978526635bed09dabc8cf8657a28a
Author: Rekha Joshi <re...@...>
Date: 2016-04-05T20:26:40Z
Merge pull request #8 from apache/master
pull latest from apache spark
commit 5e9d71827f8e2e4d07027281b80e4e073e7fecd1
Author: Rekha Joshi <re...@...>
Date: 2017-05-01T23:00:30Z
Merge pull request #9 from apache/master
Pull apache spark
commit 63d99b3ce5f222d7126133170a373591f0ac67dd
Author: Rekha Joshi <re...@...>
Date: 2017-09-30T22:26:44Z
Merge pull request #10 from apache/master
pull latest apache spark
commit a7fc787466b71784ff86f9694f617db0f1042da8
Author: Rekha Joshi <re...@...>
Date: 2018-01-21T00:17:58Z
Merge pull request #11 from apache/master
Apache spark pull latest
commit 88734413c5e33803b7d5f8c0f81899ed1d3577ff
Author: rjoshi2 <re...@...>
Date: 2018-01-21T03:40:59Z
[SPARK-11222][BUILD][PYTHON] python code style checker update
commit 871889113057f819337bd24379cf1f07516c3298
Author: rjoshi2 <re...@...>
Date: 2018-01-22T01:36:42Z
updated for review comment
commit c386a9a9b130e3974dc756b0fa89b7cff93f09ac
Author: rjoshi2 <re...@...>
Date: 2018-01-22T02:26:56Z
[SPARK-23174][BUILD][PYTHON] python code style checker update
commit d10fbb49cc6b062bfa7d463f0bae098c77b9a890
Author: rjoshi2 <re...@...>
Date: 2018-01-24T03:17:05Z
updated nit
commit 34a8590b7fa205be3d2b3f4913e252ab0e96d091
Author: rjoshi2 <re...@...>
Date: 2018-01-24T03:27:42Z
updated for review comment
commit 428fa093a8385eb977535cc7e4848ffe113dc9f3
Author: rjoshi2 <re...@...>
Date: 2018-01-24T05:16:03Z
[SPARK-11222][Build][Python] Doc style checker
commit aab86ab987c01388bcda0412162fb3cf50593ea7
Author: rjoshi2 <re...@...>
Date: 2018-01-24T05:18:27Z
[SPARK-11222][Build][Python] Doc style checker updates
commit e69827e17349196382b24ef65b843d0fbc76cc76
Author: rjoshi2 <re...@...>
Date: 2018-01-24T05:20:11Z
[SPARK-11222][Build][Python] Doc style checker updates
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm closed the pull request at:
https://github.com/apache/spark/pull/20378
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on a diff in the pull request:
https://github.com/apache/spark/pull/20378#discussion_r167148657
--- Diff: dev/lint-python ---
@@ -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"
--- End diff --
As called out earlier, this was single file python doc style checker, the latest does not have single file checker that can be included.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on a diff in the pull request:
https://github.com/apache/spark/pull/20378#discussion_r164920796
--- Diff: dev/lint-python ---
@@ -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"
--- End diff --
Just wondering if this is an official channel to get this script from? I see pep8 download has a note above to use PyPI
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
So, seems we got:
```
First line should end with a period. 293
Multiline docstring should end with 1 blank line. 279
Blank line missing after one-line summary. 265
Return value type should be mentioned. 141
All modules should have docstrings. 109
One-liner docstrings should fit on one line with quotes. 91
First line should be in imperative mood ('Do', not 'Does'). 87
Exported definitions should have docstrings. 61
Class docstring should have 1 blank line around them. 35
Use r\"\"\" if any backslashes in your docstrings. 19
The entire docstring should be indented same as code. 6
Exported classes should have docstrings. 1
No blank line before docstring in definitions. 1
```
I think we can take in:
```
First line should end with a period. 293
Multiline docstring should end with 1 blank line. 279
Blank line missing after one-line summary. 265
The entire docstring should be indented same as code. 6
Use \"\"\"triple double quotes\"\"\". 3 # this seems only in heapq3.py where we ignore pep8.
No blank line before docstring in definitions. 1
```
Not sure on:
```
Exported definitions should have docstrings. 61
Exported classes should have docstrings. 1
```
and take out
```
Return value type should be mentioned. 141
All modules should have docstrings. 109
One-liner docstrings should fit on one line with quotes. 91
First line should be in imperative mood ('Do', not 'Does'). 87
Class docstring should have 1 blank line around them. 35
Use r\"\"\" if any backslashes in your docstrings. 19
```
Also, I think we can take out cloudpickle.py, heapq3.py, shared.py, python/docs/conf.py, work/*/*.py, python/.eggs/*` as we do in pep8.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on the issue:
https://github.com/apache/spark/pull/20378
I like this idea, too, but seems like there are too many violating files so we can't enable this for now.
I'm wondering how we can encourage contributors to follow the style, hopefully automatically. Should we make a blacklist for the currently violating files and remove from it when the style is fixed, or something?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on the issue:
https://github.com/apache/spark/pull/20378
@HyukjinKwon I will check it over weekend.thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
Posted by ueshin <gi...@git.apache.org>.
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/20378#discussion_r166525289
--- Diff: dev/lint-python ---
@@ -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"
--- End diff --
Btw, seems like the latest version of pydocstyle is `2.1.1`. Should we use it instead?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
If you happen to be busy on working this one, I can take this when I have some time. That's fine. Either way works.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on the issue:
https://github.com/apache/spark/pull/20378
@HyukjinKwon @holdenk @ueshin @viirya @icexelloss @felixcheung @BryanCutler and @MrBago - This was one of the possible approach that I was running by you. I have proposed another approach at #20556 with features as below -
- Use sphinx like check, run only if pydocstyle installed on machine/jenkins
- use pydocstyle rather than single file pep257.py
- verify pydocstyle latest 2.1.1 is in use, to ensure latest doc checks are getting executed
- ignore (inclusion/exclusion) features and support via tox.ini
- Be non-breaking change and allow updating docstyle to standard at easy pace
Closing this.Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
> One question I have is, do the current violations cause significant document error?
I think this is a good point. Maybe, we could enable ones fixing actual significant problems, at least.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20378
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by icexelloss <gi...@git.apache.org>.
Github user icexelloss commented on the issue:
https://github.com/apache/spark/pull/20378
I share the same concern of backporting. If we decide to do large amounts of format changes. Should we consider backporting the format changes in one batch so future backporting can be easier?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20378
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/170/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
@rekhajoshm, would you mind if I ask to take a quick look for ones causing actual problems and identify them? It might be kind of a grunt job tho.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
Please give me few days .. Let me try to take a close look for this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20378
One question I have is, do the current violations cause significant document error?
Overall this is a good idea. However, is it worth enforcedly applying this if we consider the effort of fixing the violations, backporting difficulty in the future?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on the issue:
https://github.com/apache/spark/pull/20378
Thanks @HyukjinKwon for review.
This checks only for python files, as $PATHS_TO_CHECK is passed, and it takes only python files(find . -name "*.py").Also the single file version on pydocstyle, pep257.py does not allow include/exclude from what I have tried. Sample results attached here.thanks.
[pydocstyle_sample_results.txt](https://github.com/apache/spark/files/1671764/pydocstyle_sample_results.txt)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by BryanCutler <gi...@git.apache.org>.
Github user BryanCutler commented on the issue:
https://github.com/apache/spark/pull/20378
I looked at the example output to see see what the errors were. Specifically looking at `broadcast.__init__`
```
def __init__(self, sc=None, value=None, pickle_registry=None, path=None):
"""
Should not be called directly by users -- use L{SparkContext.broadcast()}
instead.
"""
if sc is not None:
```
it gave
```
./python/pyspark/broadcast.py:68:8: First line should end with a period.
./python/pyspark/broadcast.py:68:8: Blank line missing after one-line summary.
./python/pyspark/broadcast.py:68:8: Multiline docstring should end with 1 blank line.
```
I think a lot of docstrings are purposely formatted this way. What is the format that it is looking for here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20378
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86563/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on the issue:
https://github.com/apache/spark/pull/20378
@HyukjinKwon Identifying docstyle failures does not help much as it is not straightforward to exclude in this version.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on a diff in the pull request:
https://github.com/apache/spark/pull/20378#discussion_r164926603
--- Diff: dev/lint-python ---
@@ -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"
--- End diff --
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 - https://github.com/PyCQA/pycodestyle/issues/723
On the other hand, there is value and we could try to somehow setup latest pydocstyle from git(it would need setup on fly) rather than using single file version.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20378
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
pydocstyle seems claiming PEP 257 - https://www.python.org/dev/peps/pep-0257.
One option given https://github.com/apache/spark/pull/20378#issuecomment-361494109 and https://github.com/apache/spark/pull/20378#issuecomment-361523561 might be to note that we follow PEP 257 in http://spark.apache.org/contributing.html and then enable only ones causing actual problems.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20378
**[Test build #86563 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86563/testReport)** for PR 20378 at commit [`e69827e`](https://github.com/apache/spark/commit/e69827e17349196382b24ef65b843d0fbc76cc76).
* This patch **fails Python style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20378
**[Test build #86563 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86563/testReport)** for PR 20378 at commit [`e69827e`](https://github.com/apache/spark/commit/e69827e17349196382b24ef65b843d0fbc76cc76).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20378
Hey @holdenk, @ueshin, @viirya, @icexelloss, @felixcheung, @BryanCutler and @MrBago. What do you guys think about checking docstring and the list above? I think this could prevent nitpicking and idea itself seems good.
One vague concern is that it might make backporting super hard.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20378: [SPARK-11222][Build][Python] Python document styl...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/20378#discussion_r164672962
--- Diff: dev/run-tests.py ---
@@ -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):
--- End diff --
Can you resolve the conflict? Looks like this change is already merged from #20338.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20378: [SPARK-11222][Build][Python] Python document style check...
Posted by rekhajoshm <gi...@git.apache.org>.
Github user rekhajoshm commented on the issue:
https://github.com/apache/spark/pull/20378
Thanks @HyukjinKwon for your update.
@HyukjinKwon @holdenk @ueshin @viirya @icexelloss @felixcheung @BryanCutler and @MrBago - While you are thinking on it, below is my analysis.
As I understand, there are two things that jira "seems" to be calling out.Please validate.
1. doctest strings must be correctly formatted.
2. Doctest must NOT be included in docs?
Working on it, I found docstring style itself was not enforced at all, and that includes doctest style.
Another aspect seems to be exclude doctest from documentation (_build/html once generated.)
I am not certain on the reasoning behind this exclusion or whether it is indeed what is additionally intended in SPARK-11222.The jira subject and description say two different things, so maybe validate that understanding?
Meanwhile I had a look into/tested different configurations on epytext/sphinx extensions to see if we can achieve surpassing doctests in docs via them.
Played with RULES set in epytext.py. As per epytext manual http://epydoc.sourceforge.net/manual-epytext.html it ensures only correctly formatted doctests are rendered.Which means only if they were incorrectly formatted will they not to appear in docs.This did not seem right and contrary even.
So I played around with sphinx extensions -
'sphinx.ext.doctest',
'sphinx.ext.napoleon'
and in conf.py -
# Napoleon settings
napoleon_google_docstring = True
# Doctest settings
doctest_test_doctest_blocks=''
trim_doctest_flags=True
None of those options tried get me to surpass doctest in docs(_build/html) once the build is done.
Thanks for thinking this over.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org