Repository: mie-lab/trackintel

Issue:
Write locations with extent to postgis throws error
Hi, 
I have tried to write locations with extent to a postgis database but I received an error. Here is an example that is very similar to the existing test:

```python
from shapely.geometry import Point, Polygon
import geopandas as gpd
from sqlalchemy import create_engine
import trackintel as ti
from db_login import DSN  # database login information
import numpy as np

engine = create_engine(
    "postgresql://{db_user}:{db_password}@{db_host}:{db_port}/{db_database}".format(
        **DSN
    )
)

p1 = Point(8.5067847, 47.4)
p2 = Point(8.5067847, 47.5)
p3 = Point(8.5067847, 47.6)

list_dict = [
    {"user_id": 0, "center": p1},
    {"user_id": 0, "center": p2},
    {"user_id": 1, "center": p3},
]
locs = gpd.GeoDataFrame(data=list_dict, geometry="center", crs="EPSG:4326")
locs.index.name = "id"

coords = [[8.45, 47.6], [8.45, 47.4], [8.55, 47.4], [8.55, 47.6], [8.45, 47.6]]
extent = Polygon(coords)
locs["extent"] = extent  # broadcasting
locs["extent"] = gpd.GeoSeries(locs["extent"])  # dtype

locs.as_locations.to_postgis(name='locations', con=engine, schema="yumuv_graph_rep", if_exists='replace')
```
This throws the following error: 

```
psycopg2.errors.InvalidParameterValue: Geometry SRID (0) does not match column SRID (4326)
CONTEXT:  COPY locations, line 1, column extent: "POLYGON ((8.449999999999999 47.6, 8.449999999999999 47.4, 8.550000000000001 47.4, 8.550000000000001 ..."
```

I am honestly not really sure why as it is very similar to the existing test for locations. I could solve this by replacing [line 238](https://github.com/mie-lab/trackintel/blob/master/trackintel/io/postgis.py#L238)
`locations["extent"] = locations["extent"].apply(lambda x: WKTElement(x.wkt, srid=srid))` with the following:
`locations = gpd.GeoDataFrame(_convert_to_ewkb(locations, 'extent', srid), geometry='center', crs=srid)`

`_convert_to_ewkb` is a function from geopandas and can be imported `from geopandas.io.sql import _convert_to_ewkb`. This function is used in Geopandas to prepare the main geometry [link](https://github.com/geopandas/geopandas/blob/master/geopandas/io/sql.py#L402)


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/290?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) Report
> Merging [#290](https://codecov.io/gh/mie-lab/trackintel/pull/290?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) (9e1655d) into [master](https://codecov.io/gh/mie-lab/trackintel/commit/33d6205475ad8c4a859a9a5883241fe1098997b2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) (33d6205) will **decrease** coverage by `2.42%`.
> The diff coverage is `50.00%`.

[![Impacted file tree graph](https://codecov.io/gh/mie-lab/trackintel/pull/290/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/290?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     #290      +/-   ##
==========================================
- Coverage   96.04%   93.62%   -2.43%     
==========================================
  Files          39       39              
  Lines        1693     1693              
  Branches      266      265       -1     
==========================================
- Hits         1626     1585      -41     
- Misses         35       74      +39     
- Partials       32       34       +2     
```


| [Impacted Files](https://codecov.io/gh/mie-lab/trackintel/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab) | Coverage Δ | |
|---|---|---|
| [trackintel/io/postgis.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9pby9wb3N0Z2lzLnB5) | `52.00% <50.00%> (-42.67%)` | :arrow_down: |
| [trackintel/io/from\_geopandas.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/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% <0.00%> (-8.00%)` | :arrow_down: |
| [trackintel/model/locations.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/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.87% <0.00%> (-3.13%)` | :arrow_down: |
| [trackintel/model/trips.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC90cmlwcy5weQ==) | `96.96% <0.00%> (-3.04%)` | :arrow_down: |
| [trackintel/model/positionfixes.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/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.82% <0.00%> (-2.18%)` | :arrow_down: |
| [trackintel/model/staypoints.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC9zdGF5cG9pbnRzLnB5) | `98.03% <0.00%> (-1.97%)` | :arrow_down: |
| [trackintel/model/triplegs.py](https://codecov.io/gh/mie-lab/trackintel/pull/290/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab#diff-dHJhY2tpbnRlbC9tb2RlbC90cmlwbGVncy5weQ==) | `98.27% <0.00%> (-1.73%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/mie-lab/trackintel/pull/290?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/290?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mie-lab). Last update [33d6205...9e1655d](https://codecov.io/gh/mie-lab/trackintel/pull/290?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).

bifbof: Cool, thanks for the advice how to change it. :)
The CI file is now changed. I have left the contributing.md as it is for now, because as before I would prefer a permanent link and for that we need a commit with this change first.

Patch:
diff --git a/trackintel/io/postgis.py b/trackintel/io/postgis.py
index c898fd5..b0cba39 100644
--- a/trackintel/io/postgis.py
+++ b/trackintel/io/postgis.py
@@ -2,6 +2,8 @@ from functools import wraps
 from inspect import signature
 
 import geopandas as gpd
+from geopandas.io.sql import _get_srid_from_crs
+from shapely.wkb import dumps
 import pandas as pd
 from geoalchemy2 import Geometry, WKTElement
 from sqlalchemy import create_engine
@@ -224,10 +226,7 @@ def write_locations_postgis(
     # May build additional check for that.
     if "extent" in locations.columns:
         # geopandas.to_postgis can only handle one geometry column -> do it manually
-        if locations.crs is not None:
-            srid = locations.crs.to_epsg()
-        else:
-            srid = -1
+        srid = _get_srid_from_crs(locations)
         extent_schema = Geometry("POLYGON", srid)
 
         if dtype is None:
@@ -235,7 +234,7 @@ def write_locations_postgis(
         else:
             dtype["extent"] = extent_schema
         locations = locations.copy()
-        locations["extent"] = locations["extent"].apply(lambda x: WKTElement(x.wkt, srid=srid))
+        locations["extent"] = locations["extent"].apply(lambda x: dumps(x, srid=srid, hex=True))
 
     locations.to_postgis(
         name,
diff --git a/trackintel/preprocessing/positionfixes.py b/trackintel/preprocessing/positionfixes.py
index d604382..1ff8cf9 100644
--- a/trackintel/preprocessing/positionfixes.py
+++ b/trackintel/preprocessing/positionfixes.py
@@ -5,9 +5,9 @@ import geopandas as gpd
 import numpy as np
 import pandas as pd
 from shapely.geometry import LineString, Point
-from tqdm import tqdm
 
 from trackintel.geogr.distances import haversine_dist
+from trackintel.preprocessing.util import applyParallel
 
 
 def generate_staypoints(
@@ -20,6 +20,7 @@ def generate_staypoints(
     include_last=False,
     print_progress=False,
     exclude_duplicate_pfs=True,
+    n_jobs=1,
 ):
     """
     Generate staypoints from positionfixes.
@@ -48,18 +49,24 @@ def generate_staypoints(
         temporal gaps larger than 'gap_threshold' will be excluded from staypoints generation.
         Only valid in 'sliding' method.
 
-    include_last: boolen, default False
+    include_last: boolean, default False
         The algorithm in Li et al. (2008) only detects staypoint if the user steps out
         of that staypoint. This will omit the last staypoint (if any). Set 'include_last'
         to True to include this last staypoint.
 
-    print_progress: boolen, default False
+    print_progress: boolean, default False
         Show per-user progress if set to True.
 
     exclude_duplicate_pfs: boolean, default True
         Filters duplicate positionfixes before generating staypoints. Duplicates can lead to problems in later
         processing steps (e.g., when generating triplegs). It is not recommended to set this to False.
 
+    n_jobs: int, default 1
+        The maximum number of concurrently running jobs. If -1 all CPUs are used. If 1 is given, no parallel
+        computing code is used at all, which is useful for debugging. See
+        https://joblib.readthedocs.io/en/latest/parallel.html#parallel-reference-documentation
+        for a detailed description
+
     Returns
     -------
     pfs: GeoDataFrame (as trackintel positionfixes)
@@ -118,37 +125,20 @@ def generate_staypoints(
     # TODO: tests using a different distance function, e.g., L2 distance
     if method == "sliding":
         # Algorithm from Li et al. (2008). For details, please refer to the paper.
-        if print_progress:
-            tqdm.pandas(desc="User staypoint generation")
-            stps = (
-                pfs.groupby("user_id", as_index=False)
-                .progress_apply(
-                    _generate_staypoints_sliding_user,
-                    geo_col=geo_col,
-                    elevation_flag=elevation_flag,
-                    dist_threshold=dist_threshold,
-                    time_threshold=time_threshold,
-                    gap_threshold=gap_threshold,
-                    distance_metric=distance_metric,
-                    include_last=include_last,
-                )
-                .reset_index(drop=True)
-            )
-        else:
-            stps = (
-                pfs.groupby("user_id", as_index=False)
-                .apply(
-                    _generate_staypoints_sliding_user,
-                    geo_col=geo_col,
-                    elevation_flag=elevation_flag,
-                    dist_threshold=dist_threshold,
-                    time_threshold=time_threshold,
-                    gap_threshold=gap_threshold,
-                    distance_metric=distance_metric,
-                    include_last=include_last,
-                )
-                .reset_index(drop=True)
-            )
+        stps = applyParallel(
+            pfs.groupby("user_id", as_index=False),
+            _generate_staypoints_sliding_user,
+            n_jobs=n_jobs,
+            print_progress=print_progress,
+            geo_col=geo_col,
+            elevation_flag=elevation_flag,
+            dist_threshold=dist_threshold,
+            time_threshold=time_threshold,
+            gap_threshold=gap_threshold,
+            distance_metric=distance_metric,
+            include_last=include_last,
+        ).reset_index(drop=True)
+
         # index management
         stps["id"] = np.arange(len(stps))
         stps.set_index("id", inplace=True)
@@ -328,7 +318,7 @@ def generate_triplegs(
         tpls_diff = np.diff(tpls_starts)
 
         # get the start position of stps
-        # pd.NA causes error in boolen comparision, replace to -1
+        # pd.NA causes error in boolean comparision, replace to -1
         stps_id = pfs["staypoint_id"].copy().fillna(-1)
         unique_stps, stps_starts = np.unique(stps_id, return_index=True)
         # get the index of where the tpls_starts belong in stps_starts
diff --git a/trackintel/preprocessing/staypoints.py b/trackintel/preprocessing/staypoints.py
index 1a520b7..00040e2 100644
--- a/trackintel/preprocessing/staypoints.py
+++ b/trackintel/preprocessing/staypoints.py
@@ -5,9 +5,9 @@ import geopandas as gpd
 import pandas as pd
 from shapely.geometry import Point
 from sklearn.cluster import DBSCAN
-from tqdm import tqdm
 
 from trackintel.geogr.distances import meters_to_decimal_degrees
+from trackintel.preprocessing.util import applyParallel
 
 
 def generate_locations(
@@ -18,6 +18,7 @@ def generate_locations(
     distance_metric="haversine",
     agg_level="user",
     print_progress=False,
+    n_jobs=1,
 ):
     """
     Generate locations from the staypoints.
@@ -51,6 +52,12 @@ def generate_locations(
     print_progress : bool, default False
         If print_progress is True, the progress bar is displayed
 
+    n_jobs: int, default 1
+        The maximum number of concurrently running jobs. If -1 all CPUs are used. If 1 is given, no parallel
+        computing code is used at all, which is useful for debugging. See
+        https://joblib.readthedocs.io/en/latest/parallel.html#parallel-reference-documentation
+        for a detailed description
+
     Returns
     -------
     ret_sp: GeoDataFrame (as trackintel staypoints)
@@ -85,21 +92,15 @@ def generate_locations(
             db = DBSCAN(eps=epsilon, min_samples=num_samples, algorithm="ball_tree", metric=distance_metric)
 
         if agg_level == "user":
-            if print_progress:
-                tqdm.pandas(desc="User location generation")
-                ret_stps = ret_stps.groupby("user_id", as_index=False).progress_apply(
-                    _generate_locations_per_user,
-                    geo_col=geo_col,
-                    distance_metric=distance_metric,
-                    db=db,
-                )
-            else:
-                ret_stps = ret_stps.groupby("user_id", as_index=False).apply(
-                    _generate_locations_per_user,
-                    geo_col=geo_col,
-                    distance_metric=distance_metric,
-                    db=db,
-                )
+            ret_stps = applyParallel(
+                ret_stps.groupby("user_id", as_index=False),
+                _generate_locations_per_user,
+                n_jobs=n_jobs,
+                print_progress=print_progress,
+                geo_col=geo_col,
+                distance_metric=distance_metric,
+                db=db,
+            )
 
             # keeping track of noise labels
             ret_stps_non_noise_labels = ret_stps[ret_stps["location_id"] != -1]
diff --git a/trackintel/preprocessing/util.py b/trackintel/preprocessing/util.py
index e14485b..04c6874 100644
--- a/trackintel/preprocessing/util.py
+++ b/trackintel/preprocessing/util.py
@@ -1,3 +1,8 @@
+import pandas as pd
+from joblib import Parallel, delayed
+from tqdm import tqdm
+
+
 def calc_temp_overlap(start_1, end_1, start_2, end_2):
     """
     Calculate the portion of the first time span that overlaps with the second
@@ -59,3 +64,38 @@ def calc_temp_overlap(start_1, end_1, start_2, end_2):
         overlap_ratio = temp_overlap / dur.total_seconds()
 
     return overlap_ratio
+
+
+def applyParallel(dfGrouped, func, n_jobs, print_progress, **kwargs):
+    """
+    Funtion warpper to parallelize funtions after .groupby().
+
+    Parameters
+    ----------
+    dfGrouped: pd.DataFrameGroupBy
+        The groupby object after calling df.groupby(COLUMN).
+
+    func: function
+        Function to apply to the dfGrouped object, i.e., dfGrouped.apply(func).
+
+    n_jobs: int
+        The maximum number of concurrently running jobs. If -1 all CPUs are used. If 1 is given, no parallel
+        computing code is used at all, which is useful for debugging. See
+        https://joblib.readthedocs.io/en/latest/parallel.html#parallel-reference-documentation
+        for a detailed description
+
+    print_progress: boolean
+        If set to True print the progress of apply.
+
+    **kwargs:
+        Other arguments passed to func.
+
+    Returns
+    -------
+    pd.DataFrame:
+        The result of dfGrouped.apply(func)
+    """
+    df_ls = Parallel(n_jobs=n_jobs)(
+        delayed(func)(group, **kwargs) for _, group in tqdm(dfGrouped, disable=not print_progress)
+    )
+    return pd.concat(df_ls)


Test patch:
diff --git a/.github/workflows/CI-tests.yml b/.github/workflows/CI-tests.yml
index 2e2e170..3c267ac 100644
--- a/.github/workflows/CI-tests.yml
+++ b/.github/workflows/CI-tests.yml
@@ -60,5 +60,5 @@ jobs:
         source activate test
         conda install postgis -c conda-forge
         source ci/envs/setup_postgres.sh
-        pytest -v -r s --color=yes --cov=trackintel --cov-append --cov-report term-missing --cov-report xml tests/io/test_postgis_gpd.py | tee /dev/stderr | if grep SKIPPED >/dev/null;then echo "TESTS SKIPPED, FAILING" && exit 1;fi
+        pytest -v -r s --color=yes --cov=trackintel --cov-append --cov-report term-missing --cov-report xml tests/io/test_postgis.py | tee /dev/stderr | if grep SKIPPED >/dev/null;then echo "TESTS SKIPPED, FAILING" && exit 1;fi
     - uses: codecov/codecov-action@v1
\ No newline at end of file
diff --git a/tests/io/test_postgis_gpd.py b/tests/io/test_postgis.py
similarity index 100%
rename from tests/io/test_postgis_gpd.py
rename to tests/io/test_postgis.py
diff --git a/tests/preprocessing/test_positionfixes.py b/tests/preprocessing/test_positionfixes.py
index c1ec21d..2e84a7a 100644
--- a/tests/preprocessing/test_positionfixes.py
+++ b/tests/preprocessing/test_positionfixes.py
@@ -86,6 +86,18 @@ def example_positionfixes_isolated():
 class TestGenerate_staypoints:
     """Tests for generate_staypoints() method."""
 
+    def test_parallel_computing(self):
+        """The result obtained with parallel computing should be identical."""
+        pfs, _ = ti.io.dataset_reader.read_geolife(os.path.join("tests", "data", "geolife_long"))
+        # without parallel computing code
+        pfs_ori, stps_ori = pfs.as_positionfixes.generate_staypoints(n_jobs=1)
+        # using two cores
+        pfs_para, stps_para = pfs.as_positionfixes.generate_staypoints(n_jobs=2)
+
+        # the result of parallel computing should be identical
+        assert_geodataframe_equal(pfs_ori, pfs_para)
+        assert_geodataframe_equal(stps_ori, stps_para)
+
     def test_duplicate_pfs_warning(self, example_positionfixes):
         """Calling generate_staypoints with duplicate positionfixes should raise a warning."""
         pfs_duplicate_loc = example_positionfixes.copy()
diff --git a/tests/preprocessing/test_staypoints.py b/tests/preprocessing/test_staypoints.py
index 463cf5a..5ba08c3 100644
--- a/tests/preprocessing/test_staypoints.py
+++ b/tests/preprocessing/test_staypoints.py
@@ -7,6 +7,7 @@ import pandas as pd
 import pytest
 from shapely.geometry import Point
 from sklearn.cluster import DBSCAN
+from geopandas.testing import assert_geodataframe_equal
 
 import trackintel as ti
 from trackintel.geogr.distances import calculate_distance_matrix
@@ -57,6 +58,23 @@ def example_staypoints():
 class TestGenerate_locations:
     """Tests for generate_locations() method."""
 
+    def test_parallel_computing(self, example_staypoints):
+        """The result obtained with parallel computing should be identical."""
+        stps = example_staypoints
+
+        # without parallel computing code
+        stps_ori, locs_ori = stps.as_staypoints.generate_locations(
+            method="dbscan", epsilon=10, num_samples=2, distance_metric="haversine", agg_level="user", n_jobs=1
+        )
+        # using two cores
+        stps_para, locs_para = stps.as_staypoints.generate_locations(
+            method="dbscan", epsilon=10, num_samples=2, distance_metric="haversine", agg_level="user", n_jobs=2
+        )
+
+        # the result of parallel computing should be identical
+        assert_geodataframe_equal(locs_ori, locs_para)
+        assert_geodataframe_equal(stps_ori, stps_para)
+
     def test_dbscan_hav_euc(self):
         """Test if using haversine and euclidean distances will generate the same location result."""
         stps_file = os.path.join("tests", "data", "geolife", "geolife_staypoints.csv")
diff --git a/tests/preprocessing/test_triplegs.py b/tests/preprocessing/test_triplegs.py
index 58f5c55..84f22b9 100644
--- a/tests/preprocessing/test_triplegs.py
+++ b/tests/preprocessing/test_triplegs.py
@@ -275,8 +275,6 @@ class TestGenerate_trips:
         stps_in = gpd.GeoDataFrame(stps_in, geometry="geom")
         stps_in = ti.io.read_staypoints_gpd(stps_in, tz="utc")
 
-        assert stps_in.as_staypoints
-
         tpls_in = pd.read_csv(
             os.path.join("tests", "data", "trips", "triplegs_gaps.csv"),
             sep=";",
@@ -289,12 +287,10 @@ class TestGenerate_trips:
         tpls_in = gpd.GeoDataFrame(tpls_in, geometry="geom")
         tpls_in = ti.io.read_triplegs_gpd(tpls_in, tz="utc")
 
-        assert tpls_in.as_triplegs
-
         # load ground truth data
-        trips_loaded = ti.read_trips_csv(os.path.join("tests", "data", "trips", "trips_gaps.csv"), index_col="id")
-        trips_loaded["started_at"] = pd.to_datetime(trips_loaded["started_at"], utc=True)
-        trips_loaded["finished_at"] = pd.to_datetime(trips_loaded["finished_at"], utc=True)
+        trips_loaded = ti.read_trips_csv(
+            os.path.join("tests", "data", "trips", "trips_gaps.csv"), index_col="id", tz="utc"
+        )
 
         stps_tpls_loaded = pd.read_csv(os.path.join("tests", "data", "trips", "stps_tpls_gaps.csv"), index_col="id")
         stps_tpls_loaded["started_at"] = pd.to_datetime(stps_tpls_loaded["started_at"], utc=True)