Repository: geopandas/geopandas
Pull request #2541 (2022-08-28 10:22:16)

ENH: Added minimum_rotated_rectangle from shapely
part of #2010

SimoParmeg: Hi, for the sake of having a clean PR, is it ok to you if I create a new PR from scratch?
jorisvandenbossche: > Hi, for the sake of having a clean PR, is it ok to you if I create a new PR from scratch?

@SimoParmeg no need for that. The diff itself is clean, and when merging the PR on github, we do so by squashing all commits into one anyway. 
jorisvandenbossche: @SimoParmeg there were a few files added (images), I suppose by accident. Can you remove those again from the branch?
martinfleis: I have updated this PR and included review comments. 

@jorisvandenbossche One question here - which API name do we want to follow? `minimum_rotated_rectangle` coming from older shapely or `oriented_envelope`? I know that they are equal in shapely 2 but I would pick only one in geopandas and am not sure which one to follow here. I am personally fine with the rectangle version but would be equally fine with envelope... Just for a context, QGIS call it "Oriented minimum bounding box" to add a bit of additional confusion to the mix :D.

We can say that we want to follow what was in the shapely ecosystem available until now and call it `minimum_rotated_rectangle` but if there are plans in shapely to keep only one of the two eventually, it would be good to pick the that one.
martinfleis: Ah, one more thing to consider - shapely has `minimum_rotated_rectangle` as a property. Until now, we've been following their distinction between property and method, though it can be weird sometimes (like this time). Do we want this to be a property for consistency? I have to admit that a method feel more _right_ here.
jorisvandenbossche: > which API name do we want to follow? `minimum_rotated_rectangle` coming from older shapely or `oriented_envelope`?

Good question: in shapely we have the general guideline of following PostGIS naming where appropriate/possible (well, that was the rule we started to use in pygeos), and so that is the reason that the `oriented_envelope` name was added (and `minimum_rotated_rectangle` kept in shapely for backwards compatibility). 

Since in geopandas it is added as new functionality, we should then maybe rather go with `oriented_envelope`? 
(although personally I actually find `minimum_rotated_rectangle` more descriptive ...)

> shapely has `minimum_rotated_rectangle` as a property. Until now, we've been following their distinction between property and method, though it can be weird sometimes (like this time). Do we want this to be a property for consistency? I have to admit that a method feel more _right_ here.

Yes, I fully agree that the distinction in shapely isn't always great. Things like this which is clearly calculating a new geometry (and not a simple "characteristic") should be a method. But that's also very hard (impossible?) to change in shapely (except for new methods). 
If we are going to add a bunch of new properies/methods from shapely to geopandas, we _could_ make a better decision here. But that of course also creates inconsistencies .. which is not great either. Maybe that ship has sailed?
m-richards: >If we are going to add a bunch of new properies/methods from shapely to geopandas, we _could_ make a better decision here. But that of course also creates inconsistencies .. which is not great either. Maybe that ship has (unfortunately) sailed?

Just thought I'd chime in quickly, with a perhaps narrow focused perspective. I had a quick scan through our docs, we say that shapely is used to perform geometry operations, but not necessarily that we implement an api equivalent to shapely. Certainly there will be people that work with shapely first, or work with both geopandas and shapely simultaneously and consistency might benefit them (but also think that autocomplete in most editors would take care of that anyway), but there will also be people using GeoPandas first and if we have the chance to change something that feels weird in the api to improve internal consistency then that seems beneficial.

Whatever we decide though, it would be good to have an explicit rationale on what makes something a method and what makes something a property that is clearly documented somewhere, I guess that is trivial if we just do exactly what shapely does.


martinfleis: I personally wouldn't replicate suboptimal design choices just for the sake of consistency. Properties should be things that are either retrieving a value stored internally or that are nearly zero-cost to compute. MBR is neither, so I would prefer to have it as a method. Especially if shapely decides to default to the _old_ Python implementation as default, it would be a costly call.

Not sure about names. If shapely is not planning to sunset `minimum_rotated_rectangle` in favour of `oriented_envelope`, it is not that we are inconsistent, we're just choosing to implement only one of the options. I don't really mind which of them we'll use, some users will expect one other another. If you're coming from QGIS you are used to "oriented minimum bounding box", if from PostGIS "oriented envelope" if from R (sf) "minimum rotated rectangle", so whichever we use someone will not expect it. If shapely strongly prefers `oriented_envelope`, let's do that.
martinfleis: The decision on the API made during the last dev call is to expose this as a method called `minimum_rotated_rectangle`. We can potentially add alias to `oriented_envelope` if users will ask for it in future. It should not be a property in any case.
martinfleis: @SimoParmeg there's a test [failure](https://github.com/geopandas/geopandas/actions/runs/5562613811/jobs/10161038651?pr=2541#step:5:4474) in an env shapely dev that is built against GEOS 3.12, that fixed the underlying algorithm. Can you adapt our tests to accommodate that?

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 271c0df6..18dd96ef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ New methods:
 - Added ``concave_hull`` method from shapely to GeoSeries/GeoDataframe (#2903).
 - Added ``offset_curve`` method from shapely to GeoSeries/GeoDataframe (#2902).
 - Added ``shortest_line`` method from shapely to GeoSeries/GeoDataframe (#2960).
+- Added ``minimum_rotated_rectangle`` method from shapely to GeoSeries/GeoDataframe (#2541).
 
 New features and improvements:
 
diff --git a/doc/source/docs/reference/geoseries.rst b/doc/source/docs/reference/geoseries.rst
index 0b9872ca..9e55f779 100644
--- a/doc/source/docs/reference/geoseries.rst
+++ b/doc/source/docs/reference/geoseries.rst
@@ -95,6 +95,7 @@ Constructive methods and attributes
    GeoSeries.extract_unique_points
    GeoSeries.make_valid
    GeoSeries.minimum_bounding_circle
+   GeoSeries.minimum_rotated_rectangle
    GeoSeries.normalize
    GeoSeries.sample_points
    GeoSeries.segmentize
diff --git a/geopandas/_vectorized.py b/geopandas/_vectorized.py
index 9924dc60..9d5cafdf 100644
--- a/geopandas/_vectorized.py
+++ b/geopandas/_vectorized.py
@@ -658,6 +658,15 @@ def envelope(data):
         return _unary_geo("envelope", data)
 
 
+def minimum_rotated_rectangle(data):
+    if compat.USE_SHAPELY_20:
+        return shapely.oriented_envelope(data)
+    elif compat.USE_PYGEOS:
+        return pygeos.oriented_envelope(data)
+    else:
+        return _unary_geo("minimum_rotated_rectangle", data)
+
+
 def exterior(data):
     if compat.USE_SHAPELY_20:
         return shapely.get_exterior_ring(data)
diff --git a/geopandas/array.py b/geopandas/array.py
index e1892bab..f528c791 100644
--- a/geopandas/array.py
+++ b/geopandas/array.py
@@ -529,6 +529,11 @@ class GeometryArray(ExtensionArray):
     def envelope(self):
         return GeometryArray(vectorized.envelope(self._data), crs=self.crs)
 
+    def minimum_rotated_rectangle(self):
+        return GeometryArray(
+            vectorized.minimum_rotated_rectangle(self.data), crs=self.crs
+        )
+
     @property
     def exterior(self):
         return GeometryArray(vectorized.exterior(self._data), crs=self.crs)
diff --git a/geopandas/base.py b/geopandas/base.py
index fd8cc1b0..1c12d4a3 100644
--- a/geopandas/base.py
+++ b/geopandas/base.py
@@ -697,6 +697,46 @@ GeometryCollection
         """
         return _delegate_property("envelope", self)
 
+    def minimum_rotated_rectangle(self):
+        """Returns a ``GeoSeries`` of the general minimum bounding rectangle
+        that contains the object.
+
+        Unlike envelope this rectangle is not constrained to be parallel
+        to the coordinate axes. If the convex hull of the object is a
+        degenerate (line or point) this degenerate is returned.
+
+        Examples
+        --------
+
+        >>> from shapely.geometry import Polygon, LineString, Point, MultiPoint
+        >>> s = geopandas.GeoSeries(
+        ...     [
+        ...         Polygon([(0, 0), (1, 1), (0, 1)]),
+        ...         LineString([(0, 0), (1, 1), (1, 0)]),
+        ...         MultiPoint([(0, 0), (1, 1)]),
+        ...         Point(0, 0),
+        ...     ]
+        ... )
+        >>> s
+        0    POLYGON ((0.00000 0.00000, 1.00000 1.00000, 0....
+        1    LINESTRING (0.00000 0.00000, 1.00000 1.00000, ...
+        2        MULTIPOINT (0.00000 0.00000, 1.00000 1.00000)
+        3                              POINT (0.00000 0.00000)
+        dtype: geometry
+
+        >>> s.minimum_rotated_rectangle()
+        0    POLYGON ((1.00000 1.00000, 0.50000 1.50000, -0...
+        1    POLYGON ((0.00000 0.00000, 0.50000 -0.50000, 1...
+        2        LINESTRING (0.00000 0.00000, 1.00000 1.00000)
+        3                              POINT (0.00000 0.00000)
+        dtype: geometry
+
+        See also
+        --------
+        GeoSeries.envelope : bounding rectangle
+        """
+        return _delegate_geo_method("minimum_rotated_rectangle", self)
+
     @property
     def exterior(self):
         """Returns a ``GeoSeries`` of LinearRings representing the outer


Test patch:
diff --git a/geopandas/tests/test_geom_methods.py b/geopandas/tests/test_geom_methods.py
index b66ce57b..731cf42f 100644
--- a/geopandas/tests/test_geom_methods.py
+++ b/geopandas/tests/test_geom_methods.py
@@ -1223,6 +1223,20 @@ class TestGeomMethods:
         assert isinstance(e, GeoSeries)
         assert self.g3.crs == e.crs
 
+    def test_minimum_rotated_rectangle(self):
+        s = GeoSeries([self.sq, self.t5], crs=3857)
+        r = s.minimum_rotated_rectangle()
+        exp = GeoSeries.from_wkt(
+            [
+                "POLYGON ((0 0, 0 1, 1 1, 1 0, 0 0))",
+                "POLYGON ((2 0, 2 3, 3 3, 3 0, 2 0))",
+            ]
+        )
+
+        assert np.all(r.normalize().geom_equals_exact(exp, 0.001))
+        assert isinstance(r, GeoSeries)
+        assert s.crs == r.crs
+
     @pytest.mark.skipif(
         not (compat.USE_PYGEOS or compat.USE_SHAPELY_20),
         reason=(