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 vector rendering #69

Closed
wants to merge 13 commits into from
Closed

Add vector rendering #69

wants to merge 13 commits into from

Conversation

davidbrochart
Copy link
Collaborator

@davidbrochart davidbrochart commented Aug 3, 2022

This PR adds support for GeoDataFrame rendering.

nyc.mp4

Closes #22.

@davidbrochart
Copy link
Collaborator Author

The vector data-flow (where the data source is a GeoDataFrame) has diverged quite a bit from the raster data-flow (where the data source is a DataArray). But I think it's possible to reconcile, because an internal DataArray is built in the vector data-flow. We could just treat it as a source for a raster data-flow, and benefit from it.
For this, I think that the raster data-flow should adopt the same strategy of having an intermediate DataArray where chunks are aligned with tiles (for a given zoom level), and that is already re-projected.

@jfoster17
Copy link

I'm happy to report that this change fixed things in Safari!

I'm not entirely sure about the change to make the layers not disappear while computing. On the one hand, it retains some information while we wait, so that's good. But displaying a blank layer helps the user to recognize that the system is doing something -- if you're not watching the swirling "in-progress" badge. I think a new user might think that nothing changes on zoom and be confused by that.

@davidbrochart
Copy link
Collaborator Author

davidbrochart commented Aug 20, 2022

Happy to hear that it now works in Safari!
About not removing the layer while it's computing, it's actually what Safari didn't like. The thing is that when e.g. we zoom in, Leaflet immediately makes requests to get the new tiles. In parallel, the kernel gets notified that the zoom has changed, and if it decides to remove the layer, Leaflet has to cancel the requests, which apparently Safari doesn't like (it probably actually doesn't cancel them, and instead waits for them to time out, before making new requests).
I actually like that the previous tiles are zoomed in while it's computing, and it's also the Leaflet behavior (you can see that if the server is slow to respond). If we want to make it clearer that the system is doing something, we could make the spinner fit the whole map. What do you think?

@benbovy
Copy link
Member

benbovy commented Aug 24, 2022

That's great @davidbrochart!

The vector data-flow (where the data source is a GeoDataFrame) has diverged quite a bit from the raster data-flow (where the data source is a DataArray). But I think it's possible to reconcile, because an internal DataArray is built in the vector data-flow.

I think that eventually it would be great to also support vector data sources directly as Xarray objects (e.g., static spatial features with multi-dimensional attribute data, or even dynamic spatial features like polygons with vertices evolving over time).

The fact that pygeos (and shapely 2.0) support vectorized operations (numpy ufuncs) and that soon Xarray will fully support custom indexes make it possible (i.e., indexed coordinates with geometry objects as array items). dask-geopandas and dask support in Xarray might also make things easier.

It would be nice to also have some convenient methods (public API) for conversion between GeoDataFrame and DataArray / Dataset objects.

@davidbrochart
Copy link
Collaborator Author

I think that eventually it would be great to also support vector data sources directly as Xarray objects

I didn't know that, for me xarray was all about gridded data. That opens up new possibilities indeed!

It would be nice to also have some convenient methods (public API) for conversion between GeoDataFrame and DataArray / Dataset objects.

In this PR xarray-leaflet uses geocube internally, which does just that. But if it's all integrated in xarray, even better.

Do you know when all of that will land in xarray?

@benbovy
Copy link
Member

benbovy commented Aug 24, 2022

Hmm I think that's out of scope for having such methods implemented in Xarray itself, but it may probably fit nicely in an Xarray "geo" extension like Geocube.

@davidbrochart
Copy link
Collaborator Author

Closing in favor of #75.

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.

Support vector tiles
3 participants