[SPARK-32435][PYTHON] Remove heapq3 port from Python 3 #29229

HyukjinKwon · 2020-07-25T06:13:03Z

What changes were proposed in this pull request?

This PR removes the manual port of heapq3.py introduced from SPARK-3073. The main reason of this was to support Python 2.6 and 2.7 because Python 2's heapq.merge() doesn't not support key and reverse.

See

https://docs.python.org/2/library/heapq.html#heapq.merge in Python 2
https://docs.python.org/3.8/library/heapq.html#heapq.merge in Python 3

Since we dropped the Python 2 at SPARK-32138, we can remove this away.

Why are the changes needed?

To remove unnecessary codes. Also, we can leverage bug fixes made in Python 3.x at heapq.

Does this PR introduce any user-facing change?

No, dev-only.

How was this patch tested?

Existing tests should cover. I locally ran and verified:

./python/run-tests --python-executable=python3 --testname="pyspark.tests.test_shuffle"
./python/run-tests --python-executable=python3 --testname="pyspark.shuffle ExternalSorter"
./python/run-tests --python-executable=python3 --testname="pyspark.tests.test_rdd RDDTests.test_external_group_by_key"

HyukjinKwon · 2020-07-25T06:13:27Z

@srowen, @JoshRosen @viirya can you take a look when you're available please?

HyukjinKwon · 2020-07-25T06:17:02Z

python/pyspark/shuffle.py

@@ -498,7 +498,7 @@ def load(f):
        if current_chunk:
            chunks.append(iter(current_chunk))

-        return heapq.merge(chunks, key=key, reverse=reverse)
+        return heapq.merge(*chunks, key=key, reverse=reverse)


There was a bit of modification when this file was ported from Python 3 because heapq has to be able to compile with Python 2 as well. The diffs made are:

< def merge(iterables, key=None, reverse=False): --- > def merge(*iterables, key=None, reverse=False): 216c218,219 < h_append([next(it), order * direction, it]) --- > next = it.__next__ > h_append([next(), order * direction, next]) 223c226 < value, order, it = s = h[0] --- > value, order, next = s = h[0] 225c228 < s[0] = next(it) # raises StopIteration when exhausted --- > s[0] = next() # raises StopIteration when exhausted 231c234 < value, order, it = h[0] --- > value, order, next = h[0] 233,234c236 < for value in it: < yield value --- > yield from next.__self__ 239,240c241,243 < value = next(it) < h_append([key(value), order * direction, value, it]) --- > next = it.__next__ > value = next() > h_append([key(value), order * direction, value, next]) 247c250 < key_value, order, value, it = s = h[0] --- > key_value, order, value, next = s = h[0] 249c252 < value = next(it) --- > value = next() 256c259 < key_value, order, value, it = h[0] --- > key_value, order, value, next = h[0] 258,259c261 < for value in it: < yield value --- > yield from next.__self__

These differences don't look affecting any behaviours.

I think it was ported from Python 3.5: https://github.com/python/cpython/blob/3.5/Lib/heapq.py

viirya

LGTM. Thanks for doing this.

viirya · 2020-07-25T06:35:33Z

python/pyspark/shuffle.py

@@ -25,7 +25,7 @@
 import random
 import sys

-import pyspark.heapq3 as heapq
+import heapq


nit: put it with other python built-in imports above?

SparkQA · 2020-07-25T07:05:01Z

Test build #126529 has finished for PR 29229 at commit d15a556.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

dongjoon-hyun · 2020-07-25T08:12:27Z

Retest this please.

SparkQA · 2020-07-25T10:13:37Z

Test build #126537 has finished for PR 29229 at commit d15a556.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-07-25T10:30:50Z

Thank you guys. Let me take a look for the test failure tomorrow. Should be trivial to handle I believe.

HyukjinKwon · 2020-07-27T05:08:21Z

python/pyspark/shuffle.py

@@ -796,7 +796,7 @@ def load_partition(j):

        if self._sorted:
            # all the partitions are already sorted
-            sorted_items = heapq.merge(disk_items, key=operator.itemgetter(0))
+            sorted_items = heapq.merge(*disk_items, key=operator.itemgetter(0))


I missed one place here which caused the test failure. I double checked this is the last one I missed.

SparkQA · 2020-07-27T07:05:02Z

Test build #126612 has finished for PR 29229 at commit d6ac35d.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2020-07-27T07:39:05Z

retest this please

HyukjinKwon · 2020-07-27T08:53:19Z

Simply rebased to resolve conflicts.

SparkQA · 2020-07-27T10:31:11Z

Test build #126625 has finished for PR 29229 at commit d6ac35d.

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

HyukjinKwon · 2020-07-27T11:09:57Z

Merged to master.

Thanks @dongjoon-hyun and @viirya

SparkQA · 2020-07-27T12:09:06Z

Test build #126631 has finished for PR 29229 at commit 007c2a7.

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

HyukjinKwon force-pushed the SPARK-32435 branch from 64bb101 to d15a556 Compare July 25, 2020 06:13

probot-autolabeler bot added BUILD CORE PYTHON labels Jul 25, 2020

HyukjinKwon commented Jul 25, 2020

View reviewed changes

viirya approved these changes Jul 25, 2020

View reviewed changes

viirya reviewed Jul 25, 2020

View reviewed changes

dongjoon-hyun approved these changes Jul 25, 2020

View reviewed changes

HyukjinKwon commented Jul 27, 2020

View reviewed changes

dongjoon-hyun approved these changes Jul 27, 2020

View reviewed changes

HyukjinKwon closed this Jul 27, 2020

HyukjinKwon deleted the SPARK-32435 branch July 27, 2020 07:42

HyukjinKwon restored the SPARK-32435 branch July 27, 2020 07:45

HyukjinKwon reopened this Jul 27, 2020

HyukjinKwon added 2 commits July 27, 2020 17:52

Remove heapq3 port from Python 3

1cbfc9e

One missed place

007c2a7

HyukjinKwon force-pushed the SPARK-32435 branch from d6ac35d to 007c2a7 Compare July 27, 2020 08:52

HyukjinKwon closed this in a82aee0 Jul 27, 2020

Fokko mentioned this pull request Aug 3, 2020

[SPARK-32435] Remove heapq3 port from Python 3 zero323/pyspark-stubs#459

Merged

HyukjinKwon deleted the SPARK-32435 branch December 7, 2020 02:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-32435][PYTHON] Remove heapq3 port from Python 3 #29229

[SPARK-32435][PYTHON] Remove heapq3 port from Python 3 #29229

HyukjinKwon commented Jul 25, 2020 •

edited

HyukjinKwon commented Jul 25, 2020

HyukjinKwon Jul 25, 2020

viirya left a comment

viirya Jul 25, 2020

SparkQA commented Jul 25, 2020

dongjoon-hyun commented Jul 25, 2020

SparkQA commented Jul 25, 2020

HyukjinKwon commented Jul 25, 2020

HyukjinKwon Jul 27, 2020

SparkQA commented Jul 27, 2020

HyukjinKwon commented Jul 27, 2020

HyukjinKwon commented Jul 27, 2020

SparkQA commented Jul 27, 2020

HyukjinKwon commented Jul 27, 2020

SparkQA commented Jul 27, 2020

[SPARK-32435][PYTHON] Remove heapq3 port from Python 3 #29229

[SPARK-32435][PYTHON] Remove heapq3 port from Python 3 #29229

Conversation

HyukjinKwon commented Jul 25, 2020 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

HyukjinKwon commented Jul 25, 2020

HyukjinKwon Jul 25, 2020

Choose a reason for hiding this comment

viirya left a comment

Choose a reason for hiding this comment

viirya Jul 25, 2020

Choose a reason for hiding this comment

SparkQA commented Jul 25, 2020

dongjoon-hyun commented Jul 25, 2020

SparkQA commented Jul 25, 2020

HyukjinKwon commented Jul 25, 2020

HyukjinKwon Jul 27, 2020

Choose a reason for hiding this comment

SparkQA commented Jul 27, 2020

HyukjinKwon commented Jul 27, 2020

HyukjinKwon commented Jul 27, 2020

SparkQA commented Jul 27, 2020

HyukjinKwon commented Jul 27, 2020

SparkQA commented Jul 27, 2020

HyukjinKwon commented Jul 25, 2020 •

edited