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

Don't load any html in the notebook when no logo is required #5216

Merged
merged 3 commits into from
Mar 31, 2022

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Feb 21, 2022

This avoids calling publish_display_data in a notebook when calling hv.extension() with logo=False because unfortunately in JupyterLab each call to publish_display_data ends up adding an empty line.

The call to publish_display_data loads HTML from the load_notebook.html template which only contains the logos base64 encoded. So it seems safe to avoid loading this HTML when no logo is required.

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2022

Codecov Report

Merging #5216 (3420527) into master (514f08c) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #5216   +/-   ##
=======================================
  Coverage   86.75%   86.75%           
=======================================
  Files         298      298           
  Lines       62162    62162           
=======================================
  Hits        53929    53929           
  Misses       8233     8233           
Impacted Files Coverage Δ
holoviews/ipython/__init__.py 76.21% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 514f08c...3420527. Read the comment docs.

@maximlt
Copy link
Member Author

maximlt commented Feb 22, 2022

Two checks were cancelled because they timed out after 1hr. Looking forward to the new libmamba solver to speed things up, and maybe the caching Mridul has started to implement in hvPlot.

@@ -194,7 +194,7 @@ def __call__(self, *args, **params):
"hv-extension-comm")

# Create a message for the logo (if shown)
if not same_cell_execution:
if not same_cell_execution and p.logo:
self.load_hvjs(logo=p.logo,
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that panel handles the JS now, I guess this is now misnamed (an artifact of history). Could you double check it is only used to load HTML and if so, rename it to self.load_extension_html (or similar)?

@jlstevens
Copy link
Contributor

Looks good to me!

@jlstevens jlstevens merged commit 1f112eb into master Mar 31, 2022
@maximlt maximlt deleted the dont_load_when_nologo branch November 20, 2022 10:45
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