Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trajectory.is_latlon() update for GeoPandas 0.7 #50

Merged
merged 2 commits into from
Jan 26, 2020

Conversation

martinfleis
Copy link
Contributor

Geopandas 0.7 will come with pyproj.CRS class under gdf.crs, so following will not work in the next release as it expects self.crs to be a dict. See geopandas/geopandas#1101 for details.

https://github.com/anitagraser/movingpandas/blob/01417e5b2ab8e3b06f8060cbcee934673ea7b4e4/movingpandas/trajectory.py#L115-L126

This fix ensures compatibility with future releases of geopandas as well as older ones. crs.is_geographic checks for any lon/lat projection, not only WGS84. I assume that it is expected behaviour anyway.

cc pyOpenSci/software-submission#18

@anitagraser
Copy link
Collaborator

Excellent, thank you!

@anitagraser anitagraser merged commit 4d656cb into movingpandas:master Jan 26, 2020
@anitagraser
Copy link
Collaborator

I just noticed that this change significantly slows down the creation of Trajectory objects. Cell 20 in 0_getting_started.ipynb used to finish in < 1s but with this change it takes > 1min. (Similar slow-downs occur in all other notebooks.)

@anitagraser
Copy link
Collaborator

I'm replacing all calls to this function with access to an object variable which is computed once in the constructor ...

@martinfleis
Copy link
Contributor Author

That's because the check is done for ever single row instead of the whole gdf at once. Solution might be having Trajectory.is_latlon attribute doing this check at _init and then just checking for True/False instead of if self.is_latlon().

@martinfleis
Copy link
Contributor Author

I'm replacing all calls to this function with access to an object variable which is computed once in the constructor ...

@anitagraser yeah, you were faster :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants