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 #75

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

davidbrochart
Copy link
Collaborator

Same as #69, but from my fork of xarray-leaflet.

@davidbrochart
Copy link
Collaborator Author

@jfoster17 I made some optimizations that should hopefully fix the issues you reported.
I also changed the API so that you can choose to make a vector map dynamic, and pass an external rasterize function.

@jfoster17
Copy link

Thanks @davidbrochart -- as we discussed offline, I'm having trouble getting a new vector layer to serve through voila on a remote server. You suggested the following:

Pass get_base_url to df.leaflet.plot with:

def get_base_url(window_url):
    return window_url

and launching the application as:

jupyter server --ServerApp.default_url="voila/render/examples/vector.ipynb"

I haven't been able to get this working on my server, which I configured following the directions here. Specifically, since your instructions suggest launching voila via jupyter server, I'm sure there is some extra steps to make this work. I don't really understand the distinction...

I did try updating the nginx configuration for the proxy_pass to http://localhost:8888 since that is the port jupyter server was trying to serve to, and that got me as a login page for jupyer server, but then a 404 for the actual page.

Is there not a way to get this running with the voila command? Or is there some documentation on how to serve voila via jupyter server?

@jfoster17
Copy link

I also get this odd behavior when I am running this locally on my Mac versus (remotely) on Ubuntu. Serving the notebook notebook from Linux I get the expected behavior, but locally I get some sort of jumbled tiles where the color scales of buildings changes radically on zoom/pan and sometimes I can see obvious tile boundaries.

vector_problems.mov

@raybellwaves
Copy link
Member

Anything I can do to help review this?

@davidbrochart
Copy link
Collaborator Author

Thanks for the help @raybellwaves!
The code is a bit of a mess, I'll probably refactor later, but it would be great if you could try this PR and give your feedback.
@jfoster17 I'll answer your comments tomorrow, it's late here :)

@davidbrochart
Copy link
Collaborator Author

I did try updating the nginx configuration for the proxy_pass to http://localhost:8888/ since that is the port jupyter server was trying to serve to, and that got me as a login page for jupyer server, but then a 404 for the actual page.

@jfoster17 You can pass --port=8866 or whatever port number you want. If you land on a login page, then you need to enter the token jupyter-server showed in the terminal. Or you can disable authentication with --ServerApp.token='' --ServerApp.password=''.
Also, if your server has a base URL, e.g. https://yourdomain.com/my/base/url, then you need:

def get_base_url(window_url):
    return "https://yourdomain.com/my/base/url"

@jfoster17
Copy link

Another request -- it would be nice to be able to fiddle with the size of point-like data. Currently, points end up very small and can be hard to see. Obviously one option is to transform points back into larger polygon. Another easy fix might be to pass additional kwargs into make_geocube so that one could specify resolution to that method (at least I think that would work).

@davidbrochart
Copy link
Collaborator Author

I get some sort of jumbled tiles where the color scales of buildings changes radically on zoom/pan and sometimes I can see obvious tile boundaries.

@jfoster17 I was finally able to track down the issue. It is a regression that has been reported here. For now, a workaround is to install pandas<1.5, e.g. pip install pandas==1.4.

@davidbrochart
Copy link
Collaborator Author

it would be nice to be able to fiddle with the size of point-like data

@jfoster17 I think it would be nice to be able to apply transformations to the data, as with the raster data flow. For instance, the rivers in this notebook could not be revealed without "thickening", because rivers are represented with one-pixel wide streams in the original array. This is achieved by applying scipy.ndimage.maximum_filter(array, footprint=circle) on each tile.
I'm going to try and support transformation functions in the vector data flow.

@davidbrochart
Copy link
Collaborator Author

I added an example notebook in examples/vector_points.ipynb to show how points can be made bigger with a transform function. The issue is that it runs independently on every tile, so you can see boundary effects (points being cut).

@raybellwaves
Copy link
Member

Making some notes for reviewing this (mostly for myself).

git clone https://github.com/xarray-contrib/xarray_leaflet.git
cd xarray_leaflet
git fetch origin pull/75/head:vector
git checkout vector
mamba create -n xarray_leaflet python=3.10 --y
conda activate xarray_leaflet
mkdir -p ~/miniforge3/envs/xarray_leaflet/etc/jupyter/jupyter_notebook_config.d
mkdir -p ~/miniforge3/envs/xarray_leaflet/etc/jupyter/jupyter_server_config.d
cp etc/jupyter/jupyter_notebook_config.d/* ~/miniforge3/envs/xarray_leaflet/etc/jupyter/jupyter_notebook_config.d/
cp etc/jupyter/jupyter_server_config.d/* ~/miniforge3/envs/xarray_leaflet/etc/jupyter/jupyter_server_config.d/
mamba install jupyterlab pyarrow
pip install -e .
jupyter lab
# run examples/vector.ipynb and vector_points.ipynb

Notebooks work great.

TODO try it on a month of the NYC taxi cab data inspired by https://examples.pyviz.org/nyc_taxi/nyc_taxi.html as an example of big point data.

!wget https://d37ci6vzurychx.cloudfront.net/trip-data/yellow_tripdata_2022-01.parquet
!wget https://d37ci6vzurychx.cloudfront.net/misc/taxi_zones.zip

@davidbrochart
Copy link
Collaborator Author

Thanks @raybellwaves.
Note that if you install JupyterLab before, then you don't need the mkdir, and you only need:

cp etc/jupyter/jupyter_server_config.d/* ~/miniforge3/envs/xarray_leaflet/etc/jupyter/jupyter_server_config.d/

@jfoster17
Copy link

Hey @davidbrochart -- here are the issues I was talking about offline.

  1. Fails to deal with no data in the viewport

If the user moves the map outside of the region where there is data, xarray-leaflet hangs. For instance, starting the map in vector.ipynb so that NYC is not in the viewport:

m = Map(center=(45.74427755695474, -73.96981238541562), zoom=11, basemap=basemaps.CartoDB.DarkMatter, interpolation='nearest')

will cause the spinner to spin endlessly and nothing will ever display, even if you zoom over to New York City. You can get the same behavior moving away from a region where data has previously been rendered (perhaps a more likely scenario for most users).

@jfoster17
Copy link

  1. Fails to deal with E/W longitude swap

Again, on the vector.ipynb notebook if you specify the starting map location as positive longitude

m = Map(center=(40.74427755695474, 286), zoom=11, basemap=basemaps.CartoDB.DarkMatter, interpolation='nearest')

the map will be center on NYC but xarray-leaflet will fail to plot any data, probably because the data is in negative longitude (i.e. -73) and thus not in the range of the viewport (again, leading to infinite spinning). So this is really two problems, lack of wrapping longitude leading to the above-specified problem.

@davidbrochart
Copy link
Collaborator Author

Thanks Jonathan for reporting these issues.

@davidbrochart
Copy link
Collaborator Author

I released ipyleaflet v0.17.2 with the fix and require that version in xarray-leaflet.

@jfoster17
Copy link

Thanks for your quick response David -- indeed Issue 1 goes away in a new up-to-date env, and Issue 2 goes away for me too with the new ipyleaflet version!

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.

3 participants