[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/14567 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r77177492 --- Diff: .gitignore --- @@ -28,6 +28,7 @@ build/*.jar build/apache-maven* build/scala* build/zinc* +build/venv --- End diff -- I am used to put sphinx pep8, ... etc inside a virtual env here --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76968414 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- - tox.ini kind of becames this pep8rc, indeed. - isort only deals with sorting "import" statement, not checked by pep8, but pylint has some checks on it - style.yapf.ini is the configuration for yapf, which is a more aggressive version of 'autopep8' made by google for their chrominum projects, to strictly format all their python files. I don't recommend to enforce the usage of it, but if one want to use it on its python file in the Spark project, he can execute it and review the style before commiting. In an ideal wold, this would be a mandatory formatting (on post commit hook for example), but this may take some times before reaching this point - .editorconfig only deals with tab space, like I said, might be put outside of this PR if you want --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76967915 --- Diff: python/.editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +root = true --- End diff -- This file is to automatically configure editors that have the "editorconfig" plugin to configure 2 spaces tab for scala files and 4 spaces tab on Python files. It is not related to pep8, so can be submitted in another PR if you prefer. That's why I would prefer to be placed on the root of the project (and not the root of the python folder) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76967736 --- Diff: dev/py-validate.sh --- @@ -0,0 +1,110 @@ +#!/usr/bin/env bash --- End diff -- My point of view: - don't enforce it right away, just agree on the style - execute manualy on some files (in my next PR, it's on documentation), and on small big bang PRs - then enforce automatic formatting (preferably with Yapf) on some files. On Buildbot we use a git commit hook to check python files automatically, apply some pep8 formatting, execute pylint,... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76967333 --- Diff: dev/isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- yes it still works --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76961681 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- I'm ignorant of the config here of course, but it seems like we've replaced 1 config file (tox.ini) with 4 in total (pep8rc, isort.cfg, style.yapf.ini, .editorconfig). Maybe it would help to explain what the differences are, because maybe only some of them are really worth dealing with. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76961521 --- Diff: python/.editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +root = true --- End diff -- OK, just confirming that there are 2 additional config files needed to fully specify the style rules to enforce? what do these additional configs support beyond PEP8? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76961377 --- Diff: dev/py-validate.sh --- @@ -0,0 +1,110 @@ +#!/usr/bin/env bash --- End diff -- @davies what do you think about including this vs just enforcing the checks? If it would be used, I'm for it, but not clear how often this would be run in practice by anyone if we're not going to use its big-bang changes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76961264 --- Diff: dev/isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- Hm, really, it guesses at moves? OK, if the net effect is that `tox.ini` has been deleted, OK. If you wouldn't mind, if you make another pass here, just add back those spaces. They seem to be present in the 'canonical' text of the license and best to just avoid any minor deviation regardless of where it came from. So the `lint-python` script still works with this I take it because I see you updated it, and that's the important thing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76582188 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- I don't know if they can be merged. Will try it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76580634 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- Maybe I'm missing something but should they be in the same file or are they separate? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76577358 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- actually, tox.ini looks more similar to this pep8rc than isort.cfg, but github doesn't show it that way. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76577137 --- Diff: dev/isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- No, this is not pep8 related! I deleted dev/tox.ini and created a new file dev/isort.cfg from scratch. But the fact is git "finds" file renames based on content similarities, so it sees this as a file rename. Actually, I did deleted the tox.ini in another commit than the commit that creates isort.ini (see https://github.com/apache/spark/pull/14567/commits), so I guess this "visualisation" of a file renaming comes from github --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76574930 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- Is the rename of tox.ini addressing this -- I wasn't sure how --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76574875 --- Diff: dev/isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- Also could you comment on renaming tox.ini, I wasn't sure was requiring that --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76574749 --- Diff: dev/isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- Does PEP8 really pick this up as a violation? what's the problem? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76400303 --- Diff: python/pyspark/heapq3.py --- @@ -408,10 +408,12 @@ __all__ = ['heappush', 'heappop', 'heapify', 'heapreplace', 'merge', 'nlargest', 'nsmallest', 'heappushpop'] + --- End diff -- Done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76394998 --- Diff: .editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more --- End diff -- Meaning, do we need to fix up the style on other Python code to match what this pep8 config would enforce? yes ideally so. Much of the code in the docs are actually broken out as source files that are inlined, so hopefully they can be tested directly by pep8. For stuff that isn't otherwise tested, yes, making it match the enforced style is worthwhile. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76394439 --- Diff: .editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more --- End diff -- yes of course Question: do you want to apply this fix on other python scripts (for example, the python code from the documentation) ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76393422 --- Diff: python/pyspark/cloudpickle.py --- @@ -280,7 +279,7 @@ def extract_code_globals(co): # see if nested function have any global refs if co.co_consts: for const in co.co_consts: -if type(const) is types.CodeType: +if isinstance(const, types.CodeType): --- End diff -- this file is now ignored actually --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76393105 --- Diff: .editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more --- End diff -- Would this still work under python/ because that's where Python-related scripts would be run from? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76392662 --- Diff: .isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- With latest version it seenms I can put it in the ```dev``` folder --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76391648 --- Diff: .editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more --- End diff -- Should be at the project root, for editors to find it. The spec of all plugin is set that way: plugins will search from the opened file and go to parent dirs until it find a .editorconfig. See spec at http://editorconfig.org/ Most plugins does not support specifying an hardcoded path (like ```./dev```). For example the sublime editorconfig does not support it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76391001 --- Diff: python/run-tests.py --- @@ -37,9 +37,9 @@ sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), "../dev/")) -from sparktestsupport import SPARK_HOME # noqa (suppress pep8 warnings) -from sparktestsupport.shellutils import which, subprocess_check_output # noqa -from sparktestsupport.modules import all_modules # noqa +from sparktestsupport import SPARK_HOME # noqa (suppress pep8 warnings) isort:skip --- End diff -- it prevent these lines to be automatically moved on top of the sys.path.append. For me this is weird to have to modify the sys.path, probably because pyspark unit test are not executed inside a virtualenv --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76388621 --- Diff: .editorconfig --- @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more --- End diff -- Same, would not want to put tis in the project root. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76388567 --- Diff: .isort.cfg --- @@ -1,9 +1,9 @@ # Licensed to the Apache Software Foundation (ASF) under one or more -# contributor license agreements. See the NOTICE file distributed with +# contributor license agreements. See the NOTICE file distributed with --- End diff -- I don't think we want to 'fix' the standard license header necessarily unless it's really hard to exclude. Is this failing PEP8? Also does this now put a file at the project root? I spent time getting stuff out of the root and would prefer to keep everything under python/ --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76388360 --- Diff: python/pyspark/cloudpickle.py --- @@ -280,7 +279,7 @@ def extract_code_globals(co): # see if nested function have any global refs if co.co_consts: for const in co.co_consts: -if type(const) is types.CodeType: +if isinstance(const, types.CodeType): --- End diff -- Hm, i think this is third-party code. I'm reluctant to change it because it's just a straight copy of the file from a particular release. Can we exclude it from pep8? You're welcome of course to file a bug against cloudpickle. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r76388158 --- Diff: python/run-tests.py --- @@ -37,9 +37,9 @@ sys.path.append(os.path.join(os.path.dirname(os.path.realpath(__file__)), "../dev/")) -from sparktestsupport import SPARK_HOME # noqa (suppress pep8 warnings) -from sparktestsupport.shellutils import which, subprocess_check_output # noqa -from sparktestsupport.modules import all_modules # noqa +from sparktestsupport import SPARK_HOME # noqa (suppress pep8 warnings) isort:skip --- End diff -- For my info, what does isort:skip do? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75642520 --- Diff: python/pyspark/cloudpickle.py --- @@ -280,7 +279,7 @@ def extract_code_globals(co): # see if nested function have any global refs if co.co_consts: for const in co.co_consts: -if type(const) is types.CodeType: +if isinstance(const, types.CodeType): --- End diff -- this is quite a big error, is it enough to fix it here? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75561821 --- Diff: python/pyspark/ml/param/shared.py --- @@ -25,7 +25,8 @@ class HasMaxIter(Params): Mixin for param maxIter: max number of iterations (>= 0). --- End diff -- i got error in jenkins if I don't modify it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75561735 --- Diff: dev/py-validate.sh --- @@ -0,0 +1,110 @@ +#!/usr/bin/env bash --- End diff -- it is the script that format all python code automatically --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user davies commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75553470 --- Diff: dev/py-validate.sh --- @@ -0,0 +1,110 @@ +#!/usr/bin/env bash --- End diff -- Where is this file used? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user davies commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75553298 --- Diff: python/pyspark/cloudpickle.py --- @@ -42,17 +42,17 @@ """ --- End diff -- This file is copied, please ignore it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user davies commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75553255 --- Diff: python/pyspark/heapq3.py --- @@ -408,10 +408,12 @@ __all__ = ['heappush', 'heappop', 'heapify', 'heapreplace', 'merge', 'nlargest', 'nsmallest', 'heappushpop'] + --- End diff -- This file is copied, please ignore it --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user davies commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r75553180 --- Diff: python/pyspark/ml/param/shared.py --- @@ -25,7 +25,8 @@ class HasMaxIter(Params): Mixin for param maxIter: max number of iterations (>= 0). --- End diff -- This file is generated by _shared_params_code_gen.py., can we ignore it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r74615638 --- Diff: python/pyspark/ml/param/shared.py --- @@ -25,7 +25,8 @@ class HasMaxIter(Params): Mixin for param maxIter: max number of iterations (>= 0). --- End diff -- isort/pep8 executed on this file to make jenkins happy --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r74615609 --- Diff: python/pyspark/cloudpickle.py --- @@ -42,17 +42,17 @@ """ --- End diff -- isort/pep8 executed on this file to make jenkins happy --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r74590863 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user Stibbons commented on a diff in the pull request:
https://github.com/apache/spark/pull/14567#discussion_r74590160
--- Diff: python/pyspark/cloudpickle.py ---
@@ -194,7 +194,7 @@ def save_function(self, obj, name=None):
# we'll pickle the actual function object rather than simply
saving a
# reference (as is done in default pickler), via
save_function_tuple.
if islambda(obj) or obj.__code__.co_filename == '' or
themodule is None:
-#print("save global", islambda(obj), obj.__code__.co_filename,
modname, themodule)
+# print("save global", islambda(obj),
obj.__code__.co_filename, modname, themodule)
--- End diff --
here are the list of ignored files in tox.ini:
cloudpickle.py,heapq3.py,shared.py
Look like it would be better to style them anyway. Could I remove them from
the exclusion list?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/14567#discussion_r74362668
--- Diff: python/pyspark/cloudpickle.py ---
@@ -194,7 +194,7 @@ def save_function(self, obj, name=None):
# we'll pickle the actual function object rather than simply
saving a
# reference (as is done in default pickler), via
save_function_tuple.
if islambda(obj) or obj.__code__.co_filename == '' or
themodule is None:
-#print("save global", islambda(obj), obj.__code__.co_filename,
modname, themodule)
+# print("save global", islambda(obj),
obj.__code__.co_filename, modname, themodule)
--- End diff --
Seems like we might just want to remove this commented out line?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14567: [SPARK-16992][PYSPARK] Python Pep8 formatting and...
Github user holdenk commented on a diff in the pull request: https://github.com/apache/spark/pull/14567#discussion_r74362636 --- Diff: python/pep8rc --- @@ -0,0 +1,21 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +[pep8] --- End diff -- There is another pep8config file at ./dev/toxi.ini - seems like it would be good to have a single file (also unify the ignore lists) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
