Repository: geopandas/geopandas
Pull request #3505 (2025-02-07 08:20:09)

Do not hardcode GeoDataFrame for easier subclassing
At the moment, subclassing ``GeoDataFrame`` requires to patch a few more functions than just ``_constructor`` so that the new object does not switch back to ``GeoDataFrame`` after several operations like ``__getitem__``, ``.loc`` or ``copy``.

In the spirit of the previous discussion and PR on the same topic (#2750), I'd propose to use the object class/method rather than directly ``GeoDataFrame`` in the affected functions.

tfardet: I don't think the failing test is related to the PR, it seems like an issue with folium/graphics but let me know if I missed something
martinfleis: Failure is unrelated #3497.

I suppose the changes are fine. I know it is not that straightforward but could you try adding some tests for it? Otherwise we can't promise that it won't unintentionally break again.
tfardet: Thank you both, and I understand that subclassing is not a 1st-class use case and that's of course fine ;)
I guess I can add a test with a subclass and check that the child class is preserved by the operations I mentioned.
tfardet: > I know it is not that straightforward but could you try adding some tests for it? Otherwise we can't promise that it won't unintentionally break again.

I did, let me know if these are fine with you :)
tfardet: @m-richards thanks for the feedback, I made some slight changes, let me know if that's fine
tfardet: @m-richards I moved the function to ``classmethod``, I think the overhead should be fairly minimal, let me know if this is good enough
tfardet: Sorry, I went a bit fast instead of making sure everything was consistent. 
If the ``classmethod`` function is deemed acceptable, I believe that the current implementation solves #3517 completely and does not require to overload ``_constructor`` in child classes anymore.
tfardet: > I think it's reasonable to defer that to deal with seperately when (if?) needed

that would be great, I'm afraid I don't have the bandwidth to do more at the moment

Thanks for the review and discussions, if this is good with you, you can squash as soon as the tests pass
martinfleis: pygeoapi failures in tests are not related. pre-commit failure is, can you fix that one?
tfardet: > pygeoapi failures in tests are not related. pre-commit failure is, can you fix that one?

done, weird, though: the pre-commit was installed...
martinfleis: > the pre-commit was installed...

That usually comes from accepting code suggestions in this case.
tfardet: > That usually comes from accepting code suggestions in this case.

not in this case, but maybe I started making change before the pre-commit was installed and did not run it on all files retrospectively.

Anyway, all tests are passing now, so we're all set!


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1490ea78..c4adc380 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ New features and improvements:
 - Add ``grid_size`` parameter to ``union_all`` and ``dissolve`` (#3445).
 - `GeoDataFrame.plot` now supports `pd.Index` as an input for the `column` keyword (#3463).
 - Avoid change of the plot aspect when plotting missing values (#3438).
+- GeoDataFrame no longer hard-codes the class internally, allowing easier subclassing (#3505).
 - Improve performance of `overlay` with `how=identity` (#3504).
 - Fix ambiguous error when GeoDataFrame is initialised with a column called "crs" (#3502)
 
diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py
index a6bc893f..1c98bc99 100644
--- a/geopandas/geodataframe.py
+++ b/geopandas/geodataframe.py
@@ -43,22 +43,6 @@ if typing.TYPE_CHECKING:
     )
 
 
-def _geodataframe_constructor_with_fallback(
-    *args, **kwargs
-) -> pd.DataFrame | GeoDataFrame:
-    """
-    A flexible constructor for GeoDataFrame._constructor, which falls back
-    to returning a DataFrame (if a certain operation does not preserve the
-    geometry column)
-    """
-    df = GeoDataFrame(*args, **kwargs)
-    geometry_cols_mask = df.dtypes == "geometry"
-    if len(geometry_cols_mask) == 0 or geometry_cols_mask.sum() == 0:
-        df = pd.DataFrame(df)
-
-    return df
-
-
 def _ensure_geometry(data, crs: Any | None = None) -> GeoSeries | GeometryArray:
     """
     Ensure the data is of geometry dtype or converted to it.
@@ -1924,7 +1908,7 @@ default 'snappy'
             result.__class__ = GeoSeries
         elif isinstance(result, DataFrame):
             if (result.dtypes == "geometry").sum() > 0:
-                result.__class__ = GeoDataFrame
+                result.__class__ = type(self)
                 if geo_col in result:
                     result._geometry_column_name = geo_col
             else:
@@ -1986,7 +1970,7 @@ default 'snappy'
     def copy(self, deep: bool = True) -> GeoDataFrame:
         copied = super().copy(deep=deep)
         if type(copied) is pd.DataFrame:
-            copied.__class__ = GeoDataFrame
+            copied.__class__ = type(self)
             copied._geometry_column_name = self._geometry_column_name
         return copied
 
@@ -2030,18 +2014,36 @@ default 'snappy'
 
         return result
 
+    @classmethod
+    def _geodataframe_constructor_with_fallback(
+        cls, *args, **kwargs
+    ) -> pd.DataFrame | GeoDataFrame:
+        """
+        A flexible constructor for GeoDataFrame._constructor, which falls back
+        to returning a DataFrame (if a certain operation does not preserve the
+        geometry column)
+        """
+        df = cls(*args, **kwargs)
+
+        geometry_cols_mask = df.dtypes == "geometry"
+
+        if len(geometry_cols_mask) == 0 or geometry_cols_mask.sum() == 0:
+            df = pd.DataFrame(df)
+
+        return df
+
     @property
     def _constructor(self) -> DataFrame | GeoDataFrame:
-        return _geodataframe_constructor_with_fallback
+        return self._geodataframe_constructor_with_fallback
 
     def _constructor_from_mgr(self, mgr, axes) -> DataFrame | GeoDataFrame:
         # replicate _geodataframe_constructor_with_fallback behaviour
         # unless safe to skip
         if not any(isinstance(block.dtype, GeometryDtype) for block in mgr.blocks):
-            return _geodataframe_constructor_with_fallback(
+            return self._geodataframe_constructor_with_fallback(
                 pd.DataFrame._from_mgr(mgr, axes)
             )
-        gdf = GeoDataFrame._from_mgr(mgr, axes)
+        gdf = self._from_mgr(mgr, axes)
         # _from_mgr doesn't preserve metadata (expect __finalize__ to be called)
         # still need to mimic __init__ behaviour with geometry=None
         if (gdf.columns == "geometry").sum() == 1:  # only if "geometry" is single col
@@ -2261,7 +2263,7 @@ default 'snappy'
         )
 
         # Aggregate
-        aggregated_geometry = GeoDataFrame(g, geometry=self.geometry.name, crs=self.crs)
+        aggregated_geometry = type(self)(g, geometry=self.geometry.name, crs=self.crs)
         # Recombine
         aggregated = aggregated_geometry.join(aggregated_data)
 


Test patch:
diff --git a/geopandas/tests/test_geodataframe.py b/geopandas/tests/test_geodataframe.py
index ecbedf2e..a9b7ac04 100644
--- a/geopandas/tests/test_geodataframe.py
+++ b/geopandas/tests/test_geodataframe.py
@@ -1669,3 +1669,40 @@ def test_reduce_geometry_array():
     This warning is issued with pandas 2.2.2 (tested).
     """
     GeoDataFrame({"geometry": []}).all()
+
+
+class GDFChild(GeoDataFrame):
+    def custom_method(self):
+        return "this is a custom output"
+
+
+def test_inheritance(dfs):
+    df, _ = dfs
+    df.loc[:, "col2"] = ["a"] * len(df)
+
+    dfc = GDFChild(df)
+
+    dfc2 = dfc.rename_geometry("geometry2")
+
+    children = [
+        dfc,
+        dfc.iloc[[0]],
+        dfc.loc[dfc.col1 == 1],
+        dfc.dissolve(),
+        dfc[["col2", "geometry"]],
+        dfc.copy(),
+        dfc2,
+        dfc2.iloc[[0]],
+        dfc2.loc[dfc.col1 == 1],
+        dfc2.dissolve(),
+        dfc2[["col2", "geometry2"]],
+        dfc2.copy(),
+    ]
+
+    for v in children:
+        assert isinstance(v, GDFChild)
+        assert v.custom_method() == "this is a custom output"
+
+    df2 = dfc2.drop(columns=["geometry2"])
+
+    assert not isinstance(df2, GDFChild)