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

Add a KaTeX and MathJax based LaTeX pane #311

Merged
merged 7 commits into from Mar 17, 2019
Merged

Add a KaTeX and MathJax based LaTeX pane #311

merged 7 commits into from Mar 17, 2019

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Mar 16, 2019

I think this should probably replace the LaTeX pane because it is much more capable. That said I still need to assess how expensive it would be to load KaTeX all the time, if it's too large I will likely have to let the user request it explicitly using pn.extension('katex') (not required on bokeh server or when saving).

Screen Shot 2019-03-16 at 2 45 36 AM

Copy link
Member

@jbednar jbednar left a comment

Looks great! Much better than my matplotlib-based hack. I'm happy for it to replace the LaTeX pane unless there are cases where the image-based approach is more general.

@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Mar 16, 2019

katex.min.js + katex.min.css are ~137 KB combined. Is it worth it to always bundle that or should I require the user to run pn.extension('katex') before using the component in the notebook? A helpful warning would tell the user they forgot to do so. I'm on the edge, I don't quite think using LaTeX is common enough to make everyone pay a 137 KB penalty. If I make it optional I could also allow toggling between MathJax and KaTeX and use whichever one is loaded.

@jbednar
Copy link
Member

@jbednar jbednar commented Mar 16, 2019

I'd say it's worth saving the 137KB at a cost of having to load the extension explicitly

@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Mar 17, 2019

Okay, I've now implemented MathJax as well and refactored in such a way that the LaTeX pane will use either the MathJax or the KaTeX model depending on which was loaded. If an explicit renderer is defined on the pane it will use that and if neither is loaded it will warn.

@philippjfr philippjfr changed the title Add a KaTeX based LaTeX pane Add a KaTeX and MathJax based LaTeX pane Mar 17, 2019
@codecov-io
Copy link

@codecov-io codecov-io commented Mar 17, 2019

Codecov Report

Merging #311 into master will increase coverage by <.01%.
The diff coverage is 92.59%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #311      +/-   ##
==========================================
+ Coverage   89.28%   89.29%   +<.01%     
==========================================
  Files          65       67       +2     
  Lines        6617     6622       +5     
==========================================
+ Hits         5908     5913       +5     
  Misses        709      709
Impacted Files Coverage Δ
panel/models/katex.py 100% <100%> (ø)
panel/io.py 66.77% <100%> (ø) ⬆️
panel/pane/markup.py 92.85% <100%> (+0.1%) ⬆️
panel/tests/test_panes.py 93.84% <100%> (+0.4%) ⬆️
panel/models/mathjax.py 100% <100%> (ø)
panel/pane/equation.py 77.08% <84.61%> (-6.5%) ⬇️

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 18c71b6...2d11048. Read the comment docs.

@philippjfr philippjfr merged commit 16f32ea into master Mar 17, 2019
3 checks passed
@philippjfr philippjfr deleted the katex branch Sep 9, 2019
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.

None yet

3 participants