Repository: geopandas/geopandas
Pull request #2073 (2021-08-21 12:43:16)

BUG: GeoSeries constructor should return Series for empty array with dtype when possible (fix GeoSeries.isna for empty Series)
While looking into some more of the warnings from our tests, I noticed that `GeoSeries([]).isna()` didn't properly return a boolean Series:

```
>>> geopandas.GeoSeries([]).isna()
GeoSeries([], dtype: geometry)
```

while the result of `isna` should always be boolean.


Patch:
diff --git a/ci/envs/37-minimal.yaml b/ci/envs/37-minimal.yaml
index 0b6fd994..9e3e1a01 100644
--- a/ci/envs/37-minimal.yaml
+++ b/ci/envs/37-minimal.yaml
@@ -19,7 +19,7 @@ dependencies:
   - rtree
   - matplotlib
   - matplotlib=3.1
-  - mapclassify>=2.2.0
+  # - mapclassify=2.4.0 - doesn't build due to conflicts
   - geopy
   - SQLalchemy
   - libspatialite
diff --git a/ci/envs/37-pd10.yaml b/ci/envs/37-pd10.yaml
index e72311f6..3d1d5ff6 100644
--- a/ci/envs/37-pd10.yaml
+++ b/ci/envs/37-pd10.yaml
@@ -25,5 +25,5 @@ dependencies:
   - pip:
       - pyproj==2.3.1
       - geopy
-      - mapclassify==2.2.0
-      - pyarrow
\ No newline at end of file
+      - mapclassify==2.4.0
+      - pyarrow
diff --git a/ci/envs/38-dev.yaml b/ci/envs/38-dev.yaml
index 7a8206e2..bed25392 100644
--- a/ci/envs/38-dev.yaml
+++ b/ci/envs/38-dev.yaml
@@ -21,7 +21,7 @@ dependencies:
   - pyarrow
   - pip:
     - geopy
-    - mapclassify>=2.2.0
+    - mapclassify>=2.4.0
     # dev versions of packages
     - git+https://github.com/numpy/numpy.git@main
     - git+https://github.com/pydata/pandas.git@master
diff --git a/doc/source/docs/user_guide/io.rst b/doc/source/docs/user_guide/io.rst
index 5d4ee9ee..534891ad 100644
--- a/doc/source/docs/user_guide/io.rst
+++ b/doc/source/docs/user_guide/io.rst
@@ -197,7 +197,7 @@ Spatial databases
 Writing to PostGIS::
 
     from sqlalchemy import create_engine
-    db_connection_url = "postgres://myusername:mypassword@myhost:5432/mydatabase";
+    db_connection_url = "postgresql://myusername:mypassword@myhost:5432/mydatabase";
     engine = create_engine(db_connection_url)
     countries_gdf.to_postgis("countries_table", con=engine)
 
diff --git a/doc/source/getting_started/install.rst b/doc/source/getting_started/install.rst
index 23c5a490..2f65ad71 100644
--- a/doc/source/getting_started/install.rst
+++ b/doc/source/getting_started/install.rst
@@ -155,7 +155,7 @@ Further, optional dependencies are:
 For plotting, these additional packages may be used:
 
 - `matplotlib`_ (>= 3.1.0)
-- `mapclassify`_ (>= 2.2.0)
+- `mapclassify`_ (>= 2.4.0)
 
 
 Using the optional PyGEOS dependency
diff --git a/geopandas/_compat.py b/geopandas/_compat.py
index c1e2202b..a93139a1 100644
--- a/geopandas/_compat.py
+++ b/geopandas/_compat.py
@@ -4,6 +4,7 @@ import importlib
 import os
 import warnings
 
+import numpy as np
 import pandas as pd
 import pyproj
 import shapely
@@ -147,6 +148,19 @@ if shapely_warning is not None and not SHAPELY_GE_20:
             yield
 
 
+elif (str(np.__version__) >= LooseVersion("1.21")) and not SHAPELY_GE_20:
+
+    @contextlib.contextmanager
+    def ignore_shapely2_warnings():
+        with warnings.catch_warnings():
+            # warning from numpy for existing Shapely releases (this is fixed
+            # with Shapely 1.8)
+            warnings.filterwarnings(
+                "ignore", "An exception was ignored while fetching", DeprecationWarning
+            )
+            yield
+
+
 else:
 
     @contextlib.contextmanager
diff --git a/geopandas/_config.py b/geopandas/_config.py
index b1c9fca2..99a2fca4 100644
--- a/geopandas/_config.py
+++ b/geopandas/_config.py
@@ -59,30 +59,13 @@ class Options(object):
                 doc_text = "\n".join(textwrap.wrap(option.doc, width=70))
             else:
                 doc_text = u"No description available."
-            doc_text = indent(doc_text, prefix="    ")
+            doc_text = textwrap.indent(doc_text, prefix="    ")
             description += doc_text + "\n"
         space = "\n  "
         description = description.replace("\n", space)
         return "{}({}{})".format(cls, space, description)
 
 
-def indent(text, prefix, predicate=None):
-    """
-    This is the python 3 textwrap.indent function, which is not available in
-    python 2.
-    """
-    if predicate is None:
-
-        def predicate(line):
-            return line.strip()
-
-    def prefixed_lines():
-        for line in text.splitlines(True):
-            yield (prefix + line if predicate(line) else line)
-
-    return "".join(prefixed_lines())
-
-
 def _validate_display_precision(value):
     if value is not None:
         if not isinstance(value, int) or not (0 <= value <= 16):
diff --git a/geopandas/array.py b/geopandas/array.py
index 434246fa..bf09528b 100644
--- a/geopandas/array.py
+++ b/geopandas/array.py
@@ -951,7 +951,8 @@ class GeometryArray(ExtensionArray):
             )
         # self.data[idx] = value
         value_arr = np.empty(1, dtype=object)
-        value_arr[:] = [value]
+        with compat.ignore_shapely2_warnings():
+            value_arr[:] = [value]
         self.data[idx] = value_arr
         return self
 
@@ -1044,6 +1045,38 @@ class GeometryArray(ExtensionArray):
         else:
             return np.array([g is None for g in self.data], dtype="bool")
 
+    def value_counts(
+        self,
+        dropna: bool = True,
+    ):
+        """
+        Compute a histogram of the counts of non-null values.
+
+        Parameters
+        ----------
+        dropna : bool, default True
+            Don't include counts of NaN
+
+        Returns
+        -------
+        pd.Series
+        """
+
+        # note ExtensionArray usage of value_counts only specifies dropna,
+        # so sort, normalize and bins are not arguments
+        values = to_wkb(self)
+        from pandas import Series, Index
+
+        result = Series(values).value_counts(dropna=dropna)
+        # value_counts converts None to nan, need to convert back for from_wkb to work
+        # note result.index already has object dtype, not geometry
+        # Can't use fillna(None) or Index.putmask, as this gets converted back to nan
+        # for object dtypes
+        result.index = Index(
+            from_wkb(np.where(result.index.isna(), None, result.index))
+        )
+        return result
+
     def unique(self):
         """Compute the ExtensionArray of unique values.
 
diff --git a/geopandas/base.py b/geopandas/base.py
index f4891574..19acae91 100644
--- a/geopandas/base.py
+++ b/geopandas/base.py
@@ -6,7 +6,6 @@ from pandas import DataFrame, Series
 
 from shapely.geometry import box
 from shapely.geometry.base import BaseGeometry
-from shapely.ops import cascaded_union
 
 from .array import GeometryArray, GeometryDtype
 
@@ -699,8 +698,13 @@ GeometryCollection
 
     @property
     def cascaded_union(self):
-        """Deprecated: Return the unary_union of all geometries"""
-        return cascaded_union(np.asarray(self.geometry.values))
+        """Deprecated: use `unary_union` instead"""
+        warn(
+            "The 'cascaded_union' attribute is deprecated, use 'unary_union' instead",
+            FutureWarning,
+            stacklevel=2,
+        )
+        return self.geometry.values.unary_union()
 
     @property
     def unary_union(self):
diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py
index e513ee68..1bcc6ce1 100644
--- a/geopandas/geodataframe.py
+++ b/geopandas/geodataframe.py
@@ -654,7 +654,7 @@ class GeoDataFrame(GeoPandasBase, DataFrame):
         PostGIS
 
         >>> from sqlalchemy import create_engine  # doctest: +SKIP
-        >>> db_connection_url = "postgres://myusername:mypassword@myhost:5432/mydb"
+        >>> db_connection_url = "postgresql://myusername:mypassword@myhost:5432/mydb"
         >>> con = create_engine(db_connection_url)  # doctest: +SKIP
         >>> sql = "SELECT geom, highway FROM roads"
         >>> df = geopandas.GeoDataFrame.from_postgis(sql, con)  # doctest: +SKIP
@@ -1702,7 +1702,7 @@ box': (2.0, 1.0, 2.0, 1.0)}], 'bbox': (1.0, 1.0, 2.0, 2.0)}
         --------
 
         >>> from sqlalchemy import create_engine
-        >>> engine = create_engine("postgres://myusername:mypassword@myhost:5432\
+        >>> engine = create_engine("postgresql://myusername:mypassword@myhost:5432\
 /mydatabase")  # doctest: +SKIP
         >>> gdf.to_postgis("my_table", engine)  # doctest: +SKIP
 
diff --git a/geopandas/geoseries.py b/geopandas/geoseries.py
index 73e3f607..928f08c3 100644
--- a/geopandas/geoseries.py
+++ b/geopandas/geoseries.py
@@ -190,7 +190,8 @@ class GeoSeries(GeoPandasBase, Series):
                 s = pd.Series(data, index=index, name=name, **kwargs)
             # prevent trying to convert non-geometry objects
             if s.dtype != object:
-                if s.empty or data is None:
+                if (s.empty and s.dtype == "float64") or data is None:
+                    # pd.Series with empty data gives float64 for older pandas versions
                     s = s.astype(object)
                 else:
                     warnings.warn(_SERIES_WARNING_MSG, FutureWarning, stacklevel=2)
diff --git a/geopandas/io/file.py b/geopandas/io/file.py
index 8908db4f..fbd17f80 100644
--- a/geopandas/io/file.py
+++ b/geopandas/io/file.py
@@ -112,7 +112,7 @@ def _read_file(filename, bbox=None, mask=None, rows=None, **kwargs):
 
     Reading only geometries intersecting ``bbox``:
 
-    >>> df = geopandas.read_file("nybb.shp", bbox=(0, 10, 0, 20))  # doctest: +SKIP
+    >>> df = geopandas.read_file("nybb.shp", bbox=(0, 0, 10, 20))  # doctest: +SKIP
 
     Returns
     -------
diff --git a/geopandas/io/sql.py b/geopandas/io/sql.py
index 4aa3767d..485cf937 100644
--- a/geopandas/io/sql.py
+++ b/geopandas/io/sql.py
@@ -138,7 +138,7 @@ def _read_postgis(
     PostGIS
 
     >>> from sqlalchemy import create_engine  # doctest: +SKIP
-    >>> db_connection_url = "postgres://myusername:mypassword@myhost:5432/mydatabase"
+    >>> db_connection_url = "postgresql://myusername:mypassword@myhost:5432/mydatabase"
     >>> con = create_engine(db_connection_url)  # doctest: +SKIP
     >>> sql = "SELECT geom, highway FROM roads"
     >>> df = geopandas.read_postgis(sql, con)  # doctest: +SKIP
@@ -362,7 +362,7 @@ def _write_postgis(
     --------
 
     >>> from sqlalchemy import create_engine  # doctest: +SKIP
-    >>> engine = create_engine("postgres://myusername:mypassword@myhost:5432\
+    >>> engine = create_engine("postgresql://myusername:mypassword@myhost:5432\
 /mydatabase";)  # doctest: +SKIP
     >>> gdf.to_postgis("my_table", engine)  # doctest: +SKIP
     """
diff --git a/geopandas/plotting.py b/geopandas/plotting.py
index 1582b2c3..d460e005 100644
--- a/geopandas/plotting.py
+++ b/geopandas/plotting.py
@@ -747,12 +747,27 @@ GON (((-122.84000 49.00000, -120.0000...
         vmax = len(categories) - 1 if vmax is None else vmax
 
     if scheme is not None:
+        mc_err = (
+            "The 'mapclassify' package (>= 2.4.0) is "
+            "required to use the 'scheme' keyword."
+        )
+        try:
+            import mapclassify
+
+        except ImportError:
+            raise ImportError(mc_err)
+
+        if mapclassify.__version__ < LooseVersion("2.4.0"):
+            raise ImportError(mc_err)
+
         if classification_kwds is None:
             classification_kwds = {}
         if "k" not in classification_kwds:
             classification_kwds["k"] = k
 
-        binning = _mapclassify_choro(values[~nan_idx], scheme, **classification_kwds)
+        binning = mapclassify.classify(
+            np.asarray(values[~nan_idx]), scheme, **classification_kwds
+        )
         # set categorical to True for creating the legend
         categorical = True
         if legend_kwds is not None and "labels" in legend_kwds:
@@ -932,100 +947,3 @@ class GeoplotAccessor(PlotAccessor):
 
     def geo(self, *args, **kwargs):
         return self(kind="geo", *args, **kwargs)
-
-
-def _mapclassify_choro(values, scheme, **classification_kwds):
-    """
-    Wrapper for choropleth schemes from mapclassify for use with plot_dataframe
-
-    Parameters
-    ----------
-    values
-        Series to be plotted
-    scheme : str
-        One of mapclassify classification schemes
-        Options are BoxPlot, EqualInterval, FisherJenks,
-        FisherJenksSampled, HeadTailBreaks, JenksCaspall,
-        JenksCaspallForced, JenksCaspallSampled, MaxP,
-        MaximumBreaks, NaturalBreaks, Quantiles, Percentiles, StdMean,
-        UserDefined
-
-    **classification_kwds : dict
-        Keyword arguments for classification scheme
-        For details see mapclassify documentation:
-        https://pysal.org/mapclassify/api.html
-
-    Returns
-    -------
-    binning
-        Binning objects that holds the Series with values replaced with
-        class identifier and the bins.
-    """
-    try:
-        import mapclassify.classifiers as classifiers
-
-    except ImportError:
-        raise ImportError(
-            "The 'mapclassify' >= 2.2.0 package is required to use the 'scheme' keyword"
-        )
-    from mapclassify import __version__ as mc_version
-
-    if mc_version < LooseVersion("2.2.0"):
-        raise ImportError(
-            "The 'mapclassify' >= 2.2.0 package is required to "
-            "use the 'scheme' keyword"
-        )
-    schemes = {}
-    for classifier in classifiers.CLASSIFIERS:
-        schemes[classifier.lower()] = getattr(classifiers, classifier)
-
-    scheme = scheme.lower()
-
-    # mapclassify < 2.1 cleaned up the scheme names (removing underscores)
-    # trying both to keep compatibility with older versions and provide
-    # compatibility with newer versions of mapclassify
-    oldnew = {
-        "Box_Plot": "BoxPlot",
-        "Equal_Interval": "EqualInterval",
-        "Fisher_Jenks": "FisherJenks",
-        "Fisher_Jenks_Sampled": "FisherJenksSampled",
-        "HeadTail_Breaks": "HeadTailBreaks",
-        "Jenks_Caspall": "JenksCaspall",
-        "Jenks_Caspall_Forced": "JenksCaspallForced",
-        "Jenks_Caspall_Sampled": "JenksCaspallSampled",
-        "Max_P_Plassifier": "MaxP",
-        "Maximum_Breaks": "MaximumBreaks",
-        "Natural_Breaks": "NaturalBreaks",
-        "Std_Mean": "StdMean",
-        "User_Defined": "UserDefined",
-    }
-    scheme_names_mapping = {}
-    scheme_names_mapping.update(
-        {old.lower(): new.lower() for old, new in oldnew.items()}
-    )
-    scheme_names_mapping.update(
-        {new.lower(): old.lower() for old, new in oldnew.items()}
-    )
-
-    try:
-        scheme_class = schemes[scheme]
-    except KeyError:
-        scheme = scheme_names_mapping.get(scheme, scheme)
-        try:
-            scheme_class = schemes[scheme]
-        except KeyError:
-            raise ValueError(
-                "Invalid scheme. Scheme must be in the set: %r" % schemes.keys()
-            )
-
-    if classification_kwds["k"] is not None:
-        from inspect import getfullargspec as getspec
-
-        spec = getspec(scheme_class.__init__)
-        if "k" not in spec.args:
-            del classification_kwds["k"]
-    try:
-        binning = scheme_class(np.asarray(values), **classification_kwds)
-    except TypeError:
-        raise TypeError("Invalid keyword argument for %r " % scheme)
-    return binning
diff --git a/geopandas/tools/sjoin.py b/geopandas/tools/sjoin.py
index da5b94cf..e1a4b8d7 100644
--- a/geopandas/tools/sjoin.py
+++ b/geopandas/tools/sjoin.py
@@ -319,6 +319,7 @@ def _frame_join(indices, left_df, right_df, how, lsuffix, rsuffix):
                 left_index=True,
                 right_on="_key_left",
                 how="right",
+                suffixes=("_{}".format(lsuffix), "_{}".format(rsuffix)),
             )
             .set_index(index_right)
             .drop(["_key_left", "_key_right"], axis=1)


Test patch:
diff --git a/geopandas/tests/test_extension_array.py b/geopandas/tests/test_extension_array.py
index 1b33cccf..b7db16a4 100644
--- a/geopandas/tests/test_extension_array.py
+++ b/geopandas/tests/test_extension_array.py
@@ -434,12 +434,12 @@ class TestComparisonOps(extension_tests.BaseComparisonOpsTests):
 
 
 class TestMethods(extension_tests.BaseMethodsTests):
-    @not_yet_implemented
+    @no_sorting
     @pytest.mark.parametrize("dropna", [True, False])
     def test_value_counts(self, all_data, dropna):
         pass
 
-    @not_yet_implemented
+    @no_sorting
     def test_value_counts_with_normalize(self, data):
         pass
 
diff --git a/geopandas/tests/test_geom_methods.py b/geopandas/tests/test_geom_methods.py
index f9c0afe7..a617dc3a 100644
--- a/geopandas/tests/test_geom_methods.py
+++ b/geopandas/tests/test_geom_methods.py
@@ -350,6 +350,16 @@ class TestGeomMethods:
 
         self._test_unary_topological("unary_union", expected, g)
 
+    def test_cascaded_union_deprecated(self):
+        p1 = self.t1
+        p2 = Polygon([(2, 0), (3, 0), (3, 1)])
+        g = GeoSeries([p1, p2])
+        with pytest.warns(
+            FutureWarning, match="The 'cascaded_union' attribute is deprecated"
+        ):
+            result = g.cascaded_union
+        assert result == g.unary_union
+
     def test_contains(self):
         expected = [True, False, True, False, False, False, False]
         assert_array_dtype_equal(expected, self.g0.contains(self.t1))
diff --git a/geopandas/tests/test_geoseries.py b/geopandas/tests/test_geoseries.py
index 1cb6796b..773c718a 100644
--- a/geopandas/tests/test_geoseries.py
+++ b/geopandas/tests/test_geoseries.py
@@ -20,7 +20,7 @@ from shapely.geometry import (
 from shapely.geometry.base import BaseGeometry
 
 from geopandas import GeoSeries, GeoDataFrame
-from geopandas._compat import PYPROJ_LT_3
+from geopandas._compat import PYPROJ_LT_3, ignore_shapely2_warnings
 from geopandas.array import GeometryArray, GeometryDtype
 from geopandas.testing import assert_geoseries_equal
 
@@ -355,6 +355,13 @@ def test_missing_values():
     assert len(s.dropna()) == 3
 
 
+def test_isna_empty_geoseries():
+    # ensure that isna() result for emtpy GeoSeries has the correct bool dtype
+    s = GeoSeries([])
+    result = s.isna()
+    assert_series_equal(result, pd.Series([], dtype="bool"))
+
+
 def test_geoseries_crs():
     gs = GeoSeries()
     gs.crs = "IGNF:ETRS89UTM28"
@@ -434,12 +441,45 @@ class TestConstructor:
         s = GeoSeries(index=range(3))
         check_geoseries(s)
 
+    def test_empty_array(self):
+        # with empty data that have an explicit dtype, we use the fallback or
+        # not depending on the dtype
+        arr = np.array([], dtype="bool")
+
+        # dtypes that can never hold geometry-like data
+        for arr in [
+            np.array([], dtype="bool"),
+            np.array([], dtype="int64"),
+            np.array([], dtype="float32"),
+            # this gets converted to object dtype by pandas
+            # np.array([], dtype="str"),
+        ]:
+            with pytest.warns(FutureWarning):
+                s = GeoSeries(arr)
+            assert not isinstance(s, GeoSeries)
+            assert type(s) == pd.Series
+
+        # dtypes that can potentially hold geometry-like data (object) or
+        # can come from empty data (float64)
+        for arr in [
+            np.array([], dtype="object"),
+            np.array([], dtype="float64"),
+            np.array([], dtype="str"),
+        ]:
+            with pytest.warns(None) as record:
+                s = GeoSeries(arr)
+            assert not record
+            assert isinstance(s, GeoSeries)
+
     def test_from_series(self):
         shapes = [
             Polygon([(random.random(), random.random()) for _ in range(3)])
             for _ in range(10)
         ]
-        s = pd.Series(shapes, index=list("abcdefghij"), name="foo")
+        with ignore_shapely2_warnings():
+            # the warning here is not suppressed by GeoPandas, as this is a pure
+            # pandas construction call
+            s = pd.Series(shapes, index=list("abcdefghij"), name="foo")
         g = GeoSeries(s)
         check_geoseries(g)
 
diff --git a/geopandas/tests/test_pandas_methods.py b/geopandas/tests/test_pandas_methods.py
index 585b77be..32673139 100644
--- a/geopandas/tests/test_pandas_methods.py
+++ b/geopandas/tests/test_pandas_methods.py
@@ -5,7 +5,7 @@ from numpy.testing import assert_array_equal
 import pandas as pd
 
 import shapely
-from shapely.geometry import Point, GeometryCollection
+from shapely.geometry import Point, GeometryCollection, LineString
 
 import geopandas
 from geopandas import GeoDataFrame, GeoSeries
@@ -408,13 +408,31 @@ def test_unique():
     assert_array_equal(s.unique(), exp)
 
 
-@pytest.mark.xfail
 def test_value_counts():
     # each object is considered unique
     s = GeoSeries([Point(0, 0), Point(1, 1), Point(0, 0)])
     res = s.value_counts()
     exp = pd.Series([2, 1], index=[Point(0, 0), Point(1, 1)])
     assert_series_equal(res, exp)
+    # Check crs doesn't make a difference - note it is not kept in output index anyway
+    s2 = GeoSeries([Point(0, 0), Point(1, 1), Point(0, 0)], crs="EPSG:4326")
+    res2 = s2.value_counts()
+    assert_series_equal(res2, exp)
+
+    # check mixed geometry
+    s3 = GeoSeries([Point(0, 0), LineString([[1, 1], [2, 2]]), Point(0, 0)])
+    res3 = s3.value_counts()
+    exp3 = pd.Series([2, 1], index=[Point(0, 0), LineString([[1, 1], [2, 2]])])
+    assert_series_equal(res3, exp3)
+
+    # check None is handled
+    s4 = GeoSeries([Point(0, 0), None, Point(0, 0)])
+    res4 = s4.value_counts(dropna=True)
+    exp4_dropna = pd.Series([2], index=[Point(0, 0)])
+    assert_series_equal(res4, exp4_dropna)
+    exp4_keepna = pd.Series([2, 1], index=[Point(0, 0), None])
+    res4_keepna = s4.value_counts(dropna=False)
+    assert_series_equal(res4_keepna, exp4_keepna)
 
 
 @pytest.mark.xfail(strict=False)
@@ -458,7 +476,7 @@ def test_groupby(df):
     assert_frame_equal(res, exp)
 
     # applying on the geometry column
-    res = df.groupby("value2")["geometry"].apply(lambda x: x.cascaded_union)
+    res = df.groupby("value2")["geometry"].apply(lambda x: x.unary_union)
     if compat.PANDAS_GE_11:
         exp = GeoSeries(
             [shapely.geometry.MultiPoint([(0, 0), (2, 2)]), Point(1, 1)],
diff --git a/geopandas/tests/test_sindex.py b/geopandas/tests/test_sindex.py
index c625a5d1..d96d6ee6 100644
--- a/geopandas/tests/test_sindex.py
+++ b/geopandas/tests/test_sindex.py
@@ -1,5 +1,3 @@
-import sys
-
 from shapely.geometry import (
     Point,
     Polygon,
@@ -18,7 +16,6 @@ import pytest
 import numpy as np
 
 
-@pytest.mark.skipif(sys.platform.startswith("win"), reason="fails on AppVeyor")
 @pytest.mark.skip_no_sindex
 class TestSeriesSindex:
     def test_has_sindex(self):
@@ -107,7 +104,6 @@ class TestSeriesSindex:
         assert sliced.sindex is not original_index
 
 
-@pytest.mark.skipif(sys.platform.startswith("win"), reason="fails on AppVeyor")
 @pytest.mark.skip_no_sindex
 class TestFrameSindex:
     def setup_method(self):
diff --git a/geopandas/tools/tests/test_sjoin.py b/geopandas/tools/tests/test_sjoin.py
index 107d270a..b18f6d4a 100644
--- a/geopandas/tools/tests/test_sjoin.py
+++ b/geopandas/tools/tests/test_sjoin.py
@@ -89,6 +89,23 @@ def dfs(request):
 
 
 class TestSpatialJoin:
+    @pytest.mark.parametrize(
+        "how, lsuffix, rsuffix, expected_cols",
+        [
+            ("left", "left", "right", {"col_left", "col_right", "index_right"}),
+            ("inner", "left", "right", {"col_left", "col_right", "index_right"}),
+            ("right", "left", "right", {"col_left", "col_right", "index_left"}),
+            ("left", "lft", "rgt", {"col_lft", "col_rgt", "index_rgt"}),
+            ("inner", "lft", "rgt", {"col_lft", "col_rgt", "index_rgt"}),
+            ("right", "lft", "rgt", {"col_lft", "col_rgt", "index_lft"}),
+        ],
+    )
+    def test_suffixes(self, how: str, lsuffix: str, rsuffix: str, expected_cols):
+        left = GeoDataFrame({"col": [1], "geometry": [Point(0, 0)]})
+        right = GeoDataFrame({"col": [1], "geometry": [Point(0, 0)]})
+        joined = sjoin(left, right, how=how, lsuffix=lsuffix, rsuffix=rsuffix)
+        assert set(joined.columns) == expected_cols | set(("geometry",))
+
     @pytest.mark.parametrize("dfs", ["default-index", "string-index"], indirect=True)
     def test_crs_mismatch(self, dfs):
         index, df1, df2, expected = dfs