Repository: geopandas/geopandas
Pull request #2950 (2023-07-04 03:17:16)

BUG: Infer schema for np.int32 and pd.Int32Dtype columns properly
Let columns of dtype `np.int32` (non-nullable) and `pandas.Int32Dtype()` (nullable) be inferred as int32 columns instead of int(64). Needed for the `OGR_GMT` driver.

Related:
- https://github.com/GenericMappingTools/pygmt/pull/2592
- https://github.com/geopandas/geopandas/issues/967#issuecomment-842999302

Similar PRs in the past:
- https://github.com/geopandas/geopandas/pull/2265
- https://github.com/geopandas/geopandas/pull/2464

m-richards: Just to add some context, a fix to allow writing int32s as int32s (pandas nullable dtype or otherwise) is certainly welcome. #2464 stalled because it also tried to adjust the schema for tab files such that all integer fields were written as int32s - which is not ideal if you have int64 values outside the int32 range (this auto converting is something that could be revisited as a follow up). It doesn't seem like that is a problem for your use case, I understand from the linked issue OGR_GMT only supports 32 bit dtypes.
weiji14: > a fix to allow writing int32s as int32s (pandas nullable dtype or otherwise) is certainly welcome

Thanks @m-richards for the context! Done in 6859d6c1ebc2410d6e44141eb0b04ff1476c8a49, so I'll mark this PR ready for review. Happy to add more unit tests if needed :smiley:

You're right that we just need the auto-schema detection to output int32 when either a nullable or non-nullable int32 column is used. It doesn't look like the [`_to_file`](https://github.com/geopandas/geopandas/blob/55fa5134dd0e74388209641807ee1c4aa572967d/geopandas/io/file.py#L452-L549) function does much in terms of driver-specific dtype casting, and it's probably best not to introduce too much driver-specific logic there, so we'll probably workaround the int64 cast to int32 separately in PyGMT at https://github.com/GenericMappingTools/pygmt/pull/2592.
seisman: > You're right that we just need the auto-schema detection to output int32 when either a nullable or non-nullable int32 column is used. It doesn't look like the [`_to_file`](https://github.com/geopandas/geopandas/blob/55fa5134dd0e74388209641807ee1c4aa572967d/geopandas/io/file.py#L452-L549) function does much in terms of driver-specific dtype casting, and it's probably best not to introduce too much driver-specific logic there, so we'll probably workaround the int64 cast to int32 separately in PyGMT at [GenericMappingTools/pygmt#2592](https://github.com/GenericMappingTools/pygmt/pull/2592).

As I understand it, Fiona is the low-level library that supports different drivers? Does it makes more sense that Fiona should do driver-specific casting (e.g., `int64` to `int32` for OGR_GMT)?
weiji14: > > You're right that we just need the auto-schema detection to output int32 when either a nullable or non-nullable int32 column is used. It doesn't look like the [`_to_file`](https://github.com/geopandas/geopandas/blob/55fa5134dd0e74388209641807ee1c4aa572967d/geopandas/io/file.py#L452-L549) function does much in terms of driver-specific dtype casting, and it's probably best not to introduce too much driver-specific logic there, so we'll probably workaround the int64 cast to int32 separately in PyGMT at [GenericMappingTools/pygmt#2592](https://github.com/GenericMappingTools/pygmt/pull/2592).
> 
> As I understand it, Fiona is the low-level library that supports different drivers? Does it makes more sense that Fiona should do driver-specific casting (e.g., `int64` to `int32` for OGR_GMT)?

From https://github.com/Toblerity/Fiona/issues/1277#issuecomment-1613427767, it seems that fiona will only follow what GDAL is doing, so we might need to make the change in GDAL instead?
seisman: > so we might need to make the change in GDAL instead?

Casting int64 to int32 may cause problems if the data is out of the int32 range, so what GDAL does (converting int64 to string) makes sense to me. Maybe we can ask GDAL to make OGR_GMT support int64 types.


weiji14: > > so we might need to make the change in GDAL instead?
> 
> Casting int64 to int32 may cause problems if the data is out of the int32 range, so what GDAL does (converting int64 to string) makes sense to me. Maybe we can ask GDAL to make OGR_GMT support int64 types.

Ok, do we need to add support in GDAL for integer64 before GMT C can handle int64 columns from OGR_GMT files, or the other way round? I see this sentence at https://github.com/GenericMappingTools/gmt/blob/6.4.0/doc/rst/source/cookbook/ogrgmt-format.rst#declaration-of-aspatial-fields:

> Available datatypes should largely follow the shapefile (DB3) specification, including string, integer, double, datetime, and logical (boolean). In OGR/GMT vector files, they will be stored as appropriately formatted text strings.
>
> An example header record containing all these is
>
> ```
> # @VGMT1.0 @GPOLYGON @Nname|depth|id @Tstring|double|integer
> ```

Referring back to https://github.com/OSGeo/gdal/blob/6e9103bd5acb1ff8da305c4e77fa30335ffa3b70/ogr/ogrsf_frmts/gmt/ogrgmtlayer.cpp#L754-L773, the double refers to Real values (floating point), whereas integer is int32 only, we would need to use integer64 instead.

Alternatively, I saw at https://github.com/GenericMappingTools/gmt/pull/3296#discussion_r422554772 that there was a warning like this:

```
Warning 1: The output driver does not seem to natively support Integer64 type for field FID. Converting it to Real instead. -mapFieldType can be used to control field type conversion.
```
So maybe we could also convert int64 values to float64 in GMT?

EIther way, this PR should still be helpful for other drivers like Mapinfo TAB files.
weiji14: > I think it would be good to add a test to `test_file` to write a file and see how the int32 dtypes are round-tripped for a couple of drivers (perhaps, gpkg, geojson, gmt).
> 
> I don't think it's necessary that the round trip is one-to-one (that would be ideal), but it would be nice to still have a test showing the current limitations if any.

Sorry for the late reply, got caught up with some other stuff. I've added a test for writing to/from shp/gpkg/geojson/gmt files at dd6d5ad69c8b62a3d5aebb8ed0b2bf9482a4a2c1, see if that's ok.
m-richards: > Sorry for the late reply, got caught up with some other stuff. 

No worries at all, thanks for coming back to revisit this!

>I've added a test for writing to/from shp/gpkg/geojson/gmt files at [dd6d5ad](https://github.com/geopandas/geopandas/commit/dd6d5ad69c8b62a3d5aebb8ed0b2bf9482a4a2c1), see if that's ok.

Looks great, thanks for that. I hope you don't mind, I'm just going to push a small change - I've just checked locally and can see that we should be able to enable pyogrio for these tests, and moreover pyogrio will route trip the int32 dtype if there are no nulls.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 14041dff..63689cb2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,6 +22,7 @@ Bug fixes:
 - ``assert_geodataframe_equal`` now handles GeoDataFrames with no active geometry (#2498)
 - Fix bug in `apply` with `axis=1` where the given user defined function returns nested 
   data in the geometry column (#2959)
+- Properly infer schema for np.int32 and pd.Int32Dtype columns (#2950)
 
 ## Version 0.13.2 (Jun 6, 2023)
 
diff --git a/doc/source/getting_started/install.rst b/doc/source/getting_started/install.rst
index 60531708..23883b41 100644
--- a/doc/source/getting_started/install.rst
+++ b/doc/source/getting_started/install.rst
@@ -270,4 +270,4 @@ More specifically, whether the speedups are used or not is determined by:
 
 .. _packaging: https://packaging.pypa.io/en/latest/
 
-.. _pointpats: https://pointpats.readthedocs.io/en/latest/
\ No newline at end of file
+.. _pointpats: https://pysal.org/pointpats/
diff --git a/geopandas/geodataframe.py b/geopandas/geodataframe.py
index 41991a7f..1224a061 100644
--- a/geopandas/geodataframe.py
+++ b/geopandas/geodataframe.py
@@ -1705,9 +1705,12 @@ individually so that features may have different properties
 
         Parameters
         ----------
-        by : string, default None
-            Column whose values define groups to be dissolved. If None,
-            whole GeoDataFrame is considered a single group.
+        by : str or list-like, default None
+            Column(s) whose values define the groups to be dissolved. If None,
+            the entire GeoDataFrame is considered as a single group. If a list-like
+            object is provided, the values in the list are treated as categorical
+            labels, and polygons will be combined based on the equality of
+            these categorical labels.
         aggfunc : function or string, default "first"
             Aggregation function for manipulation of data associated
             with each group. Passed to pandas `groupby.agg` method.
diff --git a/geopandas/io/file.py b/geopandas/io/file.py
index 8832e402..2e0aa151 100644
--- a/geopandas/io/file.py
+++ b/geopandas/io/file.py
@@ -635,7 +635,13 @@ def infer_schema(df):
     from collections import OrderedDict
 
     # TODO: test pandas string type and boolean type once released
-    types = {"Int64": "int", "string": "str", "boolean": "bool"}
+    types = {
+        "Int32": "int32",
+        "int32": "int32",
+        "Int64": "int",
+        "string": "str",
+        "boolean": "bool",
+    }
 
     def convert_type(column, in_type):
         if in_type == object:


Test patch:
diff --git a/geopandas/io/tests/test_file.py b/geopandas/io/tests/test_file.py
index 9d80cea9..f92cea04 100644
--- a/geopandas/io/tests/test_file.py
+++ b/geopandas/io/tests/test_file.py
@@ -362,14 +362,31 @@ def test_to_file_types(tmpdir, df_points, engine):
     df.to_file(tempfilename, engine=engine)
 
 
-def test_to_file_int64(tmpdir, df_points, engine):
-    skip_pyogrio_not_supported(engine)  # TODO
-    tempfilename = os.path.join(str(tmpdir), "int64.shp")
+@pytest.mark.parametrize("driver,ext", driver_ext_pairs + [("OGR_GMT", ".gmt")])
+def test_to_file_int32(tmpdir, df_points, engine, driver, ext):
+    tempfilename = os.path.join(str(tmpdir), f"int32.{ext}")
+    geometry = df_points.geometry
+    df = GeoDataFrame(geometry=geometry)
+    df["data"] = pd.array([1, np.nan] * 5, dtype=pd.Int32Dtype())
+    df.to_file(tempfilename, driver=driver, engine=engine)
+    df_read = GeoDataFrame.from_file(tempfilename, driver=driver, engine=engine)
+    assert_geodataframe_equal(df_read, df, check_dtype=False, check_like=True)
+    if engine == "pyogrio":
+        tempfilename2 = os.path.join(str(tmpdir), f"int32_2.{ext}")
+        df2 = df.dropna()
+        df2.to_file(tempfilename2, driver=driver, engine=engine)
+        df2_read = GeoDataFrame.from_file(tempfilename2, driver=driver, engine=engine)
+        assert df2_read["data"].dtype == "int32"
+
+
+@pytest.mark.parametrize("driver,ext", driver_ext_pairs)
+def test_to_file_int64(tmpdir, df_points, engine, driver, ext):
+    tempfilename = os.path.join(str(tmpdir), f"int64.{ext}")
     geometry = df_points.geometry
     df = GeoDataFrame(geometry=geometry)
     df["data"] = pd.array([1, np.nan] * 5, dtype=pd.Int64Dtype())
-    df.to_file(tempfilename, engine=engine)
-    df_read = GeoDataFrame.from_file(tempfilename, engine=engine)
+    df.to_file(tempfilename, driver=driver, engine=engine)
+    df_read = GeoDataFrame.from_file(tempfilename, driver=driver, engine=engine)
     assert_geodataframe_equal(df_read, df, check_dtype=False, check_like=True)
 
 
diff --git a/geopandas/io/tests/test_infer_schema.py b/geopandas/io/tests/test_infer_schema.py
index 8565aa0b..3916cf10 100644
--- a/geopandas/io/tests/test_infer_schema.py
+++ b/geopandas/io/tests/test_infer_schema.py
@@ -10,6 +10,7 @@ from shapely.geometry import (
 )
 
 import pandas as pd
+import pytest
 import numpy as np
 from geopandas import GeoDataFrame
 from geopandas.io.file import infer_schema
@@ -278,12 +279,26 @@ def test_infer_schema_null_geometry_all():
     assert infer_schema(df) == {"geometry": "Unknown", "properties": OrderedDict()}
 
 
+@pytest.mark.parametrize(
+    "array_data,dtype", [([1, 2**31 - 1], np.int32), ([1, np.nan], pd.Int32Dtype())]
+)
+def test_infer_schema_int32(array_data, dtype):
+    int32col = pd.array(data=array_data, dtype=dtype)
+    df = GeoDataFrame(geometry=[city_hall_entrance, city_hall_balcony])
+    df["int32_column"] = int32col
+
+    assert infer_schema(df) == {
+        "geometry": "Point",
+        "properties": OrderedDict([("int32_column", "int32")]),
+    }
+
+
 def test_infer_schema_int64():
     int64col = pd.array([1, np.nan], dtype=pd.Int64Dtype())
     df = GeoDataFrame(geometry=[city_hall_entrance, city_hall_balcony])
-    df["int64"] = int64col
+    df["int64_column"] = int64col
 
     assert infer_schema(df) == {
         "geometry": "Point",
-        "properties": OrderedDict([("int64", "int")]),
+        "properties": OrderedDict([("int64_column", "int")]),
     }