Repository: mie-lab/trackintel

Issue:
Allow for flexible geometry names in `read_*_gpd` functions.
For an old model of trackintel all geometry columns had to have the same name "geom". Later we dropped this prerequisite and now we access the name directly via the geometry attribute (see [contributing.md](https://github.com/mie-lab/trackintel/blob/master/CONTRIBUTING.md#adressing-geometry-columns)). This old model still persists in the `read_*_gpd` functions.
The proposed change would be to not rename the geometry column anymore and just add it directly.

Like:
```diff
- columns = {tracked_at: "tracked_at", user_id: "user_id", geom: "geom"}
+ columns = {tracked_at: "tracked_at", user_id: "user_id"}
  columns.update(mapper)
  pfs = gdf.rename(columns=columns)
- pfs = pfs.set_geometry("geom")
+ pfs = pfs.set_geometry(geom)
```

There are may some side effects that must be handled.
Additionally may rename argument to `geom_col` to be more consistent with the rest of the io module.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) Report
> Merging [#265](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) (440df6f) into [master](https://codecov.io/gh/mie-lab/trackintel/commit/a877097c1fedb2ea6e500bd6b47a157bdccbb08b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) (a877097) will **decrease** coverage by `4.01%`.
> The diff coverage is `100.00%`.

> :exclamation: Current head 440df6f differs from pull request most recent head e8876f7. Consider uploading reports for the commit e8876f7 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/mie-lab/trackintel/pull/265/graphs/tree.svg?width=650&height=150&src=pr&token=ITDZEEE1MO&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab)](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab)

```diff
@@            Coverage Diff             @@
##           master     #265      +/-   ##
==========================================
- Coverage   95.11%   91.10%   -4.02%     
==========================================
  Files          38       38              
  Lines        1618     1618              
  Branches      265      265              
==========================================
- Hits         1539     1474      -65     
- Misses         41      106      +65     
  Partials       38       38              
```


| [Impacted Files](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) | Coverage Δ | |
|---|---|---|
| [trackintel/io/from\_geopandas.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9pby9mcm9tX2dlb3BhbmRhcy5weQ==) | `90.00% <100.00%> (ø)` | |
| [trackintel/io/postgis.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9pby9wb3N0Z2lzLnB5) | `17.80% <0.00%> (-82.20%)` | :arrow_down: |
| [trackintel/model/trips.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC90cmlwcy5weQ==) | `95.23% <0.00%> (-4.77%)` | :arrow_down: |
| [trackintel/model/locations.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC9sb2NhdGlvbnMucHk=) | `96.00% <0.00%> (-4.00%)` | :arrow_down: |
| [trackintel/model/positionfixes.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC9wb3NpdGlvbmZpeGVzLnB5) | `97.05% <0.00%> (-2.95%)` | :arrow_down: |
| [trackintel/model/staypoints.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC9zdGF5cG9pbnRzLnB5) | `97.22% <0.00%> (-2.78%)` | :arrow_down: |
| [trackintel/model/triplegs.py](https://codecov.io/gh/mie-lab/trackintel/pull/265/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC90cmlwbGVncy5weQ==) | `97.56% <0.00%> (-2.44%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab). Last update [a877097...e8876f7](https://codecov.io/gh/mie-lab/trackintel/pull/265?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab).

hong2223: We try to follow the standard pandas commit message (with prefixes) for arranging a bit our commits and pull requests, [here](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#committing-your-code). I would now rename this PR to `CLN: Access geometry via geom_col attribute without renaming`

Patch:
diff --git a/examples/trackintel_basic_tutorial.ipynb b/examples/trackintel_basic_tutorial.ipynb
index 03e9363..279dcb9 100644
--- a/examples/trackintel_basic_tutorial.ipynb
+++ b/examples/trackintel_basic_tutorial.ipynb
@@ -97,7 +97,7 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "pfs = ti.io.from_geopandas.read_positionfixes_gpd(gdf, tracked_at=\"time\", user_id=\"User\", geom=\"geometry\", tz='UTC')\n",
+    "pfs = ti.io.from_geopandas.read_positionfixes_gpd(gdf, tracked_at=\"time\", user_id=\"User\", geom_col=\"geometry\", tz='UTC')\n",
     "# now you can safely call .as_positionfixes or use any trackintel functions\n",
     "pfs.as_positionfixes.plot()"
    ]
diff --git a/trackintel/io/from_geopandas.py b/trackintel/io/from_geopandas.py
index d0b38b3..adb7636 100644
--- a/trackintel/io/from_geopandas.py
+++ b/trackintel/io/from_geopandas.py
@@ -3,7 +3,7 @@ import pandas as pd
 from trackintel.io.file import _localize_timestamp
 
 
-def read_positionfixes_gpd(gdf, tracked_at="tracked_at", user_id="user_id", geom="geom", tz=None, mapper={}):
+def read_positionfixes_gpd(gdf, tracked_at="tracked_at", user_id="user_id", geom_col="geom", tz=None, mapper={}):
     """
     Read positionfixes from GeoDataFrames.
 
@@ -20,7 +20,7 @@ def read_positionfixes_gpd(gdf, tracked_at="tracked_at", user_id="user_id", geom
     user_id : str, default 'user_id'
         name of the column storing the user_id.
 
-    geom : str, default 'geom'
+    geom_col : str, default 'geom'
         name of the column storing the geometry.
 
     tz : str, optional
@@ -36,13 +36,13 @@ def read_positionfixes_gpd(gdf, tracked_at="tracked_at", user_id="user_id", geom
 
     Examples
     --------
-    >>> trackintel.read_positionfixes_gpd(gdf, user_id='User', geom='geometry', tz='utc')
+    >>> trackintel.read_positionfixes_gpd(gdf, user_id='User', geom_col='geom', tz='utc')
     """
-    columns = {tracked_at: "tracked_at", user_id: "user_id", geom: "geom"}
+    columns = {tracked_at: "tracked_at", user_id: "user_id"}
     columns.update(mapper)
 
     pfs = gdf.rename(columns=columns)
-    pfs = pfs.set_geometry("geom")
+    pfs = pfs.set_geometry(geom_col)
 
     # check and/or set timezone
     for col in ["tracked_at"]:
@@ -55,7 +55,7 @@ def read_positionfixes_gpd(gdf, tracked_at="tracked_at", user_id="user_id", geom
 
 
 def read_staypoints_gpd(
-    gdf, started_at="started_at", finished_at="finished_at", user_id="user_id", geom="geom", tz=None, mapper={}
+    gdf, started_at="started_at", finished_at="finished_at", user_id="user_id", geom_col="geom", tz=None, mapper={}
 ):
     """
     Read staypoints from GeoDataFrames.
@@ -76,7 +76,7 @@ def read_staypoints_gpd(
     user_id : str, default 'user_id'
         name of the column storing the user_id.
 
-    geom : str, default 'geom'
+    geom_col : str, default 'geom'
         name of the column storing the geometry.
 
     tz : str, optional
@@ -94,11 +94,11 @@ def read_staypoints_gpd(
     --------
     >>> trackintel.read_staypoints_gpd(gdf, started_at='start_time', finished_at='end_time', tz='utc')
     """
-    columns = {started_at: "started_at", finished_at: "finished_at", user_id: "user_id", geom: "geom"}
+    columns = {started_at: "started_at", finished_at: "finished_at", user_id: "user_id"}
     columns.update(mapper)
 
     stps = gdf.rename(columns=columns)
-    stps = stps.set_geometry("geom")
+    stps = stps.set_geometry(geom_col)
 
     # check and/or set timezone
     for col in ["started_at", "finished_at"]:
@@ -111,7 +111,7 @@ def read_staypoints_gpd(
 
 
 def read_triplegs_gpd(
-    gdf, started_at="started_at", finished_at="finished_at", user_id="user_id", geom="geometry", tz=None, mapper={}
+    gdf, started_at="started_at", finished_at="finished_at", user_id="user_id", geom_col="geom", tz=None, mapper={}
 ):
     """
     Read triplegs from GeoDataFrames.
@@ -132,7 +132,7 @@ def read_triplegs_gpd(
     user_id : str, default 'user_id'
         name of the column storing the user_id.
 
-    geom : str, default 'geom'
+    geom_col : str, default 'geom'
         name of the column storing the geometry.
 
     tz : str, optional
@@ -148,13 +148,13 @@ def read_triplegs_gpd(
 
     Examples
     --------
-    >>> trackintel.read_triplegs_gpd(gdf, user_id='User', geom='geometry', tz='utc')
+    >>> trackintel.read_triplegs_gpd(gdf, user_id='User', geom_col='geom', tz='utc')
     """
-    columns = {started_at: "started_at", finished_at: "finished_at", user_id: "user_id", geom: "geom"}
+    columns = {started_at: "started_at", finished_at: "finished_at", user_id: "user_id"}
     columns.update(mapper)
 
     tpls = gdf.rename(columns=columns)
-    tpls = tpls.set_geometry("geom")
+    tpls = tpls.set_geometry(geom_col)
 
     # check and/or set timezone
     for col in ["started_at", "finished_at"]:


Test patch:
diff --git a/tests/io/test_from_geopandas.py b/tests/io/test_from_geopandas.py
index 558ee1d..a6a96c3 100644
--- a/tests/io/test_from_geopandas.py
+++ b/tests/io/test_from_geopandas.py
@@ -13,10 +13,11 @@ class TestFromGeopandas:
         """Test if the results of reading from gpd and csv agrees."""
         gdf = gpd.read_file(os.path.join("tests", "data", "positionfixes.geojson"))
         gdf.set_index("id", inplace=True)
-        pfs_from_gpd = ti.io.from_geopandas.read_positionfixes_gpd(gdf, user_id="User", geom="geometry", tz="utc")
+        pfs_from_gpd = ti.io.from_geopandas.read_positionfixes_gpd(gdf, user_id="User", geom_col="geometry", tz="utc")
 
         pfs_file = os.path.join("tests", "data", "positionfixes.csv")
         pfs_from_csv = ti.read_positionfixes_csv(pfs_file, sep=";", tz="utc", index_col="id")
+        pfs_from_csv = pfs_from_csv.rename(columns={"geom": "geometry"})
 
         pd.testing.assert_frame_equal(pfs_from_gpd, pfs_from_csv, check_exact=False)
 
@@ -24,10 +25,11 @@ class TestFromGeopandas:
         """Test if the results of reading from gpd and csv agrees."""
         gdf = gpd.read_file(os.path.join("tests", "data", "triplegs.geojson"))
         gdf.set_index("id", inplace=True)
-        tpls_from_gpd = ti.io.from_geopandas.read_triplegs_gpd(gdf, user_id="User", geom="geometry", tz="utc")
+        tpls_from_gpd = ti.io.from_geopandas.read_triplegs_gpd(gdf, user_id="User", geom_col="geometry", tz="utc")
 
         tpls_file = os.path.join("tests", "data", "triplegs.csv")
         tpls_from_csv = ti.read_triplegs_csv(tpls_file, sep=";", tz="utc", index_col="id")
+        tpls_from_csv = tpls_from_csv.rename(columns={"geom": "geometry"})
 
         pd.testing.assert_frame_equal(tpls_from_gpd, tpls_from_csv, check_exact=False)
 
@@ -36,11 +38,12 @@ class TestFromGeopandas:
         gdf = gpd.read_file(os.path.join("tests", "data", "staypoints.geojson"))
         gdf.set_index("id", inplace=True)
         stps_from_gpd = ti.io.from_geopandas.read_staypoints_gpd(
-            gdf, "start_time", "end_time", geom="geometry", tz="utc"
+            gdf, "start_time", "end_time", geom_col="geometry", tz="utc"
         )
 
         stps_file = os.path.join("tests", "data", "staypoints.csv")
         stps_from_csv = ti.read_staypoints_csv(stps_file, sep=";", tz="utc", index_col="id")
+        stps_from_csv = stps_from_csv.rename(columns={"geom": "geometry"})
 
         pd.testing.assert_frame_equal(stps_from_gpd, stps_from_csv, check_exact=False)