Review comments

apaszke · apaszke · commit 50e21576ac4f · 2020-03-06T15:04:20.000+01:00
diff --git a/lib/matplotlib/cbook/__init__.py b/lib/matplotlib/cbook/__init__.py
@@ -2030,7 +2030,8 @@ def _array_patch_perimeters(x, rstride, cstride):
     """Extract perimeters of patches from **arr**.
 
     Extracted patches are of size (**rstride** + 1) x (**cstride** + 1) and
-    share perimeters with their neighbors.
+    share perimeters with their neighbors. The ordering of the vertices matches
+    that returned by ``_array_perimeter``.
 
     Parameters
     ----------
@@ -2044,11 +2045,28 @@ def _array_patch_perimeters(x, rstride, cstride):
     assert rstride > 0 and cstride > 0
     assert (x.shape[0] - 1) % rstride == 0
     assert (x.shape[1] - 1) % cstride == 0
-    upper = _unfold(x[:-1:rstride, :-1], 1, cstride, cstride)
-    lower = _unfold(x[rstride::rstride, 1:], 1, cstride, cstride)[..., ::-1]
+    # We build up each perimeter from four half-open intervals. Here is an
+    # illustrated explanation for rstride == cstride == 3
+    #
+    #       T T T R
+    #       L     R
+    #       L     R
+    #       L B B B
+    #
+    # where T means that this element will be in the top array, R for right,
+    # B for bottom and L for left. Each of the arrays below has a shape of:
+    #
+    #    (number of perimeters that can be extracted vertically,
+    #     number of perimeters that can be extracted horizontally,
+    #     cstride for top and bottom and rstride for left and right)
+    #
+    # Note that _unfold doesn't incur any memory copies, so the only costly
+    # operation here is the np.concatenate.
+    top = _unfold(x[:-1:rstride, :-1], 1, cstride, cstride)
+    bottom = _unfold(x[rstride::rstride, 1:], 1, cstride, cstride)[..., ::-1]
     right = _unfold(x[:-1, cstride::cstride], 0, rstride, rstride)
     left = _unfold(x[1:, :-1:cstride], 0, rstride, rstride)[..., ::-1]
-    return (np.concatenate((upper, right, lower, left), axis=2)
+    return (np.concatenate((top, right, bottom, left), axis=2)
               .reshape(-1, 2 * (rstride + cstride)))
 
 
diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py
@@ -610,10 +610,7 @@ def check(x, rstride, cstride):
                                   x, rstride=rstride, cstride=cstride))
 
     def divisors(n):
-        for i in range(1, n // 2 + 1):
-            if n % i == 0:
-                yield i
-        yield n
+        return [i for i in range(1, n + 1) if n % i == 0]
 
     for rows, cols in [(5, 5), (7, 14), (13, 9)]:
         x = np.arange(rows * cols).reshape(rows, cols)
