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

Improvements and Documentation for Panes #313

Merged
merged 11 commits into from
Mar 18, 2019
Merged

Improvements and Documentation for Panes #313

merged 11 commits into from
Mar 18, 2019

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Mar 16, 2019

This PR provides polish to all existing pane classes and then adds reference sections for each. I'll outline all the panes and the improvements made here:

  • HoloViews
  • Plotly
  • Vega
    • Ensure it is rendered correctly on initialization
    • Add reference gallery entry

@philippjfr philippjfr force-pushed the refactor_panes branch 2 times, most recently from 47658b4 to 938c35f Compare March 16, 2019 23:20
@codecov-io
Copy link

codecov-io commented Mar 17, 2019

Codecov Report

Merging #313 into master will decrease coverage by 0.09%.
The diff coverage is 75.75%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #313     +/-   ##
========================================
- Coverage   89.29%   89.2%   -0.1%     
========================================
  Files          67      67             
  Lines        6622    6696     +74     
========================================
+ Hits         5913    5973     +60     
- Misses        709     723     +14
Impacted Files Coverage Δ
panel/tests/test_plotly.py 97.46% <100%> (ø) ⬆️
panel/models/plotly.py 100% <100%> (ø) ⬆️
panel/models/mathjax.py 100% <100%> (ø) ⬆️
panel/models/katex.py 100% <100%> (ø) ⬆️
panel/models/vega.py 100% <100%> (ø) ⬆️
panel/tests/test_holoviews.py 99.25% <100%> (+0.03%) ⬆️
panel/pane/vega.py 78.37% <100%> (ø) ⬆️
panel/io.py 63.69% <33.33%> (-3.09%) ⬇️
panel/pane/plotly.py 84.44% <80.64%> (-2.52%) ⬇️
panel/pane/holoviews.py 88.99% <87.5%> (+5.1%) ⬆️
... and 2 more

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 16f32ea...70c106e. Read the comment docs.

@philippjfr
Copy link
Member Author

So after refactoring the various models I now realize that loading the JS in this does not work for the classic notebook as noted in bokeh/bokeh#5227.

Not sure what to do about this yet, but it may require a fallback condition implemented using jQuery or similar.

@philippjfr
Copy link
Member Author

philippjfr commented Mar 17, 2019

In the worst case scenario which is that we find no non-hacky solution that could be added to bokeh itself I will likely just insert this hack in our JS:

if (window.requirejs) {
  window._define = window.define;
  // Stub does not define amd and is therefore skipped by UMD loaders
  // however Notebook bundle does not check for window.define.amd
  function _define_stub(name, deps, callback) {
    window._define(name, deps, callback)
  }
  window.define = _define_stub;
  function _reset_define() {
    window.define = window._define;
  }
  window._bokeh_onload_callbacks.push(_reset_define)
}

It temporarily replaces window.define with a stub which is not recognizable by a UMD loader but will still be used to fetch the classic notebook webpack bundle.

@philippjfr
Copy link
Member Author

philippjfr commented Mar 18, 2019

I decided to trial a fix for the issue loading dependencies in classic notebooks by adding a __js_require__ attribute to custom models, which is used in the autoload_panel_js.js template to generate a requirejs call which satisfies the dependencies. If this works well for us I'll fully flesh out the __js_require__ API and contribute it upstream.

@xavArtley If you want the VTK model to work in the classic notebook you'll be able to add something like this to your VTK model to load vtk.js correctly:

__js_require__ = {"paths": {"vtk": "//unpkg.com/vtk.js@8.3.15/dist/vtk"},
                  "shim": {"vtk": {"exports": "vtk"}}}

@philippjfr philippjfr merged commit 94e3919 into master Mar 18, 2019
@philippjfr philippjfr deleted the refactor_panes branch September 9, 2019 16:37
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