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

Added MathJax compatibility definitions #687

Merged
merged 4 commits into from Nov 13, 2017

Conversation

Projects
None yet
3 participants
@t-makaro
Copy link
Contributor

t-makaro commented Oct 7, 2017

Added definitions for \TeX and \LaTeX similar to those that MathJax uses to allow LaTeX to properly compile these. Fixes #262

t-makaro added some commits Oct 7, 2017

Added MathJax compatibility definitions
Added definitions for \TeX and \LaTeX similar to those that MathJax uses to allow LaTeX to properly compile these.
removed blank line
My first commit added the extra blank line, so this fixes it.

@takluyver takluyver added the LaTeX label Oct 9, 2017

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 12, 2017

won't these values need to change if we were to change the font metrics? (e.g., either by using a different fontface or a even different font size)? I don't like how MathJax enables this in the first place (with magic prespectified values that don't work on all fonts) so I'd prefer to have a more robust solution if we were going to implement it in nbconvert.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 12, 2017

The problem is the macros don't exist in mathmode, so rather than overwriting their definitions, check for them in the math content and replace \TeX with \textrm{\TeX}. Technically, we could just wrap all of the instances of \TeX, since in plain text it would still have the same meaning.

@t-makaro

This comment has been minimized.

Copy link
Contributor Author

t-makaro commented Oct 13, 2017

Font size appears to scale without issue for me with these definitions, but upon manually changing the font family in LaTeX I see that the \LaTeX does change, but it doesn't disfigure the text, and it stops the document from failing to compile. I'm unsure of how to make a filter to replace \TeX with \textrm{\Tex}. Would such a filter just need to be applied in the ((* block data_markdown -*)) block in the document_content.tplx file?

\textrm{\LaTeX} compiles inside and outside of math mode in XeLaTeX and pdfLaTeX without issue. \LaTeX outside of math mode in markdown in the notebook currently won't render the symbol, but the pdf conversion will. Ideally a filter would escape it outside of math mode and change to \textrm{LaTeX} inside math mode. But if we keep the current behaviour of changing it in the pdf conversion then a filter can make the change without detecting whether or not it is inside or outside of math mode.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 17, 2017

If you want to handle this in a template, look at how we handle the issue around include graphics to only display for 4/5's of the text width. You can use the same kind of logic to put the old command somewhere and overwrite the original command with this new one.

If you want to try the filter approach, just wrap all instances of \TeX and \LaTeX in \textrm whether or not they appear inside math. I like that better anyway. @takluyver do you have any thoughts on these 2 approaches?

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 17, 2017

Nope, I generally don't try to wrap my head around Latex stuff unless I have to. I'm happy to trust you on it.

@t-makaro

This comment has been minimized.

Copy link
Contributor Author

t-makaro commented Oct 17, 2017

I think that a filter that properly escapes when not in math mode would be best, but since this definition method doesn't do that either, I agree now a filter that just wraps all instances would be better than the definition method. I've looked through the code more, and I believe that I will be able to get a filter working.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 17, 2017

Ok, I'm not sure what you mean by "properly escapes when not in math mode"…

I'm concerned that doing too much parsing on the side of whether we are in math mode or not would be problematic & difficult in the general case. I am having difficulty thinking of much that we could do to separate those cases in general short of putting everything through a mistune like splitting of the math and markdown parsing. That might be the right thing to do… but that itself is the source of a tonne of our bugs, so I'm not convinced that context specific filtering is the right way to go.

But if you can figure out how to do it in the filtering way, I'm happy to look at this modified version!

@t-makaro

This comment has been minimized.

Copy link
Contributor Author

t-makaro commented Oct 17, 2017

What I mean by "escapes when not in math mode" is that currently when you type "\LaTeX" in a Jupyter markdown cell but not in math mode it will render this as plain text. When you then convert this to a pdf, the "\LaTeX" will be rendered as the LaTeX logo instead of plain text like the notebook does. I'll try to see if I can find a nice solution to this problem, but if not, I'll just make a filter that replaces all "\LaTeX" with "\textrm{\LaTeX}" and apply to just the markdown cells.

@t-makaro

This comment has been minimized.

Copy link
Contributor Author

t-makaro commented Oct 17, 2017

Actually, I've got another idea that doesn't use filters that could work. What if we redefine "\LaTeX" simply as "\textrm{\LaTeX}" directly in the latex template? I'm away from my primary computer, but I'll test it later. But maybe using a specific LaTeX macro could even get it to render as plain text outside of math mode without us needing to figure out what is and is not inside of math mode.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 17, 2017

That will create an infinite loop of definitions. That's why I pointed you to the include graphics example, that is one way around the problem.

As far as whether in the plaintext \LaTeX should be rendered, it would be better if that would work in the notebook outside of math mode.

I think we've been miscommunicating somewhat with regards to this because of a difference in how we approach this problem.
You are trying to make nbconvert behave more like the notebook front-end.
I'd like notebook front-end to behave more like nbconvert and traditional LaTeX.

It would be possible to catch bare references to \LaTeX surrounded by spaces (so in practice \LaTeX) and put those, under the hood, inside a MathJax rendering call.

Updated compatibility definitions
These new definitions allow for the original LaTeX and TeX logos inside of math mode.
@t-makaro

This comment has been minimized.

Copy link
Contributor Author

t-makaro commented Oct 17, 2017

I agree that it would be nice if the notebook acted more like LaTeX. I've updated the pull request to renew the commands using \let and \renewcommand to allow \LaTeX and \TeX to work exactly as normal but also inside of math mode.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 24, 2017

Yea… the only downside to this is that it is now (technically) impossible to write the string \LaTeX in plain text, but this is definitely the 80/20 solution (or more likely the 99/1 solution :P).

LGTM, unless anyone else has an issue, this is ready to merge.

@t-makaro

This comment has been minimized.

Copy link
Contributor Author

t-makaro commented Nov 13, 2017

Wasn't it already technically impossible to write "\LaTeX" in plain text? One would need to use "\textbackslash LaTeX". So I think the 99/1 solution is fine.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Nov 13, 2017

k merging!

@mpacer mpacer merged commit 4c4da32 into jupyter:master Nov 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mpacer mpacer added this to the 5.4 milestone Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment