Repository: geopandas/geopandas
Pull request #2497 (2022-07-16 08:15:42)

REGR: fix recursion error in GeoDataFrame constructor with MultiIndex
Closes https://github.com/geopandas/geopandas/issues/2486


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 74a277ff..a3dc849d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,8 +8,17 @@ New features and improvements:
 
 Deprecations and compatibility notes:
 
+-   The `version` parameter for the `to_feather` and `to_parquet` methods has
+    been replaced with `schema_version`.  `version` will be passed directly to
+    underlying feather or parquet writer. `version` will only be used to set
+    `schema_version` if `version` is one of 0.1.0 or 0.4.0 (#2496).
+
 Bug fixes:
 
+- Fix regression (RecursionError) in reshape methods such as ``unstack()``
+  and ``pivot()`` involving MultiIndex, or GeoDataFrame construction with
+  MultiIndex (#2486).
+
 Notes on (optional) dependencies:
 
 Version 0.11 (June 20, 2022)
diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py
index 2b4bbe30..92dc5045 100644
--- a/geopandas/geodataframe.py
+++ b/geopandas/geodataframe.py
@@ -148,7 +148,11 @@ class GeoDataFrame(GeoPandasBase, DataFrame):
             if crs is not None and data.crs != crs:
                 raise ValueError(crs_mismatch_error)
 
-        if geometry is None and "geometry" in self.columns:
+        if (
+            geometry is None
+            and self.columns.nlevels == 1
+            and "geometry" in self.columns
+        ):
             # Check for multiple columns with name "geometry". If there are,
             # self["geometry"] is a gdf and constructor gets recursively recalled
             # by pandas internals trying to access this
@@ -996,7 +1000,7 @@ individually so that features may have different properties
         return df
 
     def to_parquet(
-        self, path, index=None, compression="snappy", version=None, **kwargs
+        self, path, index=None, compression="snappy", schema_version=None, **kwargs
     ):
         """Write a GeoDataFrame to the Parquet format.
 
@@ -1026,7 +1030,7 @@ individually so that features may have different properties
             output except `RangeIndex` which is stored as metadata only.
         compression : {'snappy', 'gzip', 'brotli', None}, default 'snappy'
             Name of the compression to use. Use ``None`` for no compression.
-        version : {'0.1.0', '0.4.0', None}
+        schema_version : {'0.1.0', '0.4.0', None}
             GeoParquet specification version; if not provided will default to
             latest supported version.
         kwargs
@@ -1056,10 +1060,17 @@ individually so that features may have different properties
         from geopandas.io.arrow import _to_parquet
 
         _to_parquet(
-            self, path, compression=compression, index=index, version=version, **kwargs
+            self,
+            path,
+            compression=compression,
+            index=index,
+            schema_version=schema_version,
+            **kwargs,
         )
 
-    def to_feather(self, path, index=None, compression=None, version=None, **kwargs):
+    def to_feather(
+        self, path, index=None, compression=None, schema_version=None, **kwargs
+    ):
         """Write a GeoDataFrame to the Feather format.
 
         Any geometry columns present are serialized to WKB format in the file.
@@ -1089,7 +1100,7 @@ individually so that features may have different properties
         compression : {'zstd', 'lz4', 'uncompressed'}, optional
             Name of the compression to use. Use ``"uncompressed"`` for no
             compression. By default uses LZ4 if available, otherwise uncompressed.
-        version : {'0.1.0', '0.4.0', None}
+        schema_version : {'0.1.0', '0.4.0', None}
             GeoParquet specification version; if not provided will default to
             latest supported version.
         kwargs
@@ -1110,7 +1121,12 @@ individually so that features may have different properties
         from geopandas.io.arrow import _to_feather
 
         _to_feather(
-            self, path, index=index, compression=compression, version=version, **kwargs
+            self,
+            path,
+            index=index,
+            compression=compression,
+            schema_version=schema_version,
+            **kwargs,
         )
 
     def to_file(self, filename, driver=None, schema=None, index=None, **kwargs):
diff --git a/geopandas/io/arrow.py b/geopandas/io/arrow.py
index 3e443f2d..fea924cf 100644
--- a/geopandas/io/arrow.py
+++ b/geopandas/io/arrow.py
@@ -66,13 +66,13 @@ def _remove_id_from_member_of_ensembles(json_dict):
                 member.pop("id", None)
 
 
-def _create_metadata(df, version=None):
+def _create_metadata(df, schema_version=None):
     """Create and encode geo metadata dict.
 
     Parameters
     ----------
     df : GeoDataFrame
-    version : {'0.1.0', '0.4.0', None}
+    schema_version : {'0.1.0', '0.4.0', None}
         GeoParquet specification version; if not provided will default to
         latest supported version.
 
@@ -81,10 +81,12 @@ def _create_metadata(df, version=None):
     dict
     """
 
-    version = version or METADATA_VERSION
+    schema_version = schema_version or METADATA_VERSION
 
-    if version not in SUPPORTED_VERSIONS:
-        raise ValueError(f"version must be one of: {', '.join(SUPPORTED_VERSIONS)}")
+    if schema_version not in SUPPORTED_VERSIONS:
+        raise ValueError(
+            f"schema_version must be one of: {', '.join(SUPPORTED_VERSIONS)}"
+        )
 
     # Construct metadata for each geometry
     column_metadata = {}
@@ -94,7 +96,7 @@ def _create_metadata(df, version=None):
 
         crs = None
         if series.crs:
-            if version == "0.1.0":
+            if schema_version == "0.1.0":
                 crs = series.crs.to_wkt()
             else:  # version >= 0.4.0
                 crs = series.crs.to_json_dict()
@@ -112,7 +114,7 @@ def _create_metadata(df, version=None):
     return {
         "primary_column": df._geometry_column_name,
         "columns": column_metadata,
-        "version": METADATA_VERSION,
+        "version": schema_version or METADATA_VERSION,
         "creator": {"library": "geopandas", "version": geopandas.__version__},
     }
 
@@ -224,7 +226,7 @@ def _validate_metadata(metadata):
             raise ValueError("Only WKB geometry encoding is supported")
 
 
-def _geopandas_to_arrow(df, index=None, version=None):
+def _geopandas_to_arrow(df, index=None, schema_version=None):
     """
     Helper function with main, shared logic for to_parquet/to_feather.
     """
@@ -233,7 +235,7 @@ def _geopandas_to_arrow(df, index=None, version=None):
     _validate_dataframe(df)
 
     # create geo metadata before altering incoming data frame
-    geo_metadata = _create_metadata(df, version=version)
+    geo_metadata = _create_metadata(df, schema_version=schema_version)
 
     df = df.to_wkb()
 
@@ -243,10 +245,13 @@ def _geopandas_to_arrow(df, index=None, version=None):
     # This must be done AFTER creating the table or it is not persisted
     metadata = table.schema.metadata
     metadata.update({b"geo": _encode_metadata(geo_metadata)})
+
     return table.replace_schema_metadata(metadata)
 
 
-def _to_parquet(df, path, index=None, compression="snappy", version=None, **kwargs):
+def _to_parquet(
+    df, path, index=None, compression="snappy", schema_version=None, **kwargs
+):
     """
     Write a GeoDataFrame to the Parquet format.
 
@@ -270,7 +275,7 @@ def _to_parquet(df, path, index=None, compression="snappy", version=None, **kwar
         output except `RangeIndex` which is stored as metadata only.
     compression : {'snappy', 'gzip', 'brotli', None}, default 'snappy'
         Name of the compression to use. Use ``None`` for no compression.
-    version : {'0.1.0', '0.4.0', None}
+    schema_version : {'0.1.0', '0.4.0', None}
         GeoParquet specification version; if not provided will default to
         latest supported version.
     kwargs
@@ -280,12 +285,23 @@ def _to_parquet(df, path, index=None, compression="snappy", version=None, **kwar
         "pyarrow.parquet", extra="pyarrow is required for Parquet support."
     )
 
+    if kwargs and "version" in kwargs and kwargs["version"] is not None:
+        if schema_version is None and kwargs["version"] in SUPPORTED_VERSIONS:
+            warnings.warn(
+                "the `version` parameter has been replaced with `schema_version`. "
+                "`version` will instead be passed directly to the underlying "
+                "parquet writer unless `version` is 0.1.0 or 0.4.0.",
+                FutureWarning,
+                stacklevel=2,
+            )
+            schema_version = kwargs.pop("version")
+
     path = _expand_user(path)
-    table = _geopandas_to_arrow(df, index=index, version=version)
+    table = _geopandas_to_arrow(df, index=index, schema_version=schema_version)
     parquet.write_table(table, path, compression=compression, **kwargs)
 
 
-def _to_feather(df, path, index=None, compression=None, version=None, **kwargs):
+def _to_feather(df, path, index=None, compression=None, schema_version=None, **kwargs):
     """
     Write a GeoDataFrame to the Feather format.
 
@@ -310,7 +326,7 @@ def _to_feather(df, path, index=None, compression=None, version=None, **kwargs):
     compression : {'zstd', 'lz4', 'uncompressed'}, optional
         Name of the compression to use. Use ``"uncompressed"`` for no
         compression. By default uses LZ4 if available, otherwise uncompressed.
-    version : {'0.1.0', '0.4.0', None}
+    schema_version : {'0.1.0', '0.4.0', None}
         GeoParquet specification version; if not provided will default to
         latest supported version.
     kwargs
@@ -325,8 +341,19 @@ def _to_feather(df, path, index=None, compression=None, version=None, **kwargs):
     if Version(pyarrow.__version__) < Version("0.17.0"):
         raise ImportError("pyarrow >= 0.17 required for Feather support")
 
+    if kwargs and "version" in kwargs and kwargs["version"] is not None:
+        if schema_version is None and kwargs["version"] in SUPPORTED_VERSIONS:
+            warnings.warn(
+                "the `version` parameter has been replaced with `schema_version`. "
+                "`version` will instead be passed directly to the underlying "
+                "feather writer unless `version` is 0.1.0 or 0.4.0.",
+                FutureWarning,
+                stacklevel=2,
+            )
+            schema_version = kwargs.pop("version")
+
     path = _expand_user(path)
-    table = _geopandas_to_arrow(df, index=index, version=version)
+    table = _geopandas_to_arrow(df, index=index, schema_version=schema_version)
     feather.write_feather(table, path, compression=compression, **kwargs)
 
 
@@ -337,6 +364,7 @@ def _arrow_to_geopandas(table, metadata=None):
     df = table.to_pandas()
 
     metadata = metadata or table.schema.metadata
+
     if metadata is None or b"geo" not in metadata:
         raise ValueError(
             """Missing geo metadata in Parquet/Feather file.


Test patch:
diff --git a/geopandas/io/tests/test_arrow.py b/geopandas/io/tests/test_arrow.py
index 62657b99..ffe889aa 100644
--- a/geopandas/io/tests/test_arrow.py
+++ b/geopandas/io/tests/test_arrow.py
@@ -105,8 +105,8 @@ def test_crs_metadata_datum_ensemble():
 
 def test_write_metadata_invalid_spec_version():
     gdf = geopandas.GeoDataFrame(geometry=[box(0, 0, 10, 10)], crs="EPSG:4326")
-    with pytest.raises(ValueError, match="version must be one of"):
-        _create_metadata(gdf, version="invalid")
+    with pytest.raises(ValueError, match="schema_version must be one of"):
+        _create_metadata(gdf, schema_version="invalid")
 
 
 def test_encode_metadata():
@@ -259,7 +259,7 @@ def test_to_parquet_does_not_pass_engine_along(mock_to_parquet):
     # assert that engine keyword is not passed through to _to_parquet (and thus
     # parquet.write_table)
     mock_to_parquet.assert_called_with(
-        df, "", compression="snappy", index=None, version=None
+        df, "", compression="snappy", index=None, schema_version=None
     )
 
 
@@ -679,9 +679,10 @@ def test_write_read_default_crs(tmpdir, format):
 
 
 @pytest.mark.parametrize(
-    "format,version", product(["feather", "parquet"], [None] + SUPPORTED_VERSIONS)
+    "format,schema_version",
+    product(["feather", "parquet"], [None] + SUPPORTED_VERSIONS),
 )
-def test_write_spec_version(tmpdir, format, version):
+def test_write_spec_version(tmpdir, format, schema_version):
     if format == "feather":
         from pyarrow.feather import read_table
 
@@ -691,7 +692,7 @@ def test_write_spec_version(tmpdir, format, version):
     filename = os.path.join(str(tmpdir), f"test.{format}")
     gdf = geopandas.GeoDataFrame(geometry=[box(0, 0, 10, 10)], crs="EPSG:4326")
     write = getattr(gdf, f"to_{format}")
-    write(filename, version=version)
+    write(filename, schema_version=schema_version)
 
     # ensure that we can roundtrip data regardless of version
     read = getattr(geopandas, f"read_{format}")
@@ -700,10 +701,10 @@ def test_write_spec_version(tmpdir, format, version):
 
     table = read_table(filename)
     metadata = json.loads(table.schema.metadata[b"geo"])
-    assert metadata["version"] == version or METADATA_VERSION
+    assert metadata["version"] == schema_version or METADATA_VERSION
 
     # verify that CRS is correctly handled between versions
-    if version == "0.1.0":
+    if schema_version == "0.1.0":
         assert metadata["columns"]["geometry"]["crs"] == gdf.crs.to_wkt()
 
     else:
@@ -712,6 +713,46 @@ def test_write_spec_version(tmpdir, format, version):
         assert metadata["columns"]["geometry"]["crs"] == crs_expected
 
 
+@pytest.mark.parametrize(
+    "format,version", product(["feather", "parquet"], [None] + SUPPORTED_VERSIONS)
+)
+def test_write_deprecated_version_parameter(tmpdir, format, version):
+    if format == "feather":
+        from pyarrow.feather import read_table
+
+        version = version or 2
+
+    else:
+        from pyarrow.parquet import read_table
+
+        version = version or "2.6"
+
+    filename = os.path.join(str(tmpdir), f"test.{format}")
+    gdf = geopandas.GeoDataFrame(geometry=[box(0, 0, 10, 10)], crs="EPSG:4326")
+    write = getattr(gdf, f"to_{format}")
+
+    if version in SUPPORTED_VERSIONS:
+        with pytest.warns(
+            FutureWarning,
+            match="the `version` parameter has been replaced with `schema_version`",
+        ):
+            write(filename, version=version)
+
+    else:
+        # no warning raised if not one of the captured versions
+        write(filename, version=version)
+
+    table = read_table(filename)
+    metadata = json.loads(table.schema.metadata[b"geo"])
+
+    if version in SUPPORTED_VERSIONS:
+        # version is captured as a parameter
+        assert metadata["version"] == version
+    else:
+        # version is passed to underlying writer
+        assert metadata["version"] == METADATA_VERSION
+
+
 @pytest.mark.parametrize("version", ["0.1.0", "0.4.0"])
 def test_read_versioned_file(version):
     """
diff --git a/geopandas/tests/test_geodataframe.py b/geopandas/tests/test_geodataframe.py
index a1a02ae2..3cff5104 100644
--- a/geopandas/tests/test_geodataframe.py
+++ b/geopandas/tests/test_geodataframe.py
@@ -1220,6 +1220,25 @@ class TestConstructor:
         with pytest.raises(ValueError):
             GeoDataFrame(df3, geometry="geom")
 
+    @pytest.mark.parametrize("dtype", ["geometry", "object"])
+    def test_multiindex_with_geometry_label(self, dtype):
+        # DataFrame with MultiIndex where "geometry" label corresponds to
+        # multiple columns
+        df = pd.DataFrame([[Point(0, 0), Point(1, 1)], [Point(2, 2), Point(3, 3)]])
+        df = df.astype(dtype)
+        df.columns = pd.MultiIndex.from_product([["geometry"], [0, 1]])
+        # don't error in constructor
+        gdf = GeoDataFrame(df)
+        # Getting the .geometry column gives GeoDataFrame for both columns
+        # (but with first MultiIndex level removed)
+        # TODO should this give an error instead?
+        result = gdf.geometry
+        assert result.shape == gdf.shape
+        assert result.columns.tolist() == [0, 1]
+        assert_frame_equal(result, gdf["geometry"])
+        result = gdf[["geometry"]]
+        assert_frame_equal(result, gdf if dtype == "geometry" else pd.DataFrame(gdf))
+
 
 def test_geodataframe_crs():
     gdf = GeoDataFrame(columns=["geometry"])
diff --git a/geopandas/tests/test_op_output_types.py b/geopandas/tests/test_op_output_types.py
index 3a4155c7..5a447f39 100644
--- a/geopandas/tests/test_op_output_types.py
+++ b/geopandas/tests/test_op_output_types.py
@@ -289,6 +289,11 @@ def test_expanddim_in_unstack():
     else:  # pandas GH37369, unstack doesn't call finalize
         assert unstack._geometry_column_name == "geometry"
 
+    # https://github.com/geopandas/geopandas/issues/2486
+    s.name = "geometry"
+    unstack = s.unstack()
+    assert_object(unstack, GeoDataFrame, None, None)
+
 
 # indexing /  constructor_sliced tests
 
diff --git a/geopandas/tests/test_pandas_methods.py b/geopandas/tests/test_pandas_methods.py
index 252af1d1..678b5a09 100644
--- a/geopandas/tests/test_pandas_methods.py
+++ b/geopandas/tests/test_pandas_methods.py
@@ -657,6 +657,16 @@ def test_df_apply_returning_series(df):
     assert_series_equal(result, df["value1"].rename(None))
 
 
+def test_pivot(df):
+    # https://github.com/geopandas/geopandas/issues/2057
+    # pivot failing due to creating a MultiIndex
+    result = df.pivot(columns="value1")
+    expected = GeoDataFrame(pd.DataFrame(df).pivot(columns="value1"))
+    # TODO assert_geodataframe_equal crashes
+    assert isinstance(result, GeoDataFrame)
+    assert_frame_equal(result, expected)
+
+
 def test_preserve_attrs(df):
     # https://github.com/geopandas/geopandas/issues/1654
     df.attrs["name"] = "my_name"