Repository: mie-lab/trackintel

Issue:
ENH for merge_staypoints() function
The `merge_staypoints()` function in `trackintel/preprocessing/staypoints.py` would need a location_id column. I think this is not yet reflected in the docstring.

Also, the function is not yet added to the documentation.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/347?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) Report
> Merging [#347](https://codecov.io/gh/mie-lab/trackintel/pull/347?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) (0f5307f) into [master](https://codecov.io/gh/mie-lab/trackintel/commit/5cd2b9a492984969dd6c8d8a6ca6dbd4b31e45ad?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) (5cd2b9a) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/mie-lab/trackintel/pull/347/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/347?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     #347   +/-   ##
=======================================
  Coverage   96.41%   96.41%           
=======================================
  Files          40       40           
  Lines        1924     1926    +2     
  Branches      313      313           
=======================================
+ Hits         1855     1857    +2     
  Misses         31       31           
  Partials       38       38           
```


| [Impacted Files](https://codecov.io/gh/mie-lab/trackintel/pull/347?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) | Coverage Δ | |
|---|---|---|
| [trackintel/preprocessing/staypoints.py](https://codecov.io/gh/mie-lab/trackintel/pull/347/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9wcmVwcm9jZXNzaW5nL3N0YXlwb2ludHMucHk=) | `95.68% <100.00%> (+0.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/347?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/347?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab). Last update [5cd2b9a...0f5307f](https://codecov.io/gh/mie-lab/trackintel/pull/347?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: Wow, thanks for the rapid fix. It also works in my case. I am using an empty tpls table to merge sp, so I think it's exactly what you said the dtype get transferred to float because of the nan. Now this is solved 

Patch:
diff --git a/docs/modules/preprocessing.rst b/docs/modules/preprocessing.rst
index cbf76da..707b33c 100644
--- a/docs/modules/preprocessing.rst
+++ b/docs/modules/preprocessing.rst
@@ -31,6 +31,11 @@ frequently visits and/or infer if they correspond to activities.
 
 .. autofunction:: trackintel.preprocessing.staypoints.generate_locations
 
+Due to tracking artifacts, it can occur that one activity is split into several staypoints. 
+We can aggregate the staypoints horizontally that are close in time and at the same location.
+
+.. autofunction:: trackintel.preprocessing.staypoints.merge_staypoints
+
 Triplegs
 ========
 
diff --git a/trackintel/preprocessing/staypoints.py b/trackintel/preprocessing/staypoints.py
index 679c565..ed8837e 100644
--- a/trackintel/preprocessing/staypoints.py
+++ b/trackintel/preprocessing/staypoints.py
@@ -233,7 +233,8 @@ def merge_staypoints(staypoints, triplegs, max_time_gap="10min", agg={}):
     Parameters
     ----------
     staypoints : GeoDataFrame (as trackintel staypoints)
-        The staypoints have to follow the standard definition for staypoints DataFrames.
+        The staypoints must contain a column `location_id` (see `generate_locations` function) and have to follow the
+        standard trackintel definition for staypoints DataFrames.
 
     triplegs: GeoDataFrame (as trackintel triplegs)
         The triplegs have to follow the standard definition for triplegs DataFrames.
@@ -277,6 +278,7 @@ def merge_staypoints(staypoints, triplegs, max_time_gap="10min", agg={}):
     # otherwise check if it's a Timedelta already, and raise error if not
     elif not isinstance(max_time_gap, pd.Timedelta):
         raise TypeError("Parameter max_time_gap must be either of type String or pd.Timedelta!")
+    assert "location_id" in staypoints.columns, "Staypoints must contain column location_id"
 
     sp_merge = staypoints.copy()
     index_name = staypoints.index.name
@@ -285,6 +287,9 @@ def merge_staypoints(staypoints, triplegs, max_time_gap="10min", agg={}):
     tpls_merge = triplegs.copy()
     tpls_merge["type"] = "tripleg"
     sp_merge["type"] = "staypoint"
+    # convert datatypes in order to preserve the datatypes (especially ints) despite of NaNs during concat
+    sp_merge = sp_merge.convert_dtypes()
+
     # a joined dataframe sp_tpls is constructed to add the columns 'type' and 'next_type' to the 'sp_merge' table
     # concat and sort by time
     sp_tpls = pd.concat([sp_merge, tpls_merge]).sort_values(by=["user_id", "started_at"])
@@ -327,7 +332,13 @@ def merge_staypoints(staypoints, triplegs, max_time_gap="10min", agg={}):
         cond_old = cond.copy()
 
     # Staypoint-required columnsare aggregated in the following manner:
-    agg_dict = {index_name: "first", "user_id": "first", "started_at": "first", "finished_at": "last"}
+    agg_dict = {
+        index_name: "first",
+        "user_id": "first",
+        "started_at": "first",
+        "finished_at": "last",
+        "location_id": "first",
+    }
     # User-defined further aggregation
     agg_dict.update(agg)
 


Test patch:
diff --git a/tests/preprocessing/test_staypoints.py b/tests/preprocessing/test_staypoints.py
index 79a8745..10c4425 100644
--- a/tests/preprocessing/test_staypoints.py
+++ b/tests/preprocessing/test_staypoints.py
@@ -399,7 +399,8 @@ class TestMergeStaypoints:
         """Test staypoint merging."""
         sp, tpls = example_staypoints_merge
         # first test with empty tpls
-        merged_sp = sp.as_staypoints.merge_staypoints(tpls, agg={"geom": "first", "location_id": "first"})
+        merged_sp = sp.as_staypoints.merge_staypoints(tpls, agg={"geom": "first"})
+        merged_sp = merged_sp.reindex(columns=sp.columns)
         assert len(merged_sp) == len(sp) - 3
         # some staypoints stay the same (not merged)
         assert (merged_sp.loc[1] == sp.loc[1]).all()
@@ -458,3 +459,11 @@ class TestMergeStaypoints:
         # the geom should correspond to the first one
         assert sp.loc[7, "geom"] == merged_sp.loc[7, "geom"]
         assert sp.loc[2, "geom"] == merged_sp.loc[2, "geom"]
+
+    def test_merge_staypoints_error(self, example_staypoints_merge):
+        sp, tpls = example_staypoints_merge
+        sp.drop(columns=["location_id"], inplace=True)
+        with pytest.raises(AssertionError) as excinfo:
+            _ = sp.as_staypoints.merge_staypoints(tpls)
+
+        assert "Staypoints must contain column location_id" in str(excinfo.value)