Repository: geopandas/geopandas

Issue:
MAINT: unify spatial index error handling
Currently, there is no unified approach to error handling for empty spatial indexes or lack of a spatail index backend:
- sjoin does it's own check for rtree, this means you need rtree even if you have pygeos!
- overlay and clip do not do any checks, they would fail with something like `None does not have any attribute query_bulk`, which is not very helpful.

Further, when you create a spatial index, if:
- The spatial index is empty, `GeoDataFrame.sindex` gets set to None
- There is not spatial index backend, `GeoDataFrame.sindex` gets set to None and a _warning_ is raised.

I don't particularly like that handling. There is no way to tell what happened, aside from a warning.

I would like to propose the following:
- No modules outside of `sindex` shall check for a spatial index backend. They should just blindly try to access the `sindex` property.
- The `sindex` backends should be instantiated even with no geometries, and should have a `__len__` property that reports 0 when empty (this preserves the `if gdf.sindex:` syntax used in some modules).
- In the case of no spatial index backend, a clear error should be raised by `sindex.*

\* Currently, the error is raised only when a property of the spatial index is accessed (`sindex.intersection`). There are two options with this:
1. Preserve the current behavior, but make the error more useful. This would require setting `GeoDataFrame.sindex` to some non-None value, perhaps a dummy class that raises a user friendly error if any of its methods (`query`, `query_bulk`, etc.) are accessed and always reports a `len` of 0. The big con of this approach is complexity. We'd have to make a dummy class, etc.
2. Raise an error when `GeoDataFrame.sindex` is accessed. This is the lower complexity approach, we just replace what is currently a warning with an error. It would however require either a `try/except` block or an import of `geopandas.sindex.has_sindex` by other modules to handle this error if they want to operate without a spatial index. Currently, all modules that use a spatial index _cannot_ operate without a spatial index, so they would actually not need to do anything.

Discussion:
adriangb: @martinfleis  I think the main things to check here are:
1. The text for the errors/warnings. 
2. Do we need to edit docs / installation instructions as well, to reflect that you can use either pygeos or rtree, and that pygeos is prefered?
adriangb: I added `--cov-append` to all CI runs so that the run with no optional dependencies (which runs some tests for error handling when there is no spatial index) also gets run.
adriangb: FYI @martinfleis , you might want to enable the option in Travis which cancels old runs when new commits are pushed, no point in wasting compute.
adriangb: > I added `--cov-append` to all CI runs so that the run with no optional dependencies (which runs some tests for error handling when there is no spatial index) also gets run.

This failed, of course you can't append coverage across CI runs. I guess we just won't include those parts in coverage for now.
adriangb: Ok, all tests are passing now. Unless anyone has any good ideas for the coverage issue, I think this should be good to go.
jorisvandenbossche: @adriangb Thanks for the nice clean-up!
adriangb: @brendan-ward I think I addressed all of your comments, and the tests are passing. Let me know if there's anything else!

Patch:
diff --git a/geopandas/base.py b/geopandas/base.py
index 6503ca70..3e971b17 100644
--- a/geopandas/base.py
+++ b/geopandas/base.py
@@ -100,6 +100,17 @@ class GeoPandasBase(object):
             _sindex = sindex_cls(self.geometry)
             if not _sindex.is_empty:
                 self._sindex = _sindex
+            else:
+                warn(
+                    "Generated spatial index is empty and returned `None`. "
+                    "Future versions of GeoPandas will return zero-length spatial "
+                    "index instead of `None`. Use `len(gdf.sindex) > 0` "
+                    "or `if gdf.sindex` instead of `if gd.sindex is not None` "
+                    "to check for empty spatial indexes.",
+                    FutureWarning,
+                    stacklevel=3,
+                )
+                self._sindex = None
         self._sindex_generated = True
 
     def _invalidate_sindex(self):
diff --git a/geopandas/sindex.py b/geopandas/sindex.py
index 22b6181f..90e8ac4c 100644
--- a/geopandas/sindex.py
+++ b/geopandas/sindex.py
@@ -1,4 +1,3 @@
-from warnings import warn
 from collections import namedtuple
 
 from shapely.geometry.base import BaseGeometry
@@ -22,21 +21,27 @@ VALID_QUERY_PREDICATES = {
 def has_sindex():
     """Dynamically checks for ability to generate spatial index.
     """
-    return get_sindex_class() is not None
+    try:
+        get_sindex_class()
+        return True
+    except ImportError:
+        return False
 
 
 def get_sindex_class():
     """Dynamically chooses a spatial indexing backend.
 
     Required to comply with _compat.USE_PYGEOS.
-    The selection order goes PyGEOS > RTree > None.
+    The selection order goes PyGEOS > RTree > Error.
     """
     if compat.USE_PYGEOS:
         return PyGEOSSTRTreeIndex
     if compat.HAS_RTREE:
         return RTreeIndex
-    warn("Spatial indexes require either `rtree` or `pygeos`.")
-    return None
+    raise ImportError(
+        "Spatial indexes require either `rtree` or `pygeos`. "
+        "See installation instructions at https://geopandas.org/install.html"
+    )
 
 
 if compat.HAS_RTREE:
@@ -246,6 +251,9 @@ if compat.HAS_RTREE:
         def is_empty(self):
             return self.size == 0
 
+        def __len__(self):
+            return self.size
+
 
 if compat.HAS_PYGEOS:
 
diff --git a/geopandas/tools/sjoin.py b/geopandas/tools/sjoin.py
index 51775c05..7f67c19a 100644
--- a/geopandas/tools/sjoin.py
+++ b/geopandas/tools/sjoin.py
@@ -7,7 +7,6 @@ from shapely import prepared
 
 from geopandas import GeoDataFrame
 from geopandas import _compat as compat
-from geopandas._compat import HAS_RTREE
 
 
 def sjoin(
@@ -76,13 +75,6 @@ def sjoin(
             " joined".format(index_left, index_right)
         )
 
-    # Check if rtree is installed...
-    if not HAS_RTREE:
-        raise ImportError(
-            "Rtree must be installed to use sjoin\n\n"
-            "See installation instructions at https://geopandas.org/install.html"
-        )
-
     # Attempt to re-use spatial indexes, otherwise generate the spatial index
     # for the longer dataframe. If we are joining to an empty dataframe,
     # don't bother generating the index.


Test patch:
diff --git a/geopandas/tests/test_sindex.py b/geopandas/tests/test_sindex.py
index 91c3dc3e..ecc6f1e0 100644
--- a/geopandas/tests/test_sindex.py
+++ b/geopandas/tests/test_sindex.py
@@ -18,20 +18,36 @@ import pytest
 import numpy as np
 
 
-@pytest.mark.skipif(sindex.has_sindex(), reason="Spatial index present, skipping")
 class TestNoSindex:
-    def test_no_sindex(self):
-        """Checks that a warning is given when no spatial index is present."""
-        with pytest.warns(UserWarning):
+    @pytest.mark.skipif(sindex.has_sindex(), reason="Spatial index present, skipping")
+    def test_no_sindex_installed(self):
+        """Checks that an error is raised when no spatial index is present."""
+        with pytest.raises(ImportError):
             sindex.get_sindex_class()
 
+    @pytest.mark.skipif(
+        compat.HAS_RTREE or not compat.HAS_PYGEOS,
+        reason="rtree cannot be disabled via flags",
+    )
+    def test_no_sindex_active(self):
+        """Checks that an error is given when rtree is not installed
+        and compat.USE_PYGEOS is False.
+        """
+        state = compat.USE_PYGEOS  # try to save state
+        compat.set_use_pygeos(False)
+        with pytest.raises(ImportError):
+            sindex.get_sindex_class()
+        compat.set_use_pygeos(state)  # try to restore state
+
 
 @pytest.mark.skipif(sys.platform.startswith("win"), reason="fails on AppVeyor")
 @pytest.mark.skipif(not sindex.has_sindex(), reason="Spatial index absent, skipping")
 class TestSeriesSindex:
     def test_empty_geoseries(self):
-
-        assert GeoSeries().sindex is None
+        """Tests creating a spatial index from an empty GeoSeries."""
+        with pytest.warns(FutureWarning, match="Generated spatial index is empty"):
+            # TODO: add checking len(GeoSeries().sindex) == 0 once deprecated
+            assert not GeoSeries(dtype=object).sindex
 
     def test_point(self):
         s = GeoSeries([Point(0, 0)])
@@ -42,9 +58,13 @@ class TestSeriesSindex:
         assert len(list(hits)) == 0
 
     def test_empty_point(self):
+        """Tests that a single empty Point results in an empty tree."""
         s = GeoSeries([Point()])
 
-        assert s.sindex is None
+        with pytest.warns(FutureWarning, match="Generated spatial index is empty"):
+            # TODO: add checking len(s) == 0 once deprecated
+            assert not s.sindex
+
         assert s._sindex_generated is True
 
     def test_polygons(self):
@@ -498,11 +518,15 @@ class TestPygeosInterface:
         """Tests the `size` property."""
         assert self.df.sindex.size == self.expected_size
 
+    def test_len(self):
+        """Tests the `__len__` method of spatial indexes."""
+        assert len(self.df.sindex) == self.expected_size
+
     def test_is_empty(self):
         """Tests the `is_empty` property."""
         # create empty tree
         cls_ = sindex.get_sindex_class()
-        empty = geopandas.GeoSeries([])
+        empty = geopandas.GeoSeries(dtype=object)
         tree = cls_(empty)
         assert tree.is_empty
         # create a non-empty tree
diff --git a/geopandas/tools/tests/test_sjoin.py b/geopandas/tools/tests/test_sjoin.py
index 32d2da60..5c0f9b0a 100644
--- a/geopandas/tools/tests/test_sjoin.py
+++ b/geopandas/tools/tests/test_sjoin.py
@@ -7,12 +7,16 @@ from shapely.geometry import Point, Polygon, GeometryCollection
 
 import geopandas
 from geopandas import GeoDataFrame, GeoSeries, read_file, sindex, sjoin
-from geopandas._compat import HAS_RTREE
 
 from pandas.testing import assert_frame_equal
 import pytest
 
 
+pytestmark = pytest.mark.skipif(
+    not sindex.has_sindex(), reason="sjoin requires spatial index"
+)
+
+
 @pytest.fixture()
 def dfs(request):
     polys1 = GeoSeries(
@@ -86,15 +90,6 @@ def dfs(request):
     return [request.param, df1, df2, expected]
 
 
-@pytest.mark.parametrize("dfs", ["default-index"], indirect=True)
-def test_raises_error_if_rtree_not_install(dfs):
-    _, df1, df2, _ = dfs
-    if not HAS_RTREE:
-        with pytest.raises(ImportError):
-            geopandas.sjoin(df1, df2)
-
-
-@pytest.mark.skipif(not sindex.has_sindex(), reason="Spatial index absent, skipping")
 class TestSpatialJoin:
     @pytest.mark.parametrize("dfs", ["default-index", "string-index"], indirect=True)
     def test_crs_mismatch(self, dfs):
@@ -306,7 +301,6 @@ class TestSpatialJoin:
         assert_frame_equal(res, exp, check_index_type=False)
 
 
-@pytest.mark.skipif(not sindex.has_sindex(), reason="Spatial index absent, skipping")
 class TestSpatialJoinNYBB:
     def setup_method(self):
         nybb_filename = geopandas.datasets.get_path("nybb")
@@ -474,7 +468,6 @@ class TestSpatialJoinNYBB:
         assert df2.shape == (21, 8)
 
 
-@pytest.mark.skipif(not sindex.has_sindex(), reason="Spatial index absent, skipping")
 class TestSpatialJoinNaturalEarth:
     def setup_method(self):
         world_path = geopandas.datasets.get_path("naturalearth_lowres")