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

use mathjax config to left-align output #5371

Closed
wants to merge 2 commits into from

Conversation

minrk
Copy link
Member

@minrk minrk commented Mar 18, 2014

instead of css

closes #5365

@ahmadia
Copy link
Contributor

ahmadia commented Mar 18, 2014

Hmnn, I believe the .onload hack was in there to prevent MathJax from trying to render cells before the page was loaded. If the notebook cells are constructed dynamically after MathJax loads, they won't be rendered unless they fire MathJax render calls after they're constructed.

@damianavila
Copy link
Member

The onload hack it is necessary... if you don't load MathJax at the end, and try to load some other js libraries too (ie. reveal.js), some math is not rendered OK.

@minrk
Copy link
Member Author

minrk commented Mar 18, 2014

It's not relevant to this PR, so I have removed the mathjax patch here. We can continue the discussion of that in #5375.

@damianavila
Copy link
Member

OK, for this one I am 👍

@ellisonbg
Copy link
Member

Do we need to make s similar change to nbviewer and nbconvert to make sure equations there are handled the same way?

@ellisonbg
Copy link
Member

I just did a test and nbconvert/nbviewer latex output will be centered by this PR. We will have to update the MathJax config/calling there as well to get this behavior with no CSS. We keep running into this issue: because we config/call MathJax in multiple places, it is easy for them to fall out of sync. Can we consolidate this logic into one place?

@ellisonbg
Copy link
Member

Rather than having this call to typesetting save and reset state, what if we simply had each call to typesetting in our code base first make a call to Hub.Config to set its options. That would allow each call to not worry about managing state.

@minrk
Copy link
Member Author

minrk commented Mar 20, 2014

I don't know how expensive calling config is for ever render. It makes more sense to me to only call it when deviating from the norm, but we can do it every time if you prefer.

@ellisonbg
Copy link
Member

Here are the three places that we using MathJax to typeset:

./IPython/html/static/notebook/js/cell.js:
  189 :             MathJax.Hub.Queue(["Typeset", MathJax.Hub, cell_math]);
./IPython/html/static/notebook/js/outputarea.js:
  208 :             MathJax.Hub.Queue(["Typeset",MathJax.Hub]);
./IPython/html/static/widgets/js/widget_string.js:
   49 :             MathJax.Hub.Queue(["Typeset",MathJax.Hub,this.$el.get(0)]);

It is not at all obvious to me which of these is the "normal" case.

With this PR, the one in OutputArea has to call Hub.Config twice for each output. If someone has a notebook with lots of LaTeX output, but none in MD cells, this approach is more expensive. Given that we won't know in advance how often a user would use the different typesetting code paths, I think putting a call to Hub.Config on each of them will provide better performance averaged over all usage cases.

Do you think this analysis is right? Trying to convince myself...

@minrk
Copy link
Member Author

minrk commented Mar 20, 2014

By 'normal', I just mean based on the following statements: In notebooks, equations and images are centered. The exception to this is when those things are outputs, in which case they are left-aligned.

@minrk
Copy link
Member Author

minrk commented Mar 20, 2014

I don't have a strong feeling about it, so if attaching a mathjax_config to each class, used to update every time we typeset, that might be appropriate.

This stuff is weird and fragile enough that I think maybe we should hold it until after 2.0. The only (known) adversely affected case in master is very long lines that auto-line-break won't break (e.g. long tuples or lists in sympy).

@ellisonbg
Copy link
Member

I am fine holding this until after 2.0. Expecially because we still have to
figure out how to apply this logic to nbconvert/nbviewer.

On Thu, Mar 20, 2014 at 3:07 PM, Min RK notifications@github.com wrote:

I don't have a strong feeling about it, so if attaching a mathjax_configto each class, used to update every time we typeset, that might be
appropriate.

This stuff is weird and fragile enough that I think maybe we should hold
it until after 2.0. The only (known) adversely affected case in master is
very long lines that auto-line-break won't break (e.g. long tuples or lists
in sympy).

Reply to this email directly or view it on GitHubhttps://github.com//pull/5371#issuecomment-38227124
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg ellisonbg added this to the 3.0 milestone Mar 26, 2014
@ellisonbg
Copy link
Member

Recent work on master has added a number of new calls to MathJax in the widgets. All of these additional calls to MathJax should be left aligned. Because of this I think the approach that is in this PR is fine.

However, we do need to make sure that nbconvert output and nbviewer will still center output equations after this PR.

@ellisonbg
Copy link
Member

OK, my last comment was pure craziness. I forgot, the global config for MathJax is centering and this PR gets left alignment in output areas by calling MathJax's config logic (rather than the older CSS approach). Right now if this PR is merged, it will mean that all LaTeX in widget descriptions will be center aligned. I am not sure that makes sense.

@minrk why don't you rebase on master so we can test the widget LaTeX stuff with this PR.

@ellisonbg
Copy link
Member

Looking at how MathJax is configured on nbconvert/nbviewer and run globally there, I don't see how we can make this approach work there. I think we should reconsider possibly just using centered equations everywhere. Adding to the next dev meeting agenda.

@ellisonbg
Copy link
Member

We discussed this one at the dev meeting on June 12, but didn't put a note here. I just looked back and we said that "Min will investigate how to get this working in nbconvert by setting the MathJax config separately for output LaTeX". I think the issue with the current approach is that it won't give the same result on nbviewer/nbconvert.

@minrk
Copy link
Member Author

minrk commented Jul 26, 2014

Closing here, I might come back to it later.

@minrk minrk closed this Jul 26, 2014
@minrk minrk modified the milestones: 3.0, no action Feb 14, 2015
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.

Regression on displaying wide LaTeX output
4 participants