Merge pull request #1856 from AdeelH/geom

Improve geometry-related validation in `Scene` and `GeoJSONVectorSource`

Author: AdeelH

Diff for: rastervision_core/rastervision/core/data/scene.py

@@ -45,6 +45,11 @@ def __init__(self,
             self.aoi_polygons = []
             self.aoi_polygons_bbox_coords = []
         else:
+            for p in aoi_polygons:
+                if p.geom_type not in ['Polygon', 'MultiPolygon']:
+                    raise ValueError(
+                        'Expected all AOI geometries to be Polygons or '
+                        f'MultiPolygons. Found: {p.geom_type}.')
             bbox = self.raster_source.bbox
             bbox_geom = bbox.to_shapely()
             self.aoi_polygons = [

Diff for: rastervision_core/rastervision/core/data/vector_source/geojson_vector_source.py

@@ -1,4 +1,5 @@
 from typing import TYPE_CHECKING, List, Optional, Union
+import logging
 
 from rastervision.pipeline.file_system import download_if_needed, file_to_json
 from rastervision.core.box import Box
@@ -8,6 +9,8 @@
 if TYPE_CHECKING:
     from rastervision.core.data import CRSTransformer, VectorTransformer
 
+log = logging.getLogger(__name__)
+
 
 class GeoJSONVectorSource(VectorSource):
     """A :class:`.VectorSource` for reading GeoJSON files."""
@@ -49,13 +52,27 @@ def _get_geojson(self) -> dict:
     def _get_geojson_single(self, uri: str) -> dict:
         # download first so that it gets cached
         geojson = file_to_json(download_if_needed(uri))
-        if not self.ignore_crs_field and 'crs' in geojson:
-            raise NotImplementedError(
-                f'The GeoJSON file at {uri} contains a CRS field which '
-                'is not allowed by the current GeoJSON standard or by '
-                'Raster Vision. All coordinates are expected to be in '
-                'EPSG:4326 CRS. If the file uses EPSG:4326 (ie. lat/lng on '
-                'the WGS84 reference ellipsoid) and you would like to ignore '
-                'the CRS field, set ignore_crs_field=True in '
-                'GeoJSONVectorSourceConfig.')
+        if 'crs' in geojson:
+            if not self.ignore_crs_field:
+                raise NotImplementedError(
+                    f'The GeoJSON file at {uri} contains a CRS field which '
+                    'is not allowed by the current GeoJSON standard or by '
+                    'Raster Vision. All coordinates are expected to be in '
+                    'EPSG:4326 CRS. If the file uses EPSG:4326 (ie. lat/lng '
+                    'on the WGS84 reference ellipsoid) and you would like to '
+                    'ignore the CRS field, set ignore_crs_field=True.')
+            else:
+                crs = geojson['crs']
+                log.info(f'Ignoring CRS ({crs}) specified in {uri} '
+                         'and assuming EPSG:4326 instead.')
+                # Delete the CRS field to avoid discrepancies in case the
+                # geojson is passed to code that *does* respect the CRS field
+                # (e.g. geopandas).
+                del geojson['crs']
+                # Also delete any "crs" keys in features' properties.
+                for f in geojson.get('features', []):
+                    try:
+                        del f['properties']['crs']
+                    except KeyError:
+                        pass
         return geojson

Diff for: rastervision_pytorch_learner/rastervision/pytorch_learner/dataset/utils/aoi_sampler.py

@@ -1,7 +1,7 @@
-from typing import Sequence, Tuple
+from typing import Sequence, Tuple, Union
 
 import numpy as np
-from shapely.geometry import Polygon, MultiPolygon
+from shapely.geometry import Polygon, MultiPolygon, LinearRing
 from shapely.ops import unary_union
 from triangle import triangulate
 
@@ -86,7 +86,9 @@ def triangulate_polygon(self, polygon: Polygon) -> dict:
 
             # the triangulation algorithm requires a sample point inside each
             # hole
-            hole_centroids = np.stack([hole.centroid for hole in holes])
+            hole_centroids = [hole.centroid for hole in holes]
+            hole_centroids = np.concatenate(
+                [np.array(c.coords) for c in hole_centroids], axis=0)
 
             args = {
                 'vertices': vertices,
@@ -108,18 +110,20 @@ def triangulate_polygon(self, polygon: Polygon) -> dict:
         }
         return out
 
-    def polygon_to_graph(self,
-                         polygon: Polygon) -> Tuple[np.ndarray, np.ndarray]:
+    def polygon_to_graph(self, polygon: Union[Polygon, LinearRing]
+                         ) -> Tuple[np.ndarray, np.ndarray]:
         """Given a polygon, return its graph representation.
 
         Args:
-            polygon (Polygon): A polygon.
+            polygon (Union[Polygon, LinearRing]): A polygon or
+                polygon-exterior.
 
         Returns:
             Tuple[np.ndarray, np.ndarray]: An (N, 2) array of vertices and
             an (N, 2) array of indices to vertices representing edges.
         """
-        vertices = np.array(polygon.exterior.coords)
+        exterior = getattr(polygon, 'exterior', polygon)
+        vertices = np.array(exterior.coords)
         # Discard the last vertex - it is a duplicate of the first vertex and
         # duplicates cause problems for the Triangle library.
         vertices = vertices[:-1]

Diff for: tests/core/data/test_scene.py

@@ -43,6 +43,19 @@ def test_aoi_polygons(self):
         self.assertListEqual(scene.aoi_polygons_bbox_coords,
                              aoi_polygons_bbox_coords)
 
+    def test_invalid_aoi_polygons(self):
+        bbox = Box(100, 100, 200, 200)
+        rs = RasterioSource(self.img_uri, bbox=bbox)
+
+        aoi_polygons = [
+            Box(50, 50, 150, 150).to_shapely(),
+            Box(150, 150, 250, 250).to_shapely(),
+            # not a polygon:
+            Box(150, 150, 250, 250).to_shapely().exterior,
+        ]
+        with self.assertRaises(ValueError):
+            _ = Scene(id='', raster_source=rs, aoi_polygons=aoi_polygons)
+
 
 if __name__ == '__main__':
     unittest.main()

Diff for: tests/core/data/vector_source/test_geojson_vector_source.py

@@ -1,16 +1,17 @@
+from typing import Callable
 import unittest
 import os
 
 from shapely.geometry import shape
 
+from rastervision.core.data import (
+    BufferTransformerConfig, ClassConfig, ClassInferenceTransformerConfig,
+    GeoJSONVectorSource, GeoJSONVectorSourceConfig, IdentityCRSTransformer)
 from rastervision.core.data.vector_source.geojson_vector_source_config import (
-    GeoJSONVectorSourceConfig, geojson_vector_source_config_upgrader)
-from rastervision.core.data import (ClassConfig, IdentityCRSTransformer,
-                                    ClassInferenceTransformerConfig,
-                                    BufferTransformerConfig)
+    geojson_vector_source_config_upgrader)
 from rastervision.pipeline.file_system import json_to_file, get_tmp_dir
 
-from tests import test_config_upgrader
+from tests import test_config_upgrader, data_file_path
 from tests.core.data.mock_crs_transformer import DoubleCRSTransformer
 
 
@@ -30,6 +31,12 @@ def test_upgrader(self):
 class TestGeoJSONVectorSource(unittest.TestCase):
     """This also indirectly tests the ClassInference class."""
 
+    def assertNoError(self, fn: Callable, msg: str = ''):
+        try:
+            fn()
+        except Exception:
+            self.fail(msg)
+
     def setUp(self):
         self.tmp_dir = get_tmp_dir()
         self.uri = os.path.join(self.tmp_dir.name, 'vectors.json')
@@ -155,6 +162,19 @@ def test_transform_polygon(self):
         trans_geom = trans_geojson['features'][0]['geometry']
         self.assertTrue(shape(geom).equals(shape(trans_geom)))
 
+    def test_ignore_crs_field(self):
+        uri = data_file_path('0-aoi.geojson')
+        crs_transformer = IdentityCRSTransformer()
+
+        vs = GeoJSONVectorSource(uri, crs_transformer=crs_transformer)
+        with self.assertRaises(NotImplementedError):
+            _ = vs.get_geojson()
+
+        vs = GeoJSONVectorSource(
+            uri, crs_transformer=crs_transformer, ignore_crs_field=True)
+        self.assertNoError(lambda: vs.get_geojson())
+        self.assertNotIn('crs', vs.get_geojson())
+
 
 if __name__ == '__main__':
     unittest.main()

Diff for: tests/pytorch_learner/dataset/test_aoi_sampler.py

@@ -1,3 +1,4 @@
+from typing import Callable
 import unittest
 from itertools import product
 
@@ -9,6 +10,18 @@
 
 
 class TestAoiSampler(unittest.TestCase):
+    def assertNoError(self, fn: Callable, msg: str = ''):
+        try:
+            fn()
+        except Exception:
+            self.fail(msg)
+
+    def test_polygon_with_holes(self):
+        p1 = Polygon.from_bounds(0, 0, 20, 20)
+        p2 = Polygon.from_bounds(5, 5, 15, 15)
+        polygon_with_holes = p1 - p2
+        self.assertNoError(lambda: AoiSampler([polygon_with_holes]).sample())
+
     def test_sampler(self, nsamples: int = 200):
         """Attempt to check if points are distributed uniformly within an AOI.