Repository: mie-lab/trackintel (python)

Issue:
trips with geometries cause error in id assignment
The problem is that geopandas overrides the explode method of pandas (see issue [here](https://github.com/geopandas/geopandas/issues/2061)). This causes a problem [here](https://github.com/geopandas/geopandas/issues/2061) and [here](https://github.com/mie-lab/trackintel/blob/master/trackintel/preprocessing/triplegs.py#L270) as we want to access the original pandas explode method. 
I think the simplest fix would be to call the dataframe constructor before using explode. It is not too expensive and then we don't have to wait until the geopandas issue is fixed and we are not limited to only new geopandas versions.
E.g.:
`temp = pd.DataFrame(trips).explode("tpls")`
`temp = pd.DataFrame(trips).explode("spts")`

Pull request:
BUG: Explode function now only called on dataframes.
Closes #305 

I solved the problem differently than we discussed. 
The GeoDataFrame is only created in this `if add_geometry` block, so I moved the block after using the exploding, that way we only use the DataFrame variant.
Creating a test for this is a bit difficult because we need to specify the geopandas version. So I just leave it without one.

Patch:
diff --git a/docs/environment.yml b/docs/environment.yml
index 4750c75..1392606 100644
--- a/docs/environment.yml
+++ b/docs/environment.yml
@@ -3,19 +3,19 @@ channels:
   - conda-forge
   - defaults
 dependencies:
-- python=3.8
-- numpy
-- matplotlib
-- pandas
+# - python=3.8
 - geopandas
-- scikit-learn
-- networkx
-- pint
-- pip
-- geoalchemy2
-- osmnx
-- psycopg2
-- sphinx
-- sphinx_rtd_theme
-- tqdm
-- similaritymeasures
\ No newline at end of file
+- pip:
+  - numpy
+  - matplotlib
+  - pandas
+  - scikit-learn
+  - networkx
+  - pint
+  - geoalchemy2
+  - osmnx
+  - psycopg2
+  - sphinx
+  - sphinx_rtd_theme
+  - tqdm
+  - similaritymeasures
diff --git a/trackintel/io/postgis.py b/trackintel/io/postgis.py
index 16c59dd..2a9c0d2 100644
--- a/trackintel/io/postgis.py
+++ b/trackintel/io/postgis.py
@@ -620,7 +620,7 @@ __doc = """Stores {long} to PostGIS. Usually, this is directly called on a {long
     Examples
     --------
     >>> {short}.as_{long}.to_postgis(conn_string, table_name)
-    >>> ti.io.postgis.write_{long}_postgis(pfs, conn_string, table_name)
+    >>> ti.io.postgis.write_{long}_postgis({short}, conn_string, table_name)
 """
 
 write_positionfixes_postgis.__doc__ = __doc.format(long="positionfixes", short="pfs")
diff --git a/trackintel/preprocessing/triplegs.py b/trackintel/preprocessing/triplegs.py
index a27ad58..12dfb90 100644
--- a/trackintel/preprocessing/triplegs.py
+++ b/trackintel/preprocessing/triplegs.py
@@ -3,7 +3,7 @@ import warnings
 import geopandas as gpd
 import numpy as np
 import pandas as pd
-from shapely.geometry import MultiPoint
+from shapely.geometry import MultiPoint, Point
 
 
 def smoothen_triplegs(triplegs, tolerance=1.0, preserve_topology=True):
@@ -237,6 +237,19 @@ def generate_trips(spts, tpls, gap_threshold=15, add_geometry=True):
         axis=1,
     )
 
+    # now handle the data that is aggregated in the trips
+    # assign trip_id to tpls
+    temp = trips.explode("tpls")
+    temp.index = temp["tpls"]
+    temp = temp[temp["tpls"].notna()]
+    tpls = tpls.join(temp["trip_id"], how="left")
+
+    # assign trip_id to spts, for non-activity spts
+    temp = trips.explode("spts")
+    temp.index = temp["spts"]
+    temp = temp[temp["spts"].notna()]
+    spts = spts.join(temp["trip_id"], how="left")
+
     # fill missing points and convert to MultiPoint
     # for all trips with missing 'origin_staypoint_id' we now assign the startpoint of the first tripleg of the trip.
     # for all tripls with missing 'destination_staypoint_id' we now assign the endpoint of the last tripleg of the trip.
@@ -245,13 +258,13 @@ def generate_trips(spts, tpls, gap_threshold=15, add_geometry=True):
         origin_nan_rows = trips[pd.isna(trips["origin_staypoint_id"])].copy()
         trips.loc[pd.isna(trips["origin_staypoint_id"]), "origin_geom"] = origin_nan_rows.tpls.map(
             # from tpls table, get the first point of the first tripleg for the trip
-            lambda x: tpls.loc[x[0], tpls.geometry.name].boundary[0]
+            lambda x: Point(tpls.loc[x[0], tpls.geometry.name].coords[0])
         )
         # fill geometry for destionations staypoints that are NaN
         destination_nan_rows = trips[pd.isna(trips["destination_staypoint_id"])].copy()
         trips.loc[pd.isna(trips["destination_staypoint_id"]), "destination_geom"] = destination_nan_rows.tpls.map(
             # from tpls table, get the last point of the last tripleg on the trip
-            lambda x: tpls.loc[x[-1], tpls.geometry.name].boundary[1]
+            lambda x: Point(tpls.loc[x[-1], tpls.geometry.name].coords[-1])
         )
         # convert to GeoDataFrame with MultiPoint column
         trips["geom"] = [MultiPoint([x, y]) for x, y in zip(trips.origin_geom, trips.destination_geom)]
@@ -259,19 +272,6 @@ def generate_trips(spts, tpls, gap_threshold=15, add_geometry=True):
         # cleanup
         trips.drop(["origin_geom", "destination_geom"], inplace=True, axis=1)
 
-    # now handle the data that is aggregated in the trips
-    # assign trip_id to tpls
-    temp = trips.explode("tpls")
-    temp.index = temp["tpls"]
-    temp = temp[temp["tpls"].notna()]
-    tpls = tpls.join(temp["trip_id"], how="left")
-
-    # assign trip_id to spts, for non-activity spts
-    temp = trips.explode("spts")
-    temp.index = temp["spts"]
-    temp = temp[temp["spts"].notna()]
-    spts = spts.join(temp["trip_id"], how="left")
-
     # final cleaning
     tpls.drop(columns=["type"], inplace=True)
     spts.drop(columns=["type"], inplace=True)


Test patch:
diff --git a/tests/preprocessing/test_triplegs.py b/tests/preprocessing/test_triplegs.py
index 84f22b9..5ac1537 100644
--- a/tests/preprocessing/test_triplegs.py
+++ b/tests/preprocessing/test_triplegs.py
@@ -14,6 +14,29 @@ import trackintel as ti
 from trackintel.preprocessing.triplegs import generate_trips
 
 
+@pytest.fixture
+def example_triplegs():
+    """Generate input data for trip generation from geolife positionfixes"""
+    pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
+    pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
+    stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
+    pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+    return stps, tpls
+
+
+@pytest.fixture
+def example_triplegs_higher_gap_threshold():
+    """Generate input data for trip generation, but with a higher gap threshold in stp generation"""
+    # create trips from geolife (based on positionfixes)
+    pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
+    pfs, stps = pfs.as_positionfixes.generate_staypoints(
+        method="sliding", dist_threshold=25, time_threshold=5, gap_threshold=1e6
+    )
+    stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
+    pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+    return stps, tpls
+
+
 class TestSmoothen_triplegs:
     def test_smoothen_triplegs(self):
         tpls_file = os.path.join("tests", "data", "triplegs_with_too_many_points_test.csv")
@@ -35,13 +58,10 @@ class TestSmoothen_triplegs:
 class TestGenerate_trips:
     """Tests for generate_trips() method."""
 
-    def test_duplicate_columns(self):
+    def test_duplicate_columns(self, example_triplegs):
         """Test if running the function twice, the generated column does not yield exception in join statement"""
         # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        stps, tpls = example_triplegs
 
         # generate trips and a joint staypoint/triplegs dataframe
         stps_run_1, tpls_run_1, _ = generate_trips(stps, tpls, gap_threshold=15)
@@ -51,18 +71,13 @@ class TestGenerate_trips:
         assert set(tpls_run_1.columns) == set(tpls_run_2.columns)
         assert set(stps_run_1.columns) == set(stps_run_2.columns)
 
-    def test_generate_trips(self):
+    def test_generate_trips(self, example_triplegs_higher_gap_threshold):
         """Test if we can generate the example trips based on example data."""
         # load pregenerated trips
         trips_loaded = ti.read_trips_csv(os.path.join("tests", "data", "geolife_long", "trips.csv"), index_col="id")
 
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(
-            method="sliding", dist_threshold=25, time_threshold=5, gap_threshold=1e6
-        )
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # create trips from geolife (based on positionfixes) - with gap_threshold 1e6
+        stps, tpls = example_triplegs_higher_gap_threshold
 
         # generate trips and a joint staypoint/triplegs dataframe
         stps, tpls, trips = generate_trips(stps, tpls, gap_threshold=15)
@@ -72,15 +87,10 @@ class TestGenerate_trips:
         # test if generated trips are equal
         assert_geodataframe_equal(trips_loaded, trips)
 
-    def test_trip_wo_geom(self):
+    def test_trip_wo_geom(self, example_triplegs_higher_gap_threshold):
         """Test if the add_geometry parameter shows correct behavior"""
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(
-            method="sliding", dist_threshold=25, time_threshold=5, gap_threshold=1e6
-        )
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # create trips from geolife (based on positionfixes) - with gap_threshold 1e6
+        stps, tpls = example_triplegs_higher_gap_threshold
 
         # generate trips dataframe with geometry
         _, _, trips = generate_trips(stps, tpls, gap_threshold=15)
@@ -92,15 +102,10 @@ class TestGenerate_trips:
         # test if generated trips are equal
         assert_frame_equal(trips_wo_geom, trips)
 
-    def test_trip_coordinates(self):
+    def test_trip_coordinates(self, example_triplegs_higher_gap_threshold):
         """Test if coordinates of start and destination are correct"""
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(
-            method="sliding", dist_threshold=25, time_threshold=5, gap_threshold=1e6
-        )
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # create trips from geolife (based on positionfixes) - with gap_threshold 1e6
+        stps, tpls = example_triplegs_higher_gap_threshold
 
         # generate trips and a joint staypoint/triplegs dataframe
         stps, tpls, trips = ti.preprocessing.triplegs.generate_trips(stps, tpls, gap_threshold=15)
@@ -133,16 +138,11 @@ class TestGenerate_trips:
 
             assert correct_dest_point == dest_point_trips
 
-    def test_accessor(self):
+    def test_accessor(self, example_triplegs):
         """Test if the accessor leads to the same results as the explicit function."""
-        # load pregenerated trips
-        trips_loaded = ti.read_trips_csv(os.path.join("tests", "data", "geolife_long", "trips.csv"), index_col="id")
 
-        # prepare data
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # get geolife test data (based on positionfixes)
+        stps, tpls = example_triplegs
 
         # generate trips using the explicit function import
         stps_expl, tpls_expl, trips_expl = ti.preprocessing.triplegs.generate_trips(stps, tpls, gap_threshold=15)
@@ -155,16 +155,11 @@ class TestGenerate_trips:
         assert_geodataframe_equal(stps_expl, stps_acc)
         assert_geodataframe_equal(tpls_expl, tpls_acc)
 
-    def test_accessor_arguments(self):
+    def test_accessor_arguments(self, example_triplegs):
         """Test if the accessor is robust to different ways to receive arguments"""
-        # load pregenerated trips
-        trips_loaded = ti.read_trips_csv(os.path.join("tests", "data", "geolife_long", "trips.csv"), index_col="id")
 
-        # prepare data
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, spts = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        spts = spts.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(spts)
+        # get geolife test data (based on positionfixes)
+        spts, tpls = example_triplegs
 
         # accessor with only arguments (not allowed)
         with pytest.raises(AssertionError):
@@ -181,13 +176,10 @@ class TestGenerate_trips:
         assert_geodataframe_equal(tpls_1, tpls_2)
         assert_geodataframe_equal(trips_1, trips_2)
 
-    def test_generate_trips_missing_link(self):
+    def test_generate_trips_missing_link(self, example_triplegs):
         """Test nan is assigned for missing link between stps and trips, and tpls and trips."""
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # get geolife test data (based on positionfixes)
+        stps, tpls = example_triplegs
 
         # generate trips and a joint staypoint/triplegs dataframe
         stps, tpls, _ = generate_trips(stps, tpls, gap_threshold=15)
@@ -195,13 +187,10 @@ class TestGenerate_trips:
         assert pd.isna(stps["prev_trip_id"]).any()
         assert pd.isna(stps["next_trip_id"]).any()
 
-    def test_generate_trips_dtype_consistent(self):
+    def test_generate_trips_dtype_consistent(self, example_triplegs):
         """Test the dtypes for the generated columns."""
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # get geolife test data (based on positionfixes)
+        stps, tpls = example_triplegs
 
         # generate trips and a joint staypoint/triplegs dataframe
         stps, tpls, trips = generate_trips(stps, tpls, gap_threshold=15)
@@ -214,15 +203,12 @@ class TestGenerate_trips:
         assert stps["next_trip_id"].dtype == "Int64"
         assert tpls["trip_id"].dtype == "Int64"
 
-    def test_compare_to_old_trip_function(self):
+    def test_compare_to_old_trip_function(self, example_triplegs):
         """Test if we can generate the example trips based on example data."""
         # load pregenerated trips
 
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # get geolife test data (based on positionfixes)
+        stps, tpls = example_triplegs
 
         # generate trips and a joint staypoint/triplegs dataframe
         stps, tpls, trips = generate_trips(stps, tpls, gap_threshold=15)
@@ -235,12 +221,10 @@ class TestGenerate_trips:
         assert_frame_equal(stps, stps_, check_like=True, check_index_type=False)
         assert_frame_equal(tpls, tpls_, check_like=True, check_index_type=False)
 
-    def test_generate_trips_index_start(self):
+    def test_generate_trips_index_start(self, example_triplegs):
         """Test the generated index start from 0 for different methods."""
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(method="sliding", dist_threshold=25, time_threshold=5)
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        # get geolife test data (based on positionfixes)
+        stps, tpls = example_triplegs
 
         # generate trips and a joint staypoint/triplegs dataframe
         _, _, trips = generate_trips(stps, tpls, gap_threshold=15)
@@ -307,20 +291,14 @@ class TestGenerate_trips:
         # test if generated staypoints/triplegs are equal (especially important for trip ids)
         assert_frame_equal(stps_tpls_loaded, stps_tpls, check_dtype=False)
 
-    def test_generate_trips_id_management(self):
+    def test_generate_trips_id_management(self, example_triplegs_higher_gap_threshold):
         """Test if we can generate the example trips based on example data."""
         stps_tpls_loaded = pd.read_csv(os.path.join("tests", "data", "geolife_long", "stps_tpls.csv"), index_col="id")
         stps_tpls_loaded["started_at"] = pd.to_datetime(stps_tpls_loaded["started_at"])
         stps_tpls_loaded["started_at_next"] = pd.to_datetime(stps_tpls_loaded["started_at_next"])
         stps_tpls_loaded["finished_at"] = pd.to_datetime(stps_tpls_loaded["finished_at"])
 
-        # create trips from geolife (based on positionfixes)
-        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
-        pfs, stps = pfs.as_positionfixes.generate_staypoints(
-            method="sliding", dist_threshold=25, time_threshold=5, gap_threshold=1e6
-        )
-        stps = stps.as_staypoints.create_activity_flag(time_threshold=15)
-        pfs, tpls = pfs.as_positionfixes.generate_triplegs(stps)
+        stps, tpls = example_triplegs_higher_gap_threshold
 
         # generate trips and a joint staypoint/triplegs dataframe
         gap_threshold = 15
@@ -356,6 +334,20 @@ class TestGenerate_trips:
         assert (tpls_["trip_id"] == 0).all()
         assert len(trips) == 1
 
+    def test_loop_linestring_case(self, example_triplegs):
+        """Test corner case where a tripleg starts and ends at the same point"""
+        # input data: preprocessed stps and tpls
+        stps, tpls = example_triplegs
+
+        # add a tripleg with same start as end, by modifying the first tripleg
+        tpls.loc[0, "geom"] = LineString([(0, 0), (1, 1), (0, 0)])
+
+        # generate trips and a joint staypoint/triplegs dataframe
+        stps, tpls, trips = ti.preprocessing.triplegs.generate_trips(stps, tpls, gap_threshold=15)
+
+        # test if start of first trip is (0,0)
+        assert trips.loc[0, "geom"][0] == Point(0, 0)
+
 
 def _create_debug_stps_tpls_data(stps, tpls, gap_threshold):
     """Preprocess stps and tpls for "test_generate_trips_*."""