nglviewer / nglview

Jupyter widget to interactively view molecular structures and trajectories
http://nglviewer.org/nglview/latest/
Other
816 stars 133 forks source link

Run all archived messages when embedded widget is uninitialized #1064

Closed Yoshanuikabundi closed 1 year ago

Yoshanuikabundi commented 1 year ago

This PR implements the proposal discussed in #1063.

I made two changes that weren't discussed there:

  1. _ngl_is_initialized is set in createStage, not handleEmbed, to ensure it is always set when the JS runs (but also after handleEmbed)
  2. I had to make on_msg async. This is because it is essential that each message completes before the next one is triggered to avoid things like trying to add representations to a missing component. I learnt this the hard way. My understanding is that this won't change behavior - on paths where on_msg doesn't await, execution will never be suspended and thus the function will run synchronously; on the other hand, when the function does await it's the last thing it does, so the only difference is that the on_msg function sticks around on the queue instead of the already async inner function. What it does do is allow on_msg to be awaited so that messages can be executed and completed in order. This may be of use elsewhere, I'm not sure - I feel like NGLView does sometimes "forget" something I told it.

I also added the jupyterlab~=3.0 pin to the Conda environment as discussed.

I have a lot of files that have been generated by NPM (or something), including three that are already checked in - so I've added them in a separate commit in case this is a mistake of some sort.

    modified:   nglview/static/index.js
    modified:   nglview/static/index.js.map
    modified:   nglview/staticlab/package.json

Hope this is useful! I think from what I've learned I can hack something together for my cookbook even if this doesn't get merged, so thanks for your help!

Closes #1063

hai-schrodinger commented 1 year ago

Does this PR actually work as expected?

Yoshanuikabundi commented 1 year ago

Yep, seems to! I was surprised too 😂 I made a notebook that creates a widget, clears it's representations and adds new ones. It appears correctly when executed either with jupyter nbconvert --execute --to html notebook.ipynb or in JupyterLab. I did have to manually copy over the compiled JavaScript file but my understanding is thats just how the whole embedded widget packaging thing works. I'll put together a more comprehensive demo today.

Yoshanuikabundi commented 1 year ago

Sorry Hai, I'm not going to have time to put that more comprehensive demonstration together this week. Here's the test I was developing off of, along with the compiled JS and the converted HTML file - let me know what more you need to review this!

pr-demo.zip

hainm commented 1 year ago

I have a lot of files that have been generated by NPM (or something), including three that are already checked in - so I've added them in a separate commit in case this is a mistake of some sort.

  modified:   nglview/static/index.js
  modified:   nglview/static/index.js.map
  modified:   nglview/staticlab/package.json

yeah, those files are intentional to be included in nglview folder so we don't need npm to install the dev nglview python package.

Yoshanuikabundi commented 1 year ago

OK I've addressed those points! One thing I've noticed is that js/package.json still thinks its 3.0.1 - does that need to be updated?

hainm commented 1 year ago

One thing I've noticed is that js/package.json still thinks its 3.0.1 - does that need to be updated?

I will do when doing release.

hainm commented 1 year ago

Thanks @Yoshanuikabundi very much for your contribution.