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

Use anywidget for viewer without jupyter-server-proxy #219

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

Conversation

banesullivan
Copy link
Owner

@banesullivan banesullivan commented Jul 21, 2024

@manzt gave an excellent talk on anywidget at SciPy 2024 and I was blown away! Immediately, I saw a ton of potential for the widgets I work on across localtileserver and pyvista.

I thought I'd experiment with anywidget and see if I could use it to mitigate all of the issues around localtileserver's reliance on jupyter-server-proxy since there are countless examples of people struggling to get jupyter-server-proxy properly configured.

The changes in this PR are the result of a few hours of hacking with @manzt's help (thank you!!). This provides a fallback widget for the _ipython_display_ method on the TileClient class to immediately provide a working viewer for any raster after installation -- no configuration needed!

Ideally, I'd like to see if I can use the comms mechanism demonstrated here when creating a TileLayer with both ipyleaflet and folium. However, I couldn't figure out how to get this widget to talk to and add its TileLayer element to those widgets' maps... challenge for another day.

cc @giswqs

from localtileserver import examples
client = examples.get_bahamas()
client

Screenshot 2024-07-20 at 5 40 11 PM

@banesullivan banesullivan changed the title [wip] add anywidget for viewer without jupyter-server-proxy Add anywidget for viewer without jupyter-server-proxy Jul 21, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 7.54717% with 49 lines in your changes missing coverage. Please review.

Project coverage is 80.05%. Comparing base (e106125) to head (e543d12).
Report is 1 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #219      +/-   ##
==========================================
- Coverage   83.93%   80.05%   -3.89%     
==========================================
  Files          26       27       +1     
  Lines        1021     1073      +52     
==========================================
+ Hits          857      859       +2     
- Misses        164      214      +50     

@giswqs
Copy link

giswqs commented Jul 21, 2024

Wow, this is amazing! I am excited. This can potentially resolve the proxy issue once and for all🚀

@banesullivan banesullivan changed the title Add anywidget for viewer without jupyter-server-proxy Use anywidget for viewer without jupyter-server-proxy Jul 21, 2024
@giswqs
Copy link

giswqs commented Jul 21, 2024

I asked ChatGPT for help. See the conversations here. Not sure if it helps.

@giswqs
Copy link

giswqs commented Aug 14, 2024

This maybe relevant https://x.com/kylebarron2/status/1823849445575852135?s=46

@kylebarron
Copy link

This maybe relevant x.com/kylebarron2/status/1823849445575852135?s=46

Well @manzt had pointed me to this PR for reference, so we have some circular references going on 😉 .

Comment on lines +69 to +70
# NOTE: when calling from a thread executor, we need a different dataset handle for each thread
# finding that the kernel crashes... is rio-tiler or rasterio not thread safe?

Choose a reason for hiding this comment

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

No, definitely not thread safe by default. https://github.com/gjoseph92/stackstac/blob/main/stackstac/rio_reader.py Is a really cool implementation of threading for some specific rasterio drivers (e.g. COG)


# t = threading.Thread(target=target)
# t.start()
THREAD_EXECUTOR.submit(target)

Choose a reason for hiding this comment

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

I'm curious if you get much of a speedup with a threadpool executor. I would've thought the GIL would be held most of the time.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, I think I did notice a significant speed up using multiple threads -- I'll have to make a comparison exmaple

@kylebarron
Copy link

In the longer term, I'm curious if we can make this performant by having the python callback be async so that it's not blocking other tiles' requests. But that's a bit of work to get there.

@banesullivan
Copy link
Owner Author

In the longer term, I'm curious if we can make this performant by having the python callback be async so that it's not blocking other tiles' requests. But that's a bit of work to get there.

That's an interesting idea! I'll have to give this a try!

Hey @kylebarron, love seeing you here -- I've seen you up to some awesome work with Lonboard! I'm curious if you're looking at localtileserver (or rio-tiler more broadly) for some raster support in Lonboard? I'd love to contribute/help (albeit my time is incredibly limited these days)

@kylebarron
Copy link

Thank you! I'm bummed I missed the chance to say hello at scipy!

The prototype I have in lonboard uses rio-tiler to manage the abstraction of getting tiles to send to the frontend.

It's not really possible to make rio-tiler async, at least not with the gdal backend, so the long term goal is to have a stable async cog reader that we can use instead of gdal for COG inputs specifically

refactor: Remove use of anywidget-command kind
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.

5 participants