Repository: Toblerity/Shapely
Pull request #658 (2018-11-19 19:42:08)

Fixing bug in ops.substring
If `min_dist` is equal to 0 (i.e. the first coordinate) then the loop immediately breaks.

This means that internal coordinates are skipped because only `(start_point.x, start_point.y)` and `(end_point.x, end_point.y)` are written to the new `vertex_list`.

By changing the less than to a less-than-equal sign the loop behaves as intended.

shongololo: Fixes #659 
coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20187507/badge)](https://coveralls.io/builds/20187507)

Coverage remained the same at 81.152% when pulling **0fe1027d0a49b69aae24591268acf4bf6f7fd9ae on shongololo:patch-1** into **fc12a113f2c234ec76f4935d1a94d0b134cc91fb on Toblerity:master**.

sgillies: @shongololo I see what you mean. Can you add a test to tests/test_substring.py to confirm the fix? Something like this after 70 should suffice.

```
    @unittest.skipIf(geos_version < (3, 2, 0), 'GEOS 3.2.0 required')
    def test_return_substring_min_dist-zero(self):
        """Confirm fix of #658"""
        self.assertTrue(substring(self.line2, 0.0, 7.0).equals(LineString(([3.0, 0.0], [3.0, 6.0], [4.0, 6.0]))))
```

Patch:
diff --git a/shapely/ops.py b/shapely/ops.py
index 3c60f46..2fc0a70 100644
--- a/shapely/ops.py
+++ b/shapely/ops.py
@@ -516,9 +516,9 @@ def substring(geom, start_dist, end_dist, normalized=False):
     
     vertex_list = [(start_point.x, start_point.y)]
     coords = list(geom.coords)
-    for i, p in enumerate(coords):
+    for p in coords:
         pd = geom.project(Point(p))
-        if pd < min_dist:
+        if pd <= min_dist:
             pass
         elif min_dist < pd < max_dist:
             vertex_list.append(p)


Test patch:
diff --git a/tests/test_substring.py b/tests/test_substring.py
index b2015a2..1cf19d0 100644
--- a/tests/test_substring.py
+++ b/tests/test_substring.py
@@ -68,7 +68,9 @@ class SubstringTestCase(unittest.TestCase):
     def test_return_substring_with_vertices(self):
         self.assertTrue(substring(self.line2, 1, 7).equals(LineString(([3, 1], [3, 6], [4, 6]))))
         self.assertTrue(substring(self.line2, 0.2, 0.9, True).equals(LineString(([3, 1.5], [3, 6], [3.75, 6]))))
-
+        self.assertTrue(substring(self.line2, 0, 0.9, True).equals(LineString(([3, 0], [3, 6], [3.75, 6]))))
+        self.assertTrue(substring(self.line2, 0.2, 1, True).equals(LineString(([3, 1.5], [3, 6], [4.5, 6]))))
+        
 
 def test_suite():
     loader = unittest.TestLoader()