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

COMPAT: geopandas 1.0 compatibility fix in h3fy #209

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

martinfleis
Copy link
Member

GeoPandas now preserves the geometry column name if it has one. As a result, we end up with two columns called hex_id in the first step and a geometry assigned as an index in the second.

Fixed.

@knaaptime @jGaboardi we shall expand CI to test against nightly here. I caught it coincidentally when testing my teaching material against 1.0 alpha.

Also, this will need a patch release fairly soon.

@@ -139,7 +142,10 @@ def h3fy(source, resolution=6, clip=False, buffer=False, return_geoms=True):
else:
source = source.to_crs(4326)

source_unary = shapely.force_2d(source.unary_union)
if GPD_10:
source_unary = shapely.force_2d(source.union_all())
Copy link
Member Author

Choose a reason for hiding this comment

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

I future, we may want to be able to pass kwargs here geopandas/geopandas#3151

@martinfleis
Copy link
Member Author

@jGaboardi Should I try to include testing against nightly here or can I leave it out from this PR? There's a lot of dependencies in here and I'm not sure which we want to include as nightly.

@jGaboardi
Copy link
Member

@jGaboardi Should I try to include testing against nightly here or can I leave it out from this PR? There's a lot of dependencies in here and I'm not sure which we want to include as nightly.

I'd say we should do that in another PR.

@knaaptime
Copy link
Member

interesting, thanks. Wasnt aware of any of these gpd changes. Looks like we get a nice speedup with unary union? that'll be great, so probably need this fix in several other places :)

@knaaptime knaaptime merged commit 1439e42 into pysal:main Apr 25, 2024
6 checks passed
@martinfleis martinfleis deleted the gpd10-compat branch April 25, 2024 20:49
@martinfleis
Copy link
Member Author

Looks like we get a nice speedup with unary union? that'll be great, so probably need this fix in several other places :)

Not by changing unary_union to union_all(), that is calling the same shapely function. But once geopandas/geopandas#3151 lands, we may be able to use coverage union which is considerably faster if you know you have a valid coverage. Which we do here.

And yes, this patch will need to happen everywhere where unary_union is used as it is deprecated.

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

Successfully merging this pull request may close these issues.

3 participants