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

Force loading MathJax on HTML pages generated from notebooks #551

Merged
merged 2 commits into from
Apr 28, 2021

Conversation

mgeier
Copy link
Member

@mgeier mgeier commented Apr 5, 2021

Fixes #550.

See also sphinx-doc/sphinx#6981.

@casperdcl
Copy link

Haven't reviewed the codebase fully but on first reaction, I'd say unconditionally (even where no \\begin\{(equation|align) is present) enabling isn't a good idea

@mgeier
Copy link
Member Author

mgeier commented Apr 7, 2021

@casperdcl I initially wanted to only handle display equations (which are currently implemented using the :nbsphinx-math: role, which is a hack anyway), but I couldn't find a way to do this in the current implementation.

Do you have an idea for an alternative way of fixing #550?

Either way, please note that this PR also solves #369.
This enables MathJax in any kind of (HTML-based) plots and tables and similar cell outputs.

If MathJax is not loaded unconditionally, I would have to load it on each notebook which has at least one non-text cell output, which would still be nearly every notebook.
The problem is that nbsphinx cannot know whether a cell output contains equations or not.

@casperdcl
Copy link

The problem is that nbsphinx cannot know whether a cell output contains equations or not.

I would expect a robust solution would be for nbsphinx to check this using e.g. the regex I posted.

Anyway I'm personally fine with unconditional loading... just pointing out that it seems a bit presumptuous/potentially inefficient & unneeded. Maybe make it a config option?

@mgeier
Copy link
Member Author

mgeier commented Apr 8, 2021

I'm aware that this might be unnecessarily loading MathJax in some cases.
But in case of doubt I'd like to err in favor of correctness instead of efficiency.

I think using the suggested regex is not very robust, because IIRC there might be more environments than equation and align and also inline $...$ should be supported (because it is also supported in the Jupyter Notebook app).
I think checking for this via regex will be very brittle.

Maybe make it a config option?

Any ideas how this would work concretely?

I think this PR is better (because it's more correct) than the status quo, but I'm open for suggestions (or future PRs) for avoiding to load MathJax unnecessarily.

src/nbsphinx.py Outdated

def apply(self):
env = self.document.settings.env
env.get_domain('math').data['has_equations'][env.docname] = True
Copy link

@casperdcl casperdcl Apr 8, 2021

Choose a reason for hiding this comment

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

Any ideas how this would work concretely?

being able to set this in conf.py, i.e. nbsphinx_force_mathjax = False and then

Suggested change
env.get_domain('math').data['has_equations'][env.docname] = True
if nbsphinx_force_mathjax:
env.get_domain('math').data['has_equations'][env.docname] = True

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion!

I've implemented it in 001db3d, but since strictly speaking this isn't limited to MathJax (which is just an implementation detail), I've chosen the more generic name nbsphinx_assume_equations.

@casperdcl
Copy link

btw on a related note (maybe for a different issue?) placing maths in italics doesn't render:

$works$
*$doesn't work$, producing a literal ":math:`doesn't work`" instead*

@mgeier
Copy link
Member Author

mgeier commented Apr 25, 2021

btw on a related note (maybe for a different issue?) placing maths in italics doesn't render:

$works$
*$doesn't work$, producing a literal ":math:`doesn't work`" instead*

I think this comes from the known limitation that reST doesn't support nested markup.
This problem will be easier to solve once #36 is done, but don't hold your breath!

See also https://docutils.sourceforge.io/FAQ.html#is-nested-inline-markup-possible.

See also #301, #455 and #468 (comment).

@mgeier mgeier merged commit 5780328 into spatialaudio:master Apr 28, 2021
@mgeier mgeier deleted the force-mathjax branch April 28, 2021 15:04
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.

mathjax.js missing in html with pure display equations
2 participants