Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented May 31, 2017

What changes were proposed in this pull request?

Currently, if we run ./python/run-tests.py and they are aborted without cleaning up this directory, it fails pep8 check due to some Python scripts generated. For example,

spark/python/pyspark/tests.py

Lines 1955 to 1968 in 7387126

"""Submit and test a script with a dependency on another module"""
script = self.createTempFile("test.py", """
|from pyspark import SparkContext
|from mylib import myfunc
|
|sc = SparkContext()
|print(sc.parallelize([1, 2, 3]).map(myfunc).collect())
""")
zip = self.createFileInZip("mylib.py", """
|def myfunc(x):
| return x + 1
""")
proc = subprocess.Popen([self.sparkSubmit, "--py-files", zip, script],
stdout=subprocess.PIPE)

PEP8 checks failed.
./work/app-20170531190857-0000/0/test.py:5:55: W292 no newline at end of file
./work/app-20170531190909-0000/0/test.py:5:55: W292 no newline at end of file
./work/app-20170531190924-0000/0/test.py:3:1: E302 expected 2 blank lines, found 1
./work/app-20170531190924-0000/0/test.py:7:52: W292 no newline at end of file
./work/app-20170531191016-0000/0/test.py:5:55: W292 no newline at end of file
./work/app-20170531191030-0000/0/test.py:5:55: W292 no newline at end of file
./work/app-20170531191045-0000/0/test.py:3:1: E302 expected 2 blank lines, found 1
./work/app-20170531191045-0000/0/test.py:7:52: W292 no newline at end of file

For me, it is sometimes a bit annoying. This PR proposes to exclude these (assuming we want to skip per https://github.com/apache/spark/blob/master/.gitignore#L73).

Also, it moves other pep8 configurations in the script into ini configuration file in pep8.

How was this patch tested?

Manually tested via ./dev/lint-python.

@HyukjinKwon
Copy link
Member Author

@srowen and @holdenk, could you take a look please? I checked related codes were updated by both of you before (and me as well).

@HyukjinKwon HyukjinKwon changed the title [MINOR][PYTHON] Ignore pep8 on test scripts in work directory generated in tests [MINOR][PYTHON] Ignore pep8 on test scripts generated in tests in work directory May 31, 2017
@holdenk
Copy link
Contributor

holdenk commented May 31, 2017

LGTM

@SparkQA
Copy link

SparkQA commented May 31, 2017

Test build #77587 has finished for PR 18161 at commit 968d9e7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

+1, LGTM.

@HyukjinKwon
Copy link
Member Author

Thank you @srowen, @holdenk and @dongjoon-hyun .

@srowen
Copy link
Member

srowen commented Jun 2, 2017

Merged to master

@asfgit asfgit closed this in 0e31e28 Jun 2, 2017
@HyukjinKwon HyukjinKwon deleted the work-exclude-pep8 branch January 2, 2018 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants