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

🐛 FIX: Update priority for LaTeX output for html and latex builders #305

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mmcky
Copy link
Member

@mmcky mmcky commented Mar 1, 2021

This PR increases the priority for text/latex representations for html and latex builders.

For html -- mathjax renders the text/latex representation more cleanly than typical images provided by some underlying packages (i.e. for png images) so promoted to be below image/svg+xml but above other image types.

For latex -- native latex syntax should be used in preference.

@mmcky mmcky changed the title FIX: Update priority for native LaTeX output for html and latex builders FIX: Update priority for LaTeX output for html and latex builders Mar 1, 2021
@mmcky
Copy link
Member Author

mmcky commented Mar 1, 2021

@mmcky mmcky changed the title FIX: Update priority for LaTeX output for html and latex builders 🐛 FIX: Update priority for LaTeX output for html and latex builders Mar 1, 2021
@chrisjsewell
Copy link
Member

I don't think latex should be above svg

@codecov
Copy link

codecov bot commented Mar 1, 2021

Codecov Report

Merging #305 (37d949b) into master (2baade0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #305   +/-   ##
=======================================
  Coverage   87.35%   87.35%           
=======================================
  Files          12       12           
  Lines        1368     1368           
=======================================
  Hits         1195     1195           
  Misses        173      173           
Flag Coverage Δ
pytests 87.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
myst_nb/render_outputs.py 86.79% <ø> (ø)

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 2baade0...37d949b. Read the comment docs.

@mmcky
Copy link
Member Author

mmcky commented Mar 1, 2021

I don't think latex should be above svg

hey @chrisjsewell can you explain why? (i.e. page load times?) mathjax seems to do a nicer job of displaying the math. I am certainly open to demoting below svg but just want to understand why you think that is a better default priority.

For latex I think it certainly should be the highest output priority for native rendering.

@chrisjsewell
Copy link
Member

Because svg is natively supported by HTML, latex is not

@mmcky
Copy link
Member Author

mmcky commented Mar 1, 2021

Because svg is natively supported by HTML, latex is not

roger that.

@mmcky
Copy link
Member Author

mmcky commented Mar 1, 2021

@chrisjsewell I see epub is listed as a builder in the builder option list.

Do you know if epub has a mechanism to support text/latex like mathjax for html?

Hmm -- I think epub uses mathml for math but also supports svg natively so another good reason to promote image/svg+xml. I guess the question is what happens if latex is passed before png.

Open to suggestions but perhaps we should:

  • create a default priority for html based builders
  • create a default priority for epub builder (separate like LaTeX)

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.

2 participants