Repository: Toblerity/Shapely
Pull request #1075 (2021-01-26 08:09:21)

Deprecate len(..) of multi-part geometries
xref https://github.com/Toblerity/Shapely/issues/932

PR #950 already deprecated `__getitem__` and `__iter__` of multi-part geometries, but not yet `__len__`. 
Similarly to that deprecation, users can do `len(obj.geoms)` instead of `len(obj)`. We could also add a `.ngeoms` property as a shortcut for this, if that is desired. 

Note that I think deprecating `__len__` is not strictly needed for compatibility with numpy (although I would need to check this to be sure), as long as the geometries are not iterable anymore (or have an array_interface). Personally I think it follows a bit from deprecating getitem/iter, where `__len__` typically is implemented for collection-like objects (and which multi-part geometries no longer are in Shapely 2.0). So I am fine with deprecating it (you also need to know if you have a multi-part geometry or not before calling it, since `len(point)` currently fails, so if you already know that, it shouldn't be any problem to do eg `len(multipoint.geoms)` instead).

But, we could also keep `__len__` if people prefer that. Speaking in terms of the python ABCs (https://docs.python.org/3/library/collections.abc.html#collections-abstract-base-classes), multi-part geometries could be "Sized" without being "Iterable" / a "Collection".



sgillies: @jorisvandenbossche `len(geom)` used to call a GEOS method to get the number of geoms without materializing them, and was fast. As long as `len(geom.geoms)` is equally fast and efficient, that works for me, no need for a .ngeoms.
jorisvandenbossche: Yes, as long as we keep the `.geoms` attribute return a custom class and not a plain list or array, this is possible. That is also what is currently done in the shapely-2.0 branch: it kept the custom `GeometrySequence` class, and so checking the length of this directly calls the GEOS' method to check the number of parts of a Multi geometry:

https://github.com/Toblerity/Shapely/blob/34acbc1bd1fc12bcd07858f09133b2163fcb1e76/shapely/geometry/base.py#L705-L706
tomplex: Personally I agree with your initial thought @jorisvandenbossche, `len` strongly implies iterability and geometries are not iterable anymore. I suspect it will be more confusing than helpful when you can ask for the `len` of a geometry but have to use `geometry.geoms` to iterate over it.

Patch:
diff --git a/docs/manual.rst b/docs/manual.rst
index e043806..dd6e8fd 100644
--- a/docs/manual.rst
+++ b/docs/manual.rst
@@ -775,14 +775,12 @@ Its `x-y` bounding box is a ``(minx, miny, maxx, maxy)`` tuple.
   (-1.0, -1.0, 2.0, 2.0)
 
 Its members are instances of `Polygon` and are accessed via the ``geoms``
-property or via the iterator protocol using ``in`` or ``list()``.
+property.
 
 .. code-block:: pycon
 
   >>> len(polygons.geoms)
   3
-  >>> len(polygons)
-  3
 
 .. _empties:
 
@@ -1649,11 +1647,11 @@ mileage may vary when cleaning surfaces.
   True
   >>> clean
   <shapely.geometry.multipolygon.MultiPolygon object at ...>
-  >>> len(clean)
+  >>> len(clean.geoms)
   2
-  >>> list(clean[0].exterior.coords)
+  >>> list(clean.geoms[0].exterior.coords)
   [(0.0, 0.0), (0.0, 2.0), (1.0, 1.0), (0.0, 0.0)]
-  >>> list(clean[1].exterior.coords)
+  >>> list(clean.geoms[1].exterior.coords)
   [(1.0, 1.0), (2.0, 2.0), (2.0, 0.0), (1.0, 1.0)]
 
 Buffering splits the polygon in two at the point where they touch.
@@ -2147,7 +2145,7 @@ using functions in the :mod:`shapely.ops` module.
     ...     ((1, 1), (100, 100)),
     ...     ]
     >>> result, dangles, cuts, invalids = polygonize_full(lines)
-    >>> len(result)
+    >>> len(result.geoms)
     2
     >>> list(result.geoms)
     [<shapely.geometry.polygon.Polygon object at ...>, <shapely.geometry.polygon.Polygon object at ...>]
diff --git a/shapely/geometry/base.py b/shapely/geometry/base.py
index ac4ada5..fc4258f 100644
--- a/shapely/geometry/base.py
+++ b/shapely/geometry/base.py
@@ -870,6 +870,11 @@ class BaseMultipartGeometry(BaseGeometry):
             return iter([])
 
     def __len__(self):
+        warn(
+            "__len__ for multi-part geometries is deprecated and will be removed in "
+            "Shapely 2.0. Check the length of the `geoms` property instead to get the "
+            " number of parts of a multi-part geometry.",
+            ShapelyDeprecationWarning, stacklevel=2)
         if not self.is_empty:
             return len(self.geoms)
         else:
@@ -894,7 +899,7 @@ class BaseMultipartGeometry(BaseGeometry):
     def __eq__(self, other):
         return (
             type(other) == type(self) and
-            len(self) == len(other) and
+            len(self.geoms) == len(other.geoms) and
             all(x == y for x, y in zip(self.geoms, other.geoms))
         )
 
diff --git a/shapely/geometry/multipoint.py b/shapely/geometry/multipoint.py
index bf13f85..6464e9f 100644
--- a/shapely/geometry/multipoint.py
+++ b/shapely/geometry/multipoint.py
@@ -48,7 +48,7 @@ class MultiPoint(BaseMultipartGeometry):
         """
         super(MultiPoint, self).__init__()
 
-        if points is None or len(points) == 0:
+        if points is None or (not isinstance(points, MultiPoint) and len(points) == 0):
             # allow creation of empty multipoints, to support unpickling
             pass
         else:
diff --git a/shapely/ops.py b/shapely/ops.py
index 7c50c7f..11a2241 100644
--- a/shapely/ops.py
+++ b/shapely/ops.py
@@ -129,9 +129,9 @@ class CollectionOperator(object):
             "Use 'unary_union()' instead.",
             ShapelyDeprecationWarning, stacklevel=2)
         try:
-            L = len(geoms)
             if isinstance(geoms, BaseMultipartGeometry):
                 geoms = geoms.geoms
+            L = len(geoms)
         except TypeError:
             geoms = [geoms]
             L = 1
@@ -148,9 +148,9 @@ class CollectionOperator(object):
         prefered method for dissolving many polygons.
         """
         try:
-            L = len(geoms)
             if isinstance(geoms, BaseMultipartGeometry):
                 geoms = geoms.geoms
+            L = len(geoms)
         except TypeError:
             geoms = [geoms]
             L = 1


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index e34c5e0..af837cf 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -155,11 +155,10 @@ jobs:
       - name: Upload coverage
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          COVERALLS_SERVICE_NAME: github-actions
           COVERALLS_PARALLEL: true
         shell: bash
         run: |
-          coveralls || echo "!! intermittent coveralls failure"
+          coveralls --service=github || echo "!! intermittent coveralls failure"
 
   coveralls:
     name: Indicate completion to coveralls.io
diff --git a/tests/test_collection.py b/tests/test_collection.py
index e707507..09b5c11 100644
--- a/tests/test_collection.py
+++ b/tests/test_collection.py
@@ -31,7 +31,7 @@ def test_empty(geom):
     assert geom.type == "GeometryCollection"
     assert geom.type == geom.geom_type
     assert geom.is_empty
-    assert len(geom) == 0
+    assert len(geom.geoms) == 0
     assert geom.geoms == []
 
 
@@ -53,7 +53,7 @@ def test_child_with_deleted_parent():
 def test_from_geojson(geometrycollection_geojson):
     geom = shape(geometrycollection_geojson)
     assert geom.geom_type == "GeometryCollection"
-    assert len(geom) == 2
+    assert len(geom.geoms) == 2
 
     geom_types = [g.geom_type for g in geom.geoms]
     assert "Point" in geom_types
@@ -95,3 +95,22 @@ def test_geometrycollection_adapter_deprecated(geometrycollection_geojson):
     with pytest.warns(ShapelyDeprecationWarning):
         asShape(d)
 
+
+@pytest.mark.parametrize('geom', [
+    GeometryCollection(),
+    shape({"type": "GeometryCollection", "geometries": []}),
+    shape({"type": "GeometryCollection", "geometries": [
+        {"type": "Point", "coordinates": ()},
+        {"type": "LineString", "coordinates": (())}
+    ]}),
+    wkt.loads('GEOMETRYCOLLECTION EMPTY'),
+])
+def test_len_empty_deprecated(geom):
+    with pytest.warns(ShapelyDeprecationWarning, match="__len__"):
+        assert len(geom) == 0
+
+
+def test_len_deprecated(geometrycollection_geojson):
+    geom = shape(geometrycollection_geojson)
+    with pytest.warns(ShapelyDeprecationWarning, match="__len__"):
+        assert len(geom) == 2
diff --git a/tests/test_empty_polygons.py b/tests/test_empty_polygons.py
index 89f42e7..d740a21 100644
--- a/tests/test_empty_polygons.py
+++ b/tests/test_empty_polygons.py
@@ -19,4 +19,4 @@ def test_multipolygon_empty_polygon():
 
 def test_multipolygon_empty_among_polygon():
     """An empty polygon passed to MultiPolygon() is ignored."""
-    assert len(MultiPolygon([Point(0,0).buffer(1.0), Polygon()])) == 1
+    assert len(MultiPolygon([Point(0,0).buffer(1.0), Polygon()]).geoms) == 1
diff --git a/tests/test_make_valid.py b/tests/test_make_valid.py
index 78c7fd6..2166467 100644
--- a/tests/test_make_valid.py
+++ b/tests/test_make_valid.py
@@ -8,7 +8,7 @@ from tests.conftest import requires_geos_38
 def test_make_valid_invalid_input():
     geom = Polygon([(0, 0), (0, 2), (1, 1), (2, 2), (2, 0), (1, 1), (0, 0)])
     valid = make_valid(geom)
-    assert len(valid) == 2
+    assert len(valid.geoms) == 2
     assert all(geom.type == 'Polygon' for geom in valid.geoms)
 
 
diff --git a/tests/test_multipoint.py b/tests/test_multipoint.py
index b9449e9..afbffaf 100644
--- a/tests/test_multipoint.py
+++ b/tests/test_multipoint.py
@@ -140,3 +140,9 @@ def test_getitem_deprecated():
     geom = MultiPoint([[5.0, 6.0], [7.0, 8.0]])
     with pytest.warns(ShapelyDeprecationWarning, match="__getitem__"):
         part = geom[0]
+
+
+def test_len_deprecated():
+    geom = MultiPoint([[5.0, 6.0], [7.0, 8.0]])
+    with pytest.warns(ShapelyDeprecationWarning, match="__len__"):
+        assert len(geom) == 2
diff --git a/tests/test_polygonize.py b/tests/test_polygonize.py
index 53584ad..a67ef8d 100755
--- a/tests/test_polygonize.py
+++ b/tests/test_polygonize.py
@@ -32,7 +32,7 @@ class PolygonizeTestCase(unittest.TestCase):
             ]
 
         result2, dangles, cuts, invalids = polygonize_full(lines2)
-        self.assertEqual(len(result2), 2)
+        self.assertEqual(len(result2.geoms), 2)
         self.assertTrue(all([isinstance(x, Polygon) for x in result2.geoms]))
         self.assertEqual(list(dangles.geoms), [])
         self.assertTrue(all([isinstance(x, LineString) for x in cuts.geoms]))
diff --git a/tests/test_shared_paths.py b/tests/test_shared_paths.py
index b89394d..b033060 100644
--- a/tests/test_shared_paths.py
+++ b/tests/test_shared_paths.py
@@ -11,10 +11,10 @@ class SharedPaths(unittest.TestCase):
         result = shared_paths(g1, g2)
         
         self.assertTrue(isinstance(result, GeometryCollection))
-        self.assertTrue(len(result) == 2)
+        self.assertTrue(len(result.geoms) == 2)
         a, b = result.geoms
         self.assertTrue(isinstance(a, MultiLineString))
-        self.assertTrue(len(a) == 1)
+        self.assertTrue(len(a.geoms) == 1)
         self.assertEqual(a.geoms[0].coords[:], [(5, 0), (10, 0)])
         self.assertTrue(b.is_empty)
 
@@ -24,10 +24,10 @@ class SharedPaths(unittest.TestCase):
         result = shared_paths(g1, g2)
         
         self.assertTrue(isinstance(result, GeometryCollection))
-        self.assertTrue(len(result) == 2)
+        self.assertTrue(len(result.geoms) == 2)
         a, b = result.geoms
         self.assertTrue(isinstance(b, MultiLineString))
-        self.assertTrue(len(b) == 1)
+        self.assertTrue(len(b.geoms) == 1)
         self.assertEqual(b.geoms[0].coords[:], [(5, 0), (10, 0)])
         self.assertTrue(a.is_empty)
     
diff --git a/tests/test_split.py b/tests/test_split.py
index 3eaa1c8..68065da 100644
--- a/tests/test_split.py
+++ b/tests/test_split.py
@@ -10,7 +10,7 @@ class TestSplitGeometry(unittest.TestCase):
 	def helper(self, geom, splitter, expected_chunks):
 		s = split(geom, splitter)
 		self.assertEqual(s.type, "GeometryCollection")
-		self.assertEqual(len(s), expected_chunks)
+		self.assertEqual(len(s.geoms), expected_chunks)
 		if expected_chunks > 1:
 			# split --> expected collection that when merged is again equal to original geometry
 			if s.geoms[0].type == 'LineString':
diff --git a/tests/test_voronoi_diagram.py b/tests/test_voronoi_diagram.py
index 0d3ede6..79ec2c0 100644
--- a/tests/test_voronoi_diagram.py
+++ b/tests/test_voronoi_diagram.py
@@ -22,7 +22,7 @@ def test_no_regions():
     mp = MultiPoint(points=[(0.5, 0.5)])
     regions = voronoi_diagram(mp)
 
-    assert len(regions) == 0
+    assert len(regions.geoms) == 0
 
 
 @requires_geos_35
@@ -30,7 +30,7 @@ def test_two_regions():
     mp = MultiPoint(points=[(0.5, 0.5), (1.0, 1.0)])
     regions = voronoi_diagram(mp)
 
-    assert len(regions) == 2
+    assert len(regions.geoms) == 2
 
 
 @requires_geos_35
@@ -38,7 +38,7 @@ def test_edges():
     mp = MultiPoint(points=[(0.5, 0.5), (1.0, 1.0)])
     regions = voronoi_diagram(mp, edges=True)
 
-    assert len(regions) == 1
+    assert len(regions.geoms) == 1
     assert all(r.type == 'LineString' for r in regions.geoms)
 
 
@@ -49,7 +49,7 @@ def test_smaller_envelope():
 
     regions = voronoi_diagram(mp, envelope=poly)
 
-    assert len(regions) == 2
+    assert len(regions.geoms) == 2
     assert sum(r.area for r in regions.geoms) > poly.area
 
 
@@ -63,7 +63,7 @@ def test_larger_envelope():
 
     regions = voronoi_diagram(mp, envelope=poly)
 
-    assert len(regions) == 2
+    assert len(regions.geoms) == 2
     assert sum(r.area for r in regions.geoms) == poly.area
 
 
@@ -72,7 +72,7 @@ def test_from_polygon():
     poly = load_wkt('POLYGON ((0 0, 1 0, 1 1, 0 1, 0 0))')
     regions = voronoi_diagram(poly)
 
-    assert len(regions) == 4
+    assert len(regions.geoms) == 4
 
 
 @requires_geos_35
@@ -80,7 +80,7 @@ def test_from_polygon_with_enough_tolerance():
     poly = load_wkt('POLYGON ((0 0, 0.5 0, 0.5 0.5, 0 0.5, 0 0))')
     regions = voronoi_diagram(poly, tolerance=1.0)
 
-    assert len(regions) == 2
+    assert len(regions.geoms) == 2
 
 
 @requires_geos_35
@@ -131,5 +131,5 @@ def test_from_multipoint_without_tolerace_without_floating_point_coordinates():
     """But it's fine without it."""
     mp = load_wkt('MULTIPOINT (0 0, 1 0, 1 2, 0 1)')
     regions = voronoi_diagram(mp)
-    assert len(regions) == 4
+    assert len(regions.geoms) == 4