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

🤖 hack together output retention in notebooks #105

Merged
merged 15 commits into from
Jul 29, 2022

Conversation

jvdd
Copy link
Member

@jvdd jvdd commented Jul 18, 2022

This PR does;

  • retain some output of FigureResampler when notebook is disconnected
  • test the above (just 1 minimal test that runs the code)
  • monitor browser logs in selenium tests (Improve selenium tests #16)

Look more into;

  • rethink the _ipython_display_ for FigureResampler? @jonasvdd
  • Incorporating this into the example notebooks

How does this work;

This PR adds additional JavaScript code to the IPython.display.display method, which contains a base64 bytarray image of the shown figure. When the dash app is not reachable, the image is shown instead of the iframe (which embedded the connected dash app). This works because the JavaScript in the notebook output cells gets executed each time a notebook is loaded (:exclamation: the kernel should not be running for this to happen).

How is this implemented;

We create a JupyterDash subclass which extends the "inline" visualization capabilities with the functionality described above.
This extension is triggered when *"inline_persistent" *is used in the FigureResampler.show_dash method

  • we add a unique _uid to each object
  • we add a new endpoint to the underlying flask app that
    1. is only accessible via the corresponding app its _uid
    2. hasCORS rights for any origin and 'Content-Type' headers
      Note that this is the only CORS endpoint of the JupyterDash app & is only preset when "inline_persistent" is used!
  • we check in the JavaScript output of the notebook cell whether that endpoint is reachable and emits the expected message (i.e., "Alive")
    • if check is successful => display an iframe of the running Jupyter dash app
    • if check is not successful => display a static image

@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2022

Codecov Report

Merging #105 (bc0aabc) into main (c8de84b) will decrease coverage by 0.30%.
The diff coverage is 90.00%.

@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
- Coverage   97.94%   97.63%   -0.31%     
==========================================
  Files          10       10              
  Lines         777      804      +27     
==========================================
+ Hits          761      785      +24     
- Misses         16       19       +3     
Impacted Files Coverage Δ
...tly_resampler/figure_resampler/figure_resampler.py 89.10% <90.00%> (-0.09%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@jvdd jvdd marked this pull request as ready for review July 19, 2022 19:03
@jvdd
Copy link
Member Author

jvdd commented Jul 19, 2022

@jonasvdd, if you're not keen on adding kaleido as dependency I can look into serializing the figure into html and render this instead when the dash app is not reachable

@jvdd jvdd requested a review from jonasvdd July 19, 2022 19:45
Copy link
Member

@jonasvdd jonasvdd left a comment

Choose a reason for hiding this comment

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

LGTM, just the kaleido and the docs, but the latter can be tackled in #104

pyproject.toml Outdated
@@ -18,6 +18,8 @@ orjson = "^3.6.8" # faster json serialization
pandas = "^1.3.5"
trace-updater = ">=0.0.8"
numpy = ">=1.21.0"
Flask-Cors = "^3.0.10"
kaleido = "0.2.1"
Copy link
Member

Choose a reason for hiding this comment

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

Can this be made optional?

Copy link
Member

Choose a reason for hiding this comment

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

The only places we interact with kaleido is in pyproject.toml (and the underlying plotly code). So this can be made optional i.m.o. and we should just document this properly in our docs. #104

Copy link
Member

Choose a reason for hiding this comment

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

added it as a dev dependency and updated the docs of the FigureResampler.show_dash

@jonasvdd jonasvdd self-assigned this Jul 28, 2022
@jonasvdd jonasvdd added the enhancement New feature or request label Jul 28, 2022
@jonasvdd jonasvdd merged commit 5b7bcd0 into main Jul 29, 2022
@jvdd jvdd deleted the hacking_notebook_output branch December 12, 2022 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants