Fallback to the string printer if the latex printer raises an exception #65

asmeurer · 2015-10-23T22:36:52Z

Currently it just prints an error and gives an empty png. However, there is a
lot of LaTeX that matplotlib cannot parse, e.g., matrices. This gives a better
user experience for SymPy, which has a text printer that it can fallback
to. SymPy cannot just disable the LaTeX printer because it is desired for the
notebook, and there is no way for it to enable LaTeX just in the notebook, or
change the printer precedence order.

I have left in the debug logging of the error (to the terminal).

Fixes #63. This supersedes #57, although I don't know if it fixes the issue
there.

Currently it just prints an error and gives an empty png. However, there is a lot of LaTeX that matplotlib cannot parse, e.g., matrices. This gives a better user experience for SymPy, which has a text printer that it can fallback to. SymPy cannot just disable the LaTeX printer because it is desired for the notebook, and there is no way for it to enable LaTeX just in the notebook, or change the printer precedence order. I have left in the debug logging of the error (to the terminal). Fixes jupyter#63. This supersedes jupyter#57, although I don't know if it fixes the issue there.

asmeurer · 2015-10-23T22:37:53Z

@flying-sheep would love to hear if this fixes your issues or not.

I didn't see any tests for latex, so didn't add any for this fix (because I'm not sure how).

flying-sheep · 2015-10-25T13:37:10Z

hmm, no

asmeurer · 2015-10-26T00:14:21Z

I'm unclear what exactly is happening in your case, then.

ccordoba12 · 2015-12-07T15:51:50Z

@flying-sheep, please clarify.

flying-sheep · 2015-12-07T18:25:21Z

i installed the qtconsole dependencies in an almost clean venv (i symlinked my system pyqt)
i installed this branch of qtconsole
i started jupyter qtconsole --kernel=ir
i see what i show you in the screenshot

asmeurer · 2015-12-07T18:33:01Z

@flying-sheep is any error information printed to the terminal? Maybe the problem in your case is that the latex rendering "fails" but doesn't register as a failure (raise an exception) to the qtconsole. If it did, it would fallback to the text renderer in this PR.

flying-sheep · 2015-12-07T19:07:07Z

nothing.

Carreau · 2016-01-06T07:43:40Z

Oops, slipped through te cracks with all the repos.

I'm fine with that.

flying-sheep · 2016-01-06T14:34:41Z

excuse me: what is it that you’re fine with? 😆

Carreau · 2016-01-07T09:57:28Z

excuse me: what is it that you’re fine with?

The PR in general if it helps. The QtConsole is in low maintnance mode and few of the core dev use it.
So most PR should be merge accepted quickly.

Frankly the QtConsole. should be replaced by a QtWebkitView that actually support a local mathjax install + codemirror/Ace.

asmeurer · 2016-01-07T12:46:07Z

FYI the QTConsole does have users, as evidenced by the multiple bug reports we've received from SymPy for this bug.

Also, in case you missed it, this PR and #57 are mutually exclusive with one another. You guys need to pick one or the other (or suggest a third alternative).

flying-sheep · 2016-01-07T12:53:35Z

well, my problem as stated is that LaTeX → PNG isn’t a good solution to default to.

you can’t select & copy parts of the output, and it’s more overhead than simple text→text transformations.

if the explicit plan is to replace the QtConsole frontend with a QWebView based one, i’m all for merging this and moving on, but if this is going to stay for a longer time, it’s pretty ugly for IRKernel users.

Carreau · 2016-01-07T13:11:33Z

FYI the QTConsole does have users, as evidenced by the multiple bug reports we've received from SymPy for this bug.

Yes I know. I'm just stating that it receives few patch from the core team. I would have no issues giving @flying-sheep , @asmeurer and @ccordoba12 commit rights if it makes things easier for you guys. (As long as a couple of other dev agrees also). You seem to be the one both using Qtconsole, receiving bugs report,and writing patches, so it seem fair to me.

[edit] actually @ccordoba12 already have write access, (but not admin). [/edit]

Also, in case you missed it, this PR and #57 are mutually exclusive with one another. You guys need to pick one or the other (or suggest a third alternative).

Yes i though the incompatibility and why I commented on the other that I think that even partial support is better than none.

minrk · 2016-01-07T13:13:42Z

I'm torn on this one. My current inclination is to go with this patch and allow explicit disable of latex via preferences.

ccordoba12 · 2016-01-07T13:32:29Z

@Carreau, I already have commit rights (@minrk granted them to me :-)

I'm +1 with @minrk proposal, I think it's a good compromise. I'll submit a PR with the option to disable Latex, if @flying-sheep and @asmeurer agree.

Please let me test and merge this one, I promise to do it this weekend ;-)

asmeurer · 2016-01-07T13:38:00Z

That sounds good to me too.

(and btw I don't personally use the qtconsole; just fixing an issue for upstream SymPy)

asmeurer · 2016-01-07T13:50:04Z

@ccordoba12 #63 has steps to reproduce the SymPy issue for when you test this.

minrk · 2016-01-07T14:56:32Z

@ccordoba12 go ahead and merge when you are ready.

flying-sheep · 2016-01-07T16:37:12Z

well, disabling latex is already possible from the kernel, so two switches are probably confusing.

another idea: maybe we should hardcode a list of which kernels interpret “text/latex” like IPython does.

generally any MIME type means of course everything compliant to the language/format specified by that MIME type

but IPython means with “text/latex”: “a math subset of LaTeX that is roughly equivalent to what MathJax can do”

i don’t think we should accept all kernels to interpret the latex MIME type like IPython. it would be a hack but if IPython wants to keep doing that i think we should handle it in a special way.

ccordoba12 · 2016-01-07T22:33:16Z

@flying-sheep, how latex can be deactivated from the kernel?

flying-sheep · 2016-01-08T08:37:10Z

options(jupyter.plot_mimetypes = c('text/plain', 'image/png', 'image/svg+xml'))

i think that’s already the default by now…

takluyver · 2016-01-13T10:28:39Z

qtconsole/rich_jupyter_widget.py

+                    self._append_latex(data['text/latex'], True)
+                except Exception as e:
+                    self.log.error("Failed to render latex: '%s'", data['text/latex'], exc_info=True)
+                    self.log.error("Failed to render latex: %s" % e)


I think the second log call is redundant here - exc_info=True in the first one should log the the error that was caught.

I pushed a fix for that.

exc_info=True already does this.

asmeurer · 2016-01-13T15:23:57Z

I just tested this, and it seems that the exception is printed in the qtconsole, which I believe wasn't the case when I originally made this. Any idea why that would be happening, and how to disable it? I only want the error to be logged to the terminal.

minrk · 2016-01-13T16:10:45Z

@asmeurer are you sure the error isn't raised in the kernel? When I run the sample, sympy is computing the PNG kernel-side, so an error would logically be shown in the qtconsole.

asmeurer · 2016-01-13T16:12:46Z

I have no idea what that even means, so ¯_(ツ)_/¯

minrk · 2016-01-13T16:19:03Z

When I run the code (sympy 0.7.6.1):

from sympy import *
init_session()
Matrix([[1, 2], [3, 4]])

sympy computes a png and latex string and sends them in a display message. The QtConsole receives this and displays the png computed by sympy. If sympy encountered an error during that computation, the traceback would be normal kernel output, and rightly displayed in the qtconsole.

asmeurer · 2016-01-13T16:24:13Z

OK, my bad. That means that this is an issue in SymPy, and it turns out it has already been fixed (I was testing against the release, but I checked master and it went away).

takluyver · 2016-01-13T17:11:25Z

qtconsole/rich_jupyter_widget.py

-                self._append_latex(data['text/latex'], True)
+                try:
+                    self._append_latex(data['text/latex'], True)
+                    self.log.error("Failed to render latex: '%s'", data['text/latex'], exc_info=True)


This log call should be in the except block, not the try block, right?

Fallback to the string printer if the latex printer raises an exception closes #57

asmeurer mentioned this pull request Oct 23, 2015

LaTeX that matplotlib fails on should fallback to the next renderer #63

Closed

asmeurer mentioned this pull request Oct 23, 2015

Revert "Set IPython printing defaults" sympy/sympy#10005

Merged

minrk added this to the 4.2 milestone Oct 25, 2015

takluyver reviewed Jan 13, 2016
View reviewed changes

Don't log the exception

b03f612

exc_info=True already does this.

takluyver reviewed Jan 13, 2016
View reviewed changes

log message should be in except block, not try block

783f9bf

minrk added a commit that referenced this pull request Jan 21, 2016

Merge pull request #65 from asmeurer/matplotlib-latex-fallback

d78756a

Fallback to the string printer if the latex printer raises an exception closes #57

minrk merged commit d78756a into jupyter:master Jan 21, 2016

jorisvandenbossche mentioned this pull request Jan 30, 2016

BUG: _repr_latex is called in jupyter qtconsole pandas-dev/pandas#12182

Closed

asmeurer mentioned this pull request Jan 30, 2016

LaTeX support is broken #56

Closed

This was referenced Feb 14, 2016

Fix Latex support #95

Merged

Latextools: Make latex_to_png_mpl not fail on errors ipython/ipython#9203

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fallback to the string printer if the latex printer raises an exception #65

Fallback to the string printer if the latex printer raises an exception #65

asmeurer commented Oct 23, 2015

asmeurer commented Oct 23, 2015

flying-sheep commented Oct 25, 2015

asmeurer commented Oct 26, 2015

ccordoba12 commented Dec 7, 2015

flying-sheep commented Dec 7, 2015

asmeurer commented Dec 7, 2015

flying-sheep commented Dec 7, 2015

Carreau commented Jan 6, 2016

flying-sheep commented Jan 6, 2016

Carreau commented Jan 7, 2016

asmeurer commented Jan 7, 2016

flying-sheep commented Jan 7, 2016

Carreau commented Jan 7, 2016

minrk commented Jan 7, 2016

ccordoba12 commented Jan 7, 2016

asmeurer commented Jan 7, 2016

asmeurer commented Jan 7, 2016

minrk commented Jan 7, 2016

flying-sheep commented Jan 7, 2016

ccordoba12 commented Jan 7, 2016

flying-sheep commented Jan 8, 2016

takluyver Jan 13, 2016

asmeurer Jan 13, 2016

asmeurer commented Jan 13, 2016

minrk commented Jan 13, 2016

asmeurer commented Jan 13, 2016

minrk commented Jan 13, 2016

asmeurer commented Jan 13, 2016

takluyver Jan 13, 2016

asmeurer Jan 13, 2016

Fallback to the string printer if the latex printer raises an exception #65

Fallback to the string printer if the latex printer raises an exception #65

Conversation

asmeurer commented Oct 23, 2015

asmeurer commented Oct 23, 2015

flying-sheep commented Oct 25, 2015

asmeurer commented Oct 26, 2015

ccordoba12 commented Dec 7, 2015

flying-sheep commented Dec 7, 2015

asmeurer commented Dec 7, 2015

flying-sheep commented Dec 7, 2015

Carreau commented Jan 6, 2016

flying-sheep commented Jan 6, 2016

Carreau commented Jan 7, 2016

asmeurer commented Jan 7, 2016

flying-sheep commented Jan 7, 2016

Carreau commented Jan 7, 2016

minrk commented Jan 7, 2016

ccordoba12 commented Jan 7, 2016

asmeurer commented Jan 7, 2016

asmeurer commented Jan 7, 2016

minrk commented Jan 7, 2016

flying-sheep commented Jan 7, 2016

ccordoba12 commented Jan 7, 2016

flying-sheep commented Jan 8, 2016

takluyver Jan 13, 2016

Choose a reason for hiding this comment

asmeurer Jan 13, 2016

Choose a reason for hiding this comment

asmeurer commented Jan 13, 2016

minrk commented Jan 13, 2016

asmeurer commented Jan 13, 2016

minrk commented Jan 13, 2016

asmeurer commented Jan 13, 2016

takluyver Jan 13, 2016

Choose a reason for hiding this comment

asmeurer Jan 13, 2016

Choose a reason for hiding this comment