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

Test geopandas #214

Closed
wants to merge 45 commits into from
Closed

Test geopandas #214

wants to merge 45 commits into from

Conversation

michellemho
Copy link
Contributor

@michellemho michellemho commented Sep 15, 2017

This PR tests trying to write a geodataframe with different arguments

closes #201
closes #182

@coveralls
Copy link

coveralls commented Sep 15, 2017

Coverage Status

Coverage increased (+0.2%) to 91.727% when pulling 4241876 on test_geopandas into 39c53b8 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 91.392% when pulling dcf9e53 on test_geopandas into 39c53b8 on master.

1 similar comment
@coveralls
Copy link

coveralls commented Sep 18, 2017

Coverage Status

Coverage decreased (-0.1%) to 91.392% when pulling dcf9e53 on test_geopandas into 39c53b8 on master.

@@ -174,7 +174,7 @@ def write(self, df, table_name, temp_dir='/tmp', overwrite=False,

if encode_geom:
# enforce that geodataframe CRS is 4326
df = df.to_crs({'init':'epsg:4326'})
df.crs = {'init':'epsg:4326'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what re-projections look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe so. I'm going to add a test!

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c5bfd65 on test_geopandas into ** on master**.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling c5bfd65 on test_geopandas into ** on master**.

@coveralls
Copy link

coveralls commented Sep 22, 2017

Coverage Status

Changes Unknown when pulling 7cf0dfd on test_geopandas into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

6 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

@coveralls
Copy link

coveralls commented Sep 22, 2017

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

@coveralls
Copy link

coveralls commented Sep 22, 2017

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 08769d4 on test_geopandas into ** on master**.

@andy-esch
Copy link
Contributor

@michellemho, could you resolve the conflicts here?

@andy-esch andy-esch added this to the beta.6-entanglement milestone Oct 31, 2017
@michellemho
Copy link
Contributor Author

Finally got around to this! I needed these geometry encoding changes to be merged in so I can use CARTOframes with some of the GPS work!

@andy-esch
Copy link
Contributor

Great, I'll take a look this week and let's get it merged in finally!!

@@ -39,6 +39,13 @@
mpi = None
plt = None
HAS_MATPLOTLIB = plt is not None
try:
import geopandas
from shapely.geometry import Point
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we don't need geopandas or shapely.geometry.Point anywhere in cartoframes, how about testing whether they can be imported other than importing them.

Looks like there's a way to do that here: https://stackoverflow.com/questions/14050281/how-to-check-if-a-python-module-exists-without-importing-it

We'd probably need an:

if version.major_version == 2:
    python2.7 version
elfi version.version < (3, 3):
    python3.3ish version
elif version.version >= (3, 4):
   python3.4+ stuff
else:
   HAS_GEOPANDAS = False


if encode_geom:
if not HAS_GEOPANDAS:
raise RuntimeError('geopandas and shapely needs to be installed to use `encode_geom`')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

'shapely (or geopandas) needs to be installed if using the encode_geom flag'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe this is better:

Install shapely >= vx.x.x (or geopandas >= vy.y.y) to enable geometry encoding

@Jesus89
Copy link
Member

Jesus89 commented Oct 31, 2019

Since we already have geopandas as a dependency and there is no context anymore, could we close this PR @andy-esch?

@andy-esch
Copy link
Contributor

Yep!

@andy-esch andy-esch closed this Oct 31, 2019
@Jesus89 Jesus89 deleted the test_geopandas branch May 25, 2020 05:54
@Jesus89 Jesus89 restored the test_geopandas branch May 25, 2020 05:55
@Jesus89 Jesus89 deleted the test_geopandas branch May 25, 2020 05:55
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.

None yet

4 participants