Repository: geopandas/geopandas

Issue:
BUG: GeoSeries.is_ring returns False even for all LineStrings and LinearRings
Affects both pygeos and shapely backends.

```py
import geopandas
from shapely.geometry import LineString, LinearRing
s = geopandas.GeoSeries(
    [
        LineString([(0, 0), (1, 1), (1, -1)]),
        LineString([(0, 0), (1, 1), (1, -1), (0, 0)]),
        LinearRing([(0, 0), (1, 1), (1, -1)]),
    ]
)

s.is_ring

0    False
1    False
2    False
dtype: bool

s[0].is_ring, s[1].is_ring, s[2].is_ring

(False, True, True)
```

The reason is that we pass exterior to `is_ring`, which makes sense for Polygons, but return False if geometry does not have `exterior`. We should make the algorithm more clever to recognize geom types and act appropriately.

https://github.com/geopandas/geopandas/blob/72427d3d8c128039bfce1d54a76c0b652887b276/geopandas/_vectorized.py#L478-L493

Discussion:
martinfleis: Yeah, you are right. I understand it the same way, but I did not want to change the behaviour it has on polygons. It's been there since 0.1. 

Now, it always returns True for a Polygon (there's no other way), False for MultiPolygon and False for everything else. Which is pointless I'd say. But changing it to _correct_ behaviour as in GEOS totally flips it. 

I do agree that we should follow GEOS here (i.e. doing just `pygeos.is_ring(data)`). But breaking the current behaviour like this, even though it is incorrect, feels strange. Maybe add a warning at least?
brendan-ward: This might be a good case for a deprecation warning (could be added in next 0.8.x release, no need to wait for 0.9), something like:
```
is_ring currently returns True for Polygons, which is not correct.  This will be corrected to False in a future release.
```

So maybe: warn of upcoming deprecation now, correct it in another release (maybe 0.9?) with a warning is called with Polygon inputs, then drop the warning later?

Hopefully given the existing docs of the underlying function in shapely will mean that not too many folks are impacted by correcting the behavior for polygons.
martinfleis: I am not sure if there will be another 0.8.x release tbh. My proposal: Use this fix, which keeps behaviour for polygons but fixes it for LineStrings and LinearRings and add a FutureWarning that the behaviour will change for Polygons. Thoughts?
brendan-ward: That is certainly reasonable.  In that case, on the `pygeos` branch of the condition, I'd suggest
```
return pygeos.is_ring(data) | pygeos.is_ring(pygeos.get_exterior_ring(data))
```
jorisvandenbossche: > I am not sure if there will be another 0.8.x release tbh. 

And even if we would do one, we should in general not add behaviour changes / new warnings to a bug-fix release, so I think it is best to keep this for 0.9 anyway.

Patch:
diff --git a/geopandas/_vectorized.py b/geopandas/_vectorized.py
index 673cb393..291a3dfc 100644
--- a/geopandas/_vectorized.py
+++ b/geopandas/_vectorized.py
@@ -474,23 +474,28 @@ def is_simple(data):
 
 
 def is_ring(data):
-    if compat.USE_PYGEOS:
-        return pygeos.is_ring(pygeos.get_exterior_ring(data))
-    else:
-        # operates on the exterior, so can't use _unary_op()
-        # XXX needed to change this because there is now a geometry collection
-        # in the shapely ones that was something else before?
-        return np.array(
-            [
-                geom.exterior.is_ring
-                if geom is not None
-                and hasattr(geom, "exterior")
-                and geom.exterior is not None
-                else False
-                for geom in data
-            ],
-            dtype=bool,
+    if "Polygon" in geom_type(data):
+        warnings.warn(
+            "is_ring currently returns True for Polygons, which is not correct. "
+            "This will be corrected to False in a future release.",
+            FutureWarning,
+            stacklevel=3,
         )
+    if compat.USE_PYGEOS:
+        return pygeos.is_ring(data) | pygeos.is_ring(pygeos.get_exterior_ring(data))
+    else:
+        # for polygons operates on the exterior, so can't use _unary_op()
+        results = []
+        for geom in data:
+            if geom is None:
+                results.append(False)
+            elif geom.type == "Polygon":
+                results.append(geom.exterior.is_ring)
+            elif geom.type in ["LineString", "LinearRing"]:
+                results.append(geom.is_ring)
+            else:
+                results.append(False)
+        return np.array(results, dtype=bool)
 
 
 def is_closed(data):


Test patch:
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
index cf1923aa..5861d903 100644
--- a/.github/workflows/tests.yaml
+++ b/.github/workflows/tests.yaml
@@ -5,6 +5,8 @@ on:
     branches: [ master ]
   pull_request:
     branches: [ master ]
+  schedule:
+    - cron:  '0 0 * * *'
 
 jobs:
   Linting:
diff --git a/geopandas/tests/test_array.py b/geopandas/tests/test_array.py
index 79d2d64f..94d7f50b 100644
--- a/geopandas/tests/test_array.py
+++ b/geopandas/tests/test_array.py
@@ -479,6 +479,22 @@ def test_unary_predicates(attr):
     assert result.tolist() == expected
 
 
+def test_is_ring():
+    g = [
+        shapely.geometry.LinearRing([(0, 0), (1, 1), (1, -1)]),
+        shapely.geometry.LineString([(0, 0), (1, 1), (1, -1)]),
+        shapely.geometry.LineString([(0, 0), (1, 1), (1, -1), (0, 0)]),
+        shapely.geometry.Polygon([(0, 0), (1, 1), (1, -1)]),
+        shapely.geometry.Polygon(),
+        None,
+    ]
+    expected = [True, False, True, True, False, False]
+
+    result = from_shapely(g).is_ring
+
+    assert result.tolist() == expected
+
+
 @pytest.mark.parametrize("attr", ["area", "length"])
 def test_unary_float(attr):
     na_value = np.nan