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

Update examples #141

Merged
merged 3 commits into from
Nov 22, 2024
Merged

Update examples #141

merged 3 commits into from
Nov 22, 2024

Conversation

johnkit
Copy link
Collaborator

@johnkit johnkit commented Nov 21, 2024

Update pangeo and esgf examples to use latest config syntax and add some markdown cells

Pangeo Example

ESGF Example

@jourdain
Copy link
Collaborator

I don't think you want to trigger a new PyPI release of pan3d for examples/docs update. You should use a commit message like docs(examples): update pangeo example to enable viz

@johnkit johnkit force-pushed the update-examples branch 2 times, most recently from 0dcfff7 to fe2a90d Compare November 21, 2024 17:17
@johnkit
Copy link
Collaborator Author

johnkit commented Nov 21, 2024

Hacked in a commit to detect binder and hard-code local/vtkjs rendering. Good enough enable notebooks to work in binder. I didn't use the has_gpu_rendering() method in utils, which is outdated. I'll look into VTK for the general approach.

@@ -85,9 +86,11 @@ def __init__(self, server=None, local_rendering=None):
)
args, _ = self.server.cli.parse_known_args()
self.local_rendering = local_rendering
if args.wasm:
if os.environ.get("BINDER_REQUEST") is not None:
self.local_rendering = "vtkjs"
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you want to use vtkjs instead of wasm for binder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On my first test w/WASM (before I added this commit, I used the local_rendering argument), the renderwindow didn't display. Definitely possible I overlooked something, so I'll change to wasm and retest.

Copy link
Collaborator Author

@johnkit johnkit Nov 21, 2024

Choose a reason for hiding this comment

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

So "vtkjs" is ok but "wasm" isn't. It looks like part or all of trame_vtklocal isn't getting installed, or is installed in the wrong place. I'll paste the js console messages below, but basically Jupyter aborts because it does not find __trame_vtklocal/js/style.css.

index-de5ee105.js:1 
        
        
       GET https://hub.binder.opensci.2i2c.cloud/user/kitware-pan3d-irtfxelw/trame-jupyter-server/servers/esgf-viewer/__trame_vtklocal/js/style.css net::ERR_ABORTED 404 (Not Found)
(anonymous) @ index-de5ee105.js:1
mo @ index-de5ee105.js:1
Ce @ index-de5ee105.js:1
So @ index-de5ee105.js:1
Fs @ index-de5ee105.js:23
await in Fs
(anonymous) @ index-de5ee105.js:23
index-de5ee105.js:23 Uncaught (in promise) Event {isTrusted: true, type: 'error', target: link, currentTarget: null, eventPhase: 0, …}
Fs @ index-de5ee105.js:23
await in Fs
(anonymous) @ index-de5ee105.js:23

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want os.environ.get("BINDER_REQUEST") to superseed user input for local rendering request?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I know why wasm could be missing in the install.

Let's add vtklocal.initialize(self.server) somewhere since the widget get conditionally instantiated.

@johnkit johnkit force-pushed the update-examples branch 2 times, most recently from da51ac5 to b84e6d7 Compare November 21, 2024 19:55
@jourdain
Copy link
Collaborator

I think the 2 pending changes are:

  • Should the binder selection to local rendering be more of a fallback than an actual override.
  • Fix wasm to properly expose itself to overcome the conditional instantiation. (but I'm still ensure why it is working on my end with binder)

@jourdain jourdain merged commit fd92567 into main Nov 22, 2024
4 checks passed
@jourdain jourdain deleted the update-examples branch November 22, 2024 22:08
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.

2 participants