#31536 closed enhancement (fixed)

Fix Sage rich output problems by coupling MathJax with html

Reported by: Kwankyu Lee Owned by:
Priority: blocker Milestone: sage-9.3
Component: user interface Keywords:
Cc: Eric Gourgoulhon Merged in:
Authors: Kwankyu Lee Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 4c29b96 (Commits, GitHub, GitLab) Commit: 4c29b9603fe6f4fcc229f9130c7ea5ccc8f3519b
Dependencies: Stopgaps:

Status badges

Description (last modified by Kwankyu Lee)

This ticket has grown from its initial motive and now solves multiple problems related with Sage rich output. In particular, it

  • fixes the blocker https://trac.sagemath.org/ticket/31513
  • fixes the %matplotlib notebook magic, which is broken in a %display latex context in Sage 9.2
  • performs some code improvement regarding the handling of mathjax

The patch has been quite tested with Jupyter notebooks involving various types of Sage objects, Matplotlib objects and ipywidgets.

The initial motive, which is the root of the solutions, is


Presently mathjax is coupled with latex in Sage. The coupling causes dilemmas because latex is for printing and mathjax is for rendering math in html. We encounter such a dilemma in solving #11362, for example.

This ticket takes mathjax from sage.misc.latex and place it properly in sage.misc.html.

The command view(obj) is used to view the latex representation of an object. This ticket aims to make show(obj) and pretty_print(obj) show the html+mathjax representation of the object. The html+mathjax representation can be made from the latex representation or given by _html_ method of the object.

While we are at it, we also cleaned up old code written for deprecated sage notebook.

Attachments (1)

Screen Shot 2021-03-28 at 7.54.12 AM.png (326.0 KB) - added by Kwankyu Lee 20 months ago.

Download all attachments as: .zip

Change History (70)

comment:1 Changed 21 months ago by Kwankyu Lee

Summary: Move MathJax from latex to htmlDivorce MathJax from latex and marry it with html

comment:2 Changed 21 months ago by Kwankyu Lee

Summary: Divorce MathJax from latex and marry it with htmlDivorce MathJax from latex and let it marry html

comment:3 Changed 21 months ago by Kwankyu Lee

Authors: Kwankyu Lee
Branch: u/klee/31536
Description: modified (diff)

comment:4 Changed 21 months ago by git

Commit: 27c2acbe5ef922ecacb93266225bde2da0c8180d

Branch pushed to git repo; I updated commit sha1. New commits:

27c2acbDivorce mathjax from latex but let it marry html

comment:5 Changed 21 months ago by Kwankyu Lee

Status: newneeds_review

comment:6 Changed 21 months ago by Eric Gourgoulhon

Cc: Eric Gourgoulhon added

comment:7 Changed 21 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

comment:8 Changed 21 months ago by Kwankyu Lee

Description: modified (diff)

comment:9 Changed 21 months ago by Eric Gourgoulhon

As expressed in https://groups.google.com/g/sage-devel/c/Q4kWXdMxC2Q/m/5PT5QRgvAwAJ, I think that the meaning of %display latex is clearer to the end user than %display html. The latter sounds quite strange while you are working on a notebook in a browser, the main purpose of which is to display html code.

At the very least, there should be a long deprecation period, when %display latex is still available. For instance, on https://sagemanifolds.obspm.fr/examples.html and on https://luth.obspm.fr/~luthier/gourgoulhon/bh16/sage.html, we have tons of notebooks that start with %display latex.

Last edited 21 months ago by Eric Gourgoulhon (previous) (diff)

comment:10 Changed 21 months ago by git

Commit: 27c2acbe5ef922ecacb93266225bde2da0c8180d8f2560e950924844bda41e6d1d9c0f42d31eace2

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8f2560eDivorce mathjax from latex but let it marry html

comment:11 Changed 21 months ago by git

Commit: 8f2560e950924844bda41e6d1d9c0f42d31eace2caf2c864c72787cfa1c8c0f6e0407be57c204a7e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

caf2c86Divorce mathjax from latex but let it marry html

comment:12 in reply to:  9 ; Changed 21 months ago by Kwankyu Lee

Replying to egourgoulhon:

As expressed in https://groups.google.com/g/sage-devel/c/Q4kWXdMxC2Q/m/5PT5QRgvAwAJ, I think that the meaning of %display latex is clearer to the end user than %display html. The latter sounds quite strange while you are working on a notebook in a browser, the main purpose of which is to display html code.

Right.

At the very least, there should be a long deprecation period, when %display latex is still available. For instance, on https://sagemanifolds.obspm.fr/examples.html and on https://luth.obspm.fr/~luthier/gourgoulhon/bh16/sage.html, we have tons of notebooks that start with %display latex.

I agree. Though I had decided to follow the deprecation path, now I give up.

I think we can make a compromise. We make the clear distinction between latex representation and html(+mathjax) representation internally or on code level but we keep %display latex for end users.

I will commit a patch soon.

comment:13 Changed 21 months ago by git

Commit: caf2c864c72787cfa1c8c0f6e0407be57c204a7ead6c0446ae53cc10b09cf5dedee373cef6e0b70b

Branch pushed to git repo; I updated commit sha1. New commits:

ad6c044Restore %display latex

comment:14 in reply to:  12 Changed 21 months ago by Eric Gourgoulhon

Replying to klee:

I think we can make a compromise. We make the clear distinction between latex representation and html(+mathjax) representation internally or on code level but we keep %display latex for end users.

Sounds good! Thanks. :-)

comment:15 Changed 21 months ago by Eric Gourgoulhon

Does by any chance the code improvement in this ticket fix #31513 ?

comment:16 Changed 21 months ago by git

Commit: ad6c0446ae53cc10b09cf5dedee373cef6e0b70b1a8089451f898e0747d70b3c4a8951330986d076

Branch pushed to git repo; I updated commit sha1. New commits:

1a80894Latex string than mathjax string

comment:17 Changed 21 months ago by Kwankyu Lee

Description: modified (diff)

comment:18 Changed 20 months ago by Eric Gourgoulhon

I gave a try and it looks OK to me, except that the latex-typeset is broken in the pdf export from the Jupyter notebook. I guess fixing this should wait the resolution of #31513.

comment:19 in reply to:  18 Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

I gave a try and it looks OK to me, except that the latex-typeset is broken in the pdf export from the Jupyter notebook. I guess fixing this should wait the resolution of #31513.

Thanks! I agree.

comment:20 Changed 20 months ago by git

Commit: 1a8089451f898e0747d70b3c4a8951330986d07621ea546d85d8a9b0cb580f24f36afcf4bd51623c

Branch pushed to git repo; I updated commit sha1. New commits:

21ea546A fix for jupyter pdf export

comment:21 Changed 20 months ago by Eric Gourgoulhon

Thanks for the fix. There is still an issue for the pdf export of matrices. For instance,

%display latex
matrix([[1, x], [0, 0]])

produces a typeset matrix in the Jupyter notebook, but not in its pdf export.

comment:22 Changed 20 months ago by Eric Gourgoulhon

Another issue is that %matplotlib notebook is broken by this ticket: the following cell of a Jupyter notebook:

%matplotlib notebook
import matplotlib.pyplot as plt
plt.plot([1, 2, 3, 4])
plt.ylabel('some numbers')
plt.show()

yields

<IPython.core.display.HTML object>

instead of the pyplot window.

comment:23 Changed 20 months ago by git

Commit: 21ea546d85d8a9b0cb580f24f36afcf4bd51623cbfb1a8f864cb0f84aa4bb9707b23522aff263745

Branch pushed to git repo; I updated commit sha1. New commits:

bfb1a8fFix for matrix

comment:24 in reply to:  22 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Another issue is that %matplotlib notebook is broken by this ticket: the following cell of a Jupyter notebook:

%matplotlib notebook
import matplotlib.pyplot as plt
plt.plot([1, 2, 3, 4])
plt.ylabel('some numbers')
plt.show()

yields

<IPython.core.display.HTML object>

instead of the pyplot window.

Are you sure? I copy and pasted the same code into sage 9.1 and 9.2. They all fail.

comment:25 Changed 20 months ago by Kwankyu Lee

The same code also fails on sage 9.3.beta9 without this ticket.

comment:26 in reply to:  25 Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

The same code also fails on sage 9.3.beta9 without this ticket.

It works for me, both with sage 9.2 and 9.3.rc0 (note that it does not work if %display latex is set first).

comment:27 in reply to:  24 ; Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

Replying to egourgoulhon:

Another issue is that %matplotlib notebook is broken by this ticket: the following cell of a Jupyter notebook:

%matplotlib notebook
import matplotlib.pyplot as plt
plt.plot([1, 2, 3, 4])
plt.ylabel('some numbers')
plt.show()

yields

<IPython.core.display.HTML object>

instead of the pyplot window.

Are you sure? I copy and pasted the same code into sage 9.1 and 9.2. They all fail.

Yes I am sure: it works for me for sage 9.1, 9.2 and 9.3.rc0. It is very convenient to have that pyplot window, because it allows to zoom and pan, contrary to Sage 2d plot outputs. Note that, as said in comment:26, it is broken by %display latex though. But

%display plain
%matplotlib notebook
import matplotlib.pyplot as plt
plt.plot([1, 2, 3, 4])
plt.ylabel('some numbers')
plt.show()

works all the time.

Changed 20 months ago by Kwankyu Lee

comment:28 in reply to:  27 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

I am sure: it works for me for sage 9.1, 9.2 and 9.3.rc0. It is very convenient to have that pyplot window, because it allows to zoom and pan, contrary to Sage 2d plot outputs. Note that, as said in comment:26, it is broken by %display latex though. But

%display plain
%matplotlib notebook
import matplotlib.pyplot as plt
plt.plot([1, 2, 3, 4])
plt.ylabel('some numbers')
plt.show()

works all the time.

I cannot see what you see. Does it work in cocalc for you? I doen't for me. See the attachment.

comment:29 Changed 20 months ago by Kwankyu Lee

Anyway, I can observe that with %display latex, the failure is of different kind.

comment:30 Changed 20 months ago by git

Commit: bfb1a8f864cb0f84aa4bb9707b23522aff2637455f588be835e636581b933d9ba323c31e9dab4e60

Branch pushed to git repo; I updated commit sha1. New commits:

5f588beFix matplotlib output

comment:31 Changed 20 months ago by Kwankyu Lee

Please check

comment:32 Changed 20 months ago by git

Commit: 5f588be835e636581b933d9ba323c31e9dab4e6009beec0ecc167406713ee559225d0091b0025f07

Branch pushed to git repo; I updated commit sha1. New commits:

6cf27bfMerge branch 'develop' into mathjax-trac31536
09beec0Minor fixes for doctest failures

comment:33 Changed 20 months ago by Kwankyu Lee

Milestone: sage-9.4sage-9.3

comment:34 in reply to:  31 ; Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

Please check

Thanks for the new commits. I confirm that the issue with pdf export of matrices is fixed by them. However, the issue with matplotlib remains: for the code of comment:27, I still get

<IPython.core.display.HTML object>

instead of the pyplot window.

comment:35 in reply to:  28 ; Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

I cannot see what you see. Does it work in cocalc for you? I doen't for me. See the attachment.

No, it does not work in CoCalc for me either (same error message as you). Given that CoCalc's Jupyter notebook is quite customized, I would say that it is a CoCalc issue, rather than a Sage one. Can't you see the pyplot window with Sage 9.3.rc0 on your own machine? (mine is Ubuntu 20.04, with Firefox 86.0.1).

comment:36 in reply to:  34 ; Changed 20 months ago by Eric Gourgoulhon

Replying to egourgoulhon:

Replying to klee:

Please check

Thanks for the new commits. I confirm that the issue with pdf export of matrices is fixed by them. However, the issue with matplotlib remains: for the code of comment:27, I still get

<IPython.core.display.HTML object>

instead of the pyplot window.

FWIW, for plt.show(), the following happens in SageDisplayFormatter.format:

        # use IPython display for IPython native objects
        if isinstance(obj, IPYTHON_NATIVE_TYPES):     <== this test is passed
            if self.ipython_display_formatter(obj):   <== this test fails
                return {}, {}                         

Hence the return {}, {} is not performed.

comment:37 in reply to:  35 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Replying to klee:

I cannot see what you see. Does it work in cocalc for you? I doen't for me. See the attachment.

No, it does not work in CoCalc for me either (same error message as you). Given that CoCalc's Jupyter notebook is quite customized, I would say that it is a CoCalc issue, rather than a Sage one. Can't you see the pyplot window with Sage 9.3.rc0 on your own machine? (mine is Ubuntu 20.04, with Firefox 86.0.1).

No. My system is MacOS 11.2.3 with Chrome 89.0.4389.90.

I tried to install jupyter-matplotlib extension with jupyterlab 2 and jupyterlab 3. All did not work.

By the way, it works with Python 3 kernel in jupyterlab.

Last edited 20 months ago by Kwankyu Lee (previous) (diff)

comment:38 in reply to:  36 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Replying to egourgoulhon:

Replying to klee:

Please check

Thanks for the new commits. I confirm that the issue with pdf export of matrices is fixed by them. However, the issue with matplotlib remains: for the code of comment:27, I still get

<IPython.core.display.HTML object>

instead of the pyplot window.

FWIW, for plt.show(), the following happens in SageDisplayFormatter.format:

        # use IPython display for IPython native objects
        if isinstance(obj, IPYTHON_NATIVE_TYPES):     <== this test is passed
            if self.ipython_display_formatter(obj):   <== this test fails
                return {}, {}                         

Hence the return {}, {} is not performed.

self.ipython_display_formatter(obj) is an ipython native method. which has nothing to do with sage %display magic. I don't understand why it fails with %display latex but it works with %display plain.

comment:39 in reply to:  38 ; Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

Replying to egourgoulhon:

Replying to egourgoulhon:

Replying to klee:

Please check

Thanks for the new commits. I confirm that the issue with pdf export of matrices is fixed by them. However, the issue with matplotlib remains: for the code of comment:27, I still get

<IPython.core.display.HTML object>

instead of the pyplot window.

FWIW, for plt.show(), the following happens in SageDisplayFormatter.format:

        # use IPython display for IPython native objects
        if isinstance(obj, IPYTHON_NATIVE_TYPES):     <== this test is passed
            if self.ipython_display_formatter(obj):   <== this test fails
                return {}, {}                         

Hence the return {}, {} is not performed.

self.ipython_display_formatter(obj) is an ipython native method. which has nothing to do with sage %display magic. I don't understand why it fails with %display latex but it works with %display plain.

No, it fails with %display plain as well. All subsequent tests in SageDisplayFormatter.format also fail, so at the end

        return ipy_format, ipy_metadata

is achieved. The same thing happens with Sage 9.3.rc0 (i.e. outside this ticket branch), but for some reason, the output ipy_format, ipy_metadata is better handled in that case, so that the pyplot window opens.

comment:40 in reply to:  39 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Replying to klee: is achieved. The same thing happens with Sage 9.3.rc0 (i.e. outside this ticket branch), but for some reason, the output ipy_format, ipy_metadata is better handled in that case, so that the pyplot window opens.

Which do you mean, with the patch, or without the patch?

comment:41 in reply to:  40 ; Changed 20 months ago by Kwankyu Lee

Replying to klee:

Replying to egourgoulhon:

Replying to klee: is achieved. The same thing happens with Sage 9.3.rc0 (i.e. outside this ticket branch), but for some reason, the output ipy_format, ipy_metadata is better handled in that case, so that the pyplot window opens.

Which do you mean, with the patch, or without the patch?

Ah, you meant rc0, without the patch..

comment:42 in reply to:  41 Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

Replying to klee:

Replying to egourgoulhon:

Replying to klee: is achieved. The same thing happens with Sage 9.3.rc0 (i.e. outside this ticket branch), but for some reason, the output ipy_format, ipy_metadata is better handled in that case, so that the pyplot window opens.

Which do you mean, with the patch, or without the patch?

Ah, you meant rc0, without the patch..

Yes (sorry for not having been clear).

comment:43 Changed 20 months ago by Eric Gourgoulhon

To summarize: for plt.show() in Sage 9.3.rc0, with or without the patch introduced in this ticket, SageDisplayFormatter.format terminates by return ipy_format, ipy_metadata, whatever the value of %display. Then the pyplot window opens correctly only without the patch and without %display latex.

comment:44 in reply to:  37 ; Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

Replying to egourgoulhon:

Replying to klee:

I cannot see what you see. Does it work in cocalc for you? I doen't for me. See the attachment.

No, it does not work in CoCalc for me either (same error message as you). Given that CoCalc's Jupyter notebook is quite customized, I would say that it is a CoCalc issue, rather than a Sage one. Can't you see the pyplot window with Sage 9.3.rc0 on your own machine? (mine is Ubuntu 20.04, with Firefox 86.0.1).

No. My system is MacOS 11.2.3 with Chrome 89.0.4389.90.

Strange... On my side, I haven't any special setting regarding matplotlib (it's the default version included in Sage).

Regarding CoCalc, I've just got the answer from William: it works with Sage 9.2 kernel but you have to start a "Jupyter classic server" (in the "New" menu).

comment:45 in reply to:  44 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Replying to klee:

Replying to egourgoulhon:

Replying to klee:

I cannot see what you see. Does it work in cocalc for you? I doen't for me. See the attachment.

No, it does not work in CoCalc for me either (same error message as you). Given that CoCalc's Jupyter notebook is quite customized, I would say that it is a CoCalc issue, rather than a Sage one. Can't you see the pyplot window with Sage 9.3.rc0 on your own machine? (mine is Ubuntu 20.04, with Firefox 86.0.1).

No. My system is MacOS 11.2.3 with Chrome 89.0.4389.90.

Strange... On my side, I haven't any special setting regarding matplotlib (it's the default version included in Sage).

Does it work without installing jupyter-matplotlib extension?

I created a new Ubuntu vm, and installed a fresh sage from source, and installed jupyter-matplotlib from the jupyter extension manager. Even then it doesn't work and shows the same message Javascript Error: IPython is not defined. Now even with Python3 kernel.

comment:46 in reply to:  44 Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Replying to klee:

Strange... On my side, I haven't any special setting regarding matplotlib (it's the default version included in Sage).

Regarding CoCalc, I've just got the answer from William: it works with Sage 9.2 kernel but you have to start a "Jupyter classic server" (in the "New" menu).

Ok. I finally made it work with the Jupyter classic server. Let me now go back to the ticket.

comment:47 Changed 20 months ago by git

Commit: 09beec0ecc167406713ee559225d0091b0025f07b1bcda3726576d59b1afe9214a1c1481d0e80ce9

Branch pushed to git repo; I updated commit sha1. New commits:

b1bcda3Remove SageDisplayFormatter.default_mime()

comment:48 Changed 20 months ago by Kwankyu Lee

I made a rather drastic change, removing default_mime() method, which I don't understand clearly. Instead I rely on the ipython class DisplayObject to filter out ipython native objects.

It works well with the examples up to now. But I think it is necessary to test extensively in fear of any bad regressions.

comment:49 Changed 20 months ago by Kwankyu Lee

Description: modified (diff)

comment:50 Changed 20 months ago by Kwankyu Lee

Description: modified (diff)

comment:51 in reply to:  48 Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

I made a rather drastic change, removing default_mime() method, which I don't understand clearly. Instead I rely on the ipython class DisplayObject to filter out ipython native objects.

It works well with the examples up to now. But I think it is necessary to test extensively in fear of any bad regressions.

Thanks for the new code. The pyplot window is now correctly handled. Unfortunately, two new issues have appeared:

1/ Outside the context %matplotlib notebook, there is no graphical display of plt.show(): the output of the Jupyter cell

import matplotlib.pyplot as plt
plt.plot([1, 2, 3, 4])
plt.ylabel('some numbers')
plt.show()

is

<Figure size 432x288 with 1 Axes>

instead of a png figure displayed by the notebook.

2/ The native IPython rich output method _repr_latex_ is broken:

class A(SageObject):

    def __init__(self, data):
        self._data = data

    def _repr_latex_(self):
        try:
            return '$' + self._data._latex_() + '$'
        except (AttributeError, NotImplementedError):  
            return None  # if None is returned, plain text is used

A(sin(x^2))

results in

<__main__.A object at 0x7f0566d4a940>

instead of the latex-typeset formula.

These two issues were not present in the previous version of the patch, nor in plain Sage 9.3.rc0.

comment:52 in reply to:  45 Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

Replying to egourgoulhon:

Replying to klee:

Replying to egourgoulhon:

Can't you see the pyplot window with Sage 9.3.rc0 on your own machine? (mine is Ubuntu 20.04, with Firefox 86.0.1).

No. My system is MacOS 11.2.3 with Chrome 89.0.4389.90.

Strange... On my side, I haven't any special setting regarding matplotlib (it's the default version included in Sage).

Does it work without installing jupyter-matplotlib extension?

Yes, since I don't have installed such an extension: the content of local/share/jupyter/nbextensions under my Sage root is

jsmol  jupyter_jsmol  jupyter-js-widgets  mathjax  threejs

comment:53 Changed 20 months ago by git

Commit: b1bcda3726576d59b1afe9214a1c1481d0e80ce9d964733adbeda0df4fd3477f5f728ad5f2c4cc39

Branch pushed to git repo; I updated commit sha1. New commits:

d964733Use ipython output if sage output is plain

comment:54 Changed 20 months ago by Kwankyu Lee

The commit fixes 1/ and 2/ with me. Check yours.

Regardless of the patch, the matplotlib interactive display is unstable (visible or invisible randomly) on my installation. Strange...

comment:55 in reply to:  54 ; Changed 20 months ago by Eric Gourgoulhon

Reviewers: Eric Gourgoulhon

Replying to klee:

The commit fixes 1/ and 2/ with me. Check yours.

Thanks. I confirm that both 1/ and 2/ are fixed. Moreover, the new commit also fixes %matplotlib notebook in a %display latex context, which did not work with Sage 9.2 and 9.3.rc0 ! I've also checked that the pdf export of a Jupyter notebook works nicely. So I would say that everything is green from my side. Let us wait for the pachbot and other reviewers, if any.

Regardless of the patch, the matplotlib interactive display is unstable (visible or invisible randomly) on my installation. Strange...

Note that there cannot be two interactive pyplot windows in two distinct cells of a given Jupyter notebook; if you open a new window via plt.show() in a new cell, the first one becomes blank (in the same cell, it is OK). This might explain what you observe.

Last edited 20 months ago by Eric Gourgoulhon (previous) (diff)

comment:56 Changed 20 months ago by Eric Gourgoulhon

If everything is OK with this ticket, I propose that it becomes the fix for the blocker #31513.

comment:57 in reply to:  55 Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

Replying to klee:

Regardless of the patch, the matplotlib interactive display is unstable (visible or invisible randomly) on my installation. Strange...

Note that there cannot be two interactive pyplot windows in two distinct cells of a given Jupyter notebook; if you open a new window via plt.show() in a new cell, the first one becomes blank (in the same cell, it is OK). This might explain what you observe.

I see. Thanks.

comment:58 Changed 20 months ago by Eric Gourgoulhon

I think the python3 error reported by the patchbot is a false positive, triggered by the presence of <> in the string, so we can disregard it. Can you comment on the pyflake errors?

comment:59 Changed 20 months ago by git

Commit: d964733adbeda0df4fd3477f5f728ad5f2c4cc394c29b9603fe6f4fcc229f9130c7ea5ccc8f3519b

Branch pushed to git repo; I updated commit sha1. New commits:

4c29b96Fix some pyflakes errors

comment:60 in reply to:  58 ; Changed 20 months ago by Kwankyu Lee

Replying to egourgoulhon:

I think the python3 error reported by the patchbot is a false positive, triggered by the presence of <> in the string, so we can disregard it. Can you comment on the pyflake errors?

I fixed some pyflakes errors, which are safe to fix. Anyway none of them are new.

comment:61 in reply to:  60 Changed 20 months ago by Eric Gourgoulhon

Replying to klee:

I fixed some pyflakes errors, which are safe to fix.

Thanks!

Anyway none of them are new.

Indeed. Thanks for having taken this opportunity to fix them.

comment:62 Changed 20 months ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

It's time to move on, if we want this in Sage 9.3. Thanks for all your work on this ticket!

comment:63 Changed 20 months ago by Kwankyu Lee

Thank you for a careful review. Hope this sail on peacefully.

comment:64 Changed 20 months ago by Matthias Köppe

Priority: majorblocker

comment:65 Changed 20 months ago by Matthias Köppe

It would probably be good to revise the ticket summary/description based on https://groups.google.com/g/sage-release/c/fXpbYwaqt9o/m/xYOwhU26CAAJ

comment:66 in reply to:  48 ; Changed 20 months ago by John Palmieri

Replying to klee:

I made a rather drastic change, removing default_mime() method, which I don't understand clearly. Instead I rely on the ipython class DisplayObject to filter out ipython native objects.

It works well with the examples up to now. But I think it is necessary to test extensively in fear of any bad regressions.

Because of your last paragraph, I think this can't be a blocker for 9.3: it should be one of the first tickets merged in 9.4 instead.

comment:67 in reply to:  66 Changed 20 months ago by Eric Gourgoulhon

Replying to jhpalmieri:

Because of your last paragraph, I think this can't be a blocker for 9.3: it should be one of the first tickets merged in 9.4 instead.

Well, since this paragraph was written, much more tests have been performed. For instance, the patch has been successfully run on this notebook, which mixes latex display, Sage 2d and 3d plots, pure Matplotlib plots via pyplot, PIL images and ipywidgets (progress bars).

comment:68 Changed 20 months ago by Kwankyu Lee

Description: modified (diff)
Summary: Divorce MathJax from latex and let it marry htmlFix Sage rich output problems by coupling MathJax with html

comment:69 Changed 20 months ago by Volker Braun

Branch: u/klee/315364c29b9603fe6f4fcc229f9130c7ea5ccc8f3519b
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.