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

Add support for rasterizing geopandas dataframes directly #5958

Merged
merged 3 commits into from May 17, 2024

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Oct 25, 2023

Should consider whether to move the geopandas interface from geoviews to holoviews, but here's it working:

import geopandas as gpd
import geoviews
import holoviews as hv

from geodatasets import get_path
from holoviews.operation.datashader import rasterize

hv.extension('bokeh')

nybb = gpd.read_file(get_path("nybb"))

polys = hv.Polygons(nybb, datatype=['geodataframe'])

rasterize(polys)

bokeh_plot - 2023-10-25T175156 792

Fixes #5951

@jbednar
Copy link
Member

jbednar commented Oct 25, 2023

Nice! My understanding from @ianthomas23 was that the geopandas rendering was slower than the spatialpandas rendering. Are there any implications that we need to deal with? E.g. do we need to let users decide between directly rendering as geoparquet and converting to spatialpandas first? If the rendering is a lot faster, converting to sp might take more time initially and use more memory but then give faster zooms. I haven't tried the two alternatives, though, so this is just speculation.

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.39%. Comparing base (023ad71) to head (cb5949d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5958   +/-   ##
=======================================
  Coverage   88.39%   88.39%           
=======================================
  Files         323      323           
  Lines       67609    67610    +1     
=======================================
+ Hits        59761    59762    +1     
  Misses       7848     7848           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ianthomas23
Copy link
Member

We are in a difficult situation here with regard to automatic conversion from geopandas to spatialpandas or not. If we had never previously converted then we would be fine to always leave the data in the format passed to us, and provide documentation on how to convert between the two. But if we turn off automatic conversion from geopandas to spatialpandas then users with a certain class of problem (data above a certain size) will experience a performance loss.

@jbednar
Copy link
Member

jbednar commented Nov 1, 2023

Yes, but we can also go by whether SpatialPandas is installed, using it if it's installed and passing through to GeoPandas support if it isn't. I think that should cover most cases right now, and if eventually we stopped using SpatialPandas, that branch would never get executed, and we can silently move on.

@hoxbro hoxbro added the type: enhancement Minor feature or improvement to an existing feature label Nov 23, 2023
@philippjfr philippjfr added this to the 1.19.0 milestone May 17, 2024
@philippjfr
Copy link
Member Author

The explicit thing to do is to respect what the user provided, if they passed in geopandas we should keep in geopandas.

@hoxbro hoxbro enabled auto-merge (squash) May 17, 2024 11:20
@hoxbro hoxbro merged commit 802edf7 into main May 17, 2024
13 checks passed
@hoxbro hoxbro deleted the geopandas_ds_support branch May 17, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Minor feature or improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support GeoPandas passthrough to Datashader
5 participants