-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Conversation
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 ReportAll modified and coverable lines are covered by tests ✅
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. |
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. |
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. |
The explicit thing to do is to respect what the user provided, if they passed in geopandas we should keep in geopandas. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Should consider whether to move the geopandas interface from geoviews to holoviews, but here's it working:
Fixes #5951