Deprecate sage.misc.viewer

[jhpalmieri]

As part of #32957, let's deprecate sage.misc.viewer in favor of (a) for images, a version that uses Sage's Features and (b) for web browsing, the Python library webbrowser.

CC: @seblabbe

Component: misc

Author: John Palmieri

Branch/Commit: u/jhpalmieri/deprecate-viewer @ cb099ed

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/33931

[jhpalmieri]

Branch: u/jhpalmieri/deprecate-viewer

[jhpalmieri]

Commit: bbe8110

[jhpalmieri]

comment:2

I tested this by running these commands in Sage from the command line and in the notebook:

C = graphs.CubeGraph(8)
C.plot(vertex_labels=False, vertex_size=0, graph_border=True).show()

view(4)
view(4, viewer='pdf')

from sage.misc.latex_standalone import Standalone
s = Standalone('hello world')
s.pdf()
s.png()
s.svg()  # fails on my machine: missing feature

browse_sage_doc._open('reference')
browse_sage_doc(identity_matrix)

I did this with the develop branch, with this branch, and with a more radical branch in which I completely deleted sage.misc.viewer and its entry in the reference manual. It looked like the same behavior in all of these cases. This was all on OS X, I encourage more testing on more platforms.

---

New commits:

bbe8110

trac 33931: deprecate sage.misc.viewer

[jhpalmieri]

comment:3

This is not working on an Ubuntu virtual machine. It tries to open every png and pdf in a web browser and produces a message like File not found. Firefox can't find the file at //tmp/..... Not sure why there are two slashes, but the file is there with the right name and the right contents.

[sagetrac-git]

Changed commit from bbe8110 to 426e39d

[sagetrac-git]

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

046849e	trac 33931: deprecate sage.misc.viewer
426e39d	trad 33931: try PIL first, then a browser

[jhpalmieri]

comment:5

With these changes (trying PIL first, then a browser as a backup), some images work for me on Ubuntu, but PDF files still fail with the same error message. Suggestions?

[seblabbe]

comment:7

Replying to @jhpalmieri:

I tested this by running these commands in Sage from the command line and in the notebook:

C = graphs.CubeGraph(8)
C.plot(vertex_labels=False, vertex_size=0, graph_border=True).show()

view(4)
view(4, viewer='pdf')

from sage.misc.latex_standalone import Standalone
s = Standalone('hello world')
s.pdf()
s.png()
s.svg()  # fails on my machine: missing feature

browse_sage_doc._open('reference')
browse_sage_doc(identity_matrix)

I did this with the develop branch, with this branch, and with a more radical branch in which I completely deleted sage.misc.viewer and its entry in the reference manual. It looked like the same behavior in all of these cases. This was all on OS X, I encourage more testing on more platforms.

On Ubuntu 20.04, all of the above work. The behavior of s.pdf(), s.png() and s.svg() changed as it now open in the browser at url like file:////tmp/tmp0jvfbjec/tikz_u5t2ot7z.pdf. It is fine with me.

Positive review from my side.

[sagetrac-git]

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

c76c948

Merge branch 'develop' into t/33931/deprecate-viewer

[sagetrac-git]

Changed commit from 426e39d to c76c948

[jhpalmieri]

comment:9

Rebased to 9.7.beta8, ready for review.

[seblabbe]

comment:10

Replying to @jhpalmieri:

With these changes (trying PIL first, then a browser as a backup), some images work for me on Ubuntu, but PDF files still fail with the same error message. Suggestions?

Default webbrowser do not open pdf files. Maybe this is why?

[jhpalmieri]

comment:11

Replying to @seblabbe:

Replying to @jhpalmieri:

With these changes (trying PIL first, then a browser as a backup), some images work for me on Ubuntu, but PDF files still fail with the same error message. Suggestions?

Default webbrowser do not open pdf files. Maybe this is why?

What, are we still living in the 90s?

[jhpalmieri]

comment:12

Maybe we need to keep some of the old functionality for opening pdfs, or reimplement it using Features.

[seblabbe]

comment:13

Replying to @seblabbe:

On Ubuntu 20.04, all of the above work. The behavior of s.pdf(), s.png() and s.svg() changed as it now open in the browser at url like file:////tmp/tmp0jvfbjec/tikz_u5t2ot7z.pdf. It is fine with me.
Positive review from my side.

Looking at this ticket again, I don't think I am confident anymore to give a positive review. Deprecating viewer in favor of webbrowser changes a behavior in sage that exists since 15 years. In particular, on my Ubuntu, viewer is an equivalent for the more general xdg-open:

sage: from sage.misc.viewer import viewer
sage: viewer('pdf')
'xdg-open'

Also, the current state which exists for years allows the user to specify its preferated viewer:

sage: from sage.misc.viewer import viewer, _viewer_prefs
sage: viewer.pdf_viewer('evince')
sage: _viewer_prefs
{'pdf_viewer': 'evince'}

So, the current branch might be a step backward for many long time users which have old written code. Personnaly, I am afraid the current change will break something on a platform I am not familiar with like Windows or OS X, which I can't test.

Finally, is the motivation for the current branch only to get rid of have_program? If so, I would suggest to create a feature for xdg-open and use webbrowser in the lines where firefox, etc. is involved.

[jhpalmieri]

comment:15

(The motivation came from the brief discussion at #32957 comment:14.)

[seblabbe]

comment:16

Replying to @jhpalmieri:

(The motivation came from the brief discussion at #32957 comment:14.)

Ok, I remember now. Maybe we can start by replacing occurences of have_program by some usage of webbrowser.

Also, in the documentation of webbrowser.open, I see:

"Note that on some platforms, trying to open a filename using this function, may work and start the operating system’s associated program. However, this is neither supported nor portable."

However, I don't know if this comment refers to r'file:///path/to/file' or only r'path/to/file'.

[sagetrac-git]

Changed commit from c76c948 to b07ec30

[sagetrac-git]

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

b07ec30

trac 33931: deprecate viewer.py as part of eliminating the use of

[jhpalmieri]

comment:18

Here is a new version. This deprecates viewer.py and introduces image_viewer.py. A few differences between these: image_viewer.py only deals with images: web browsing is done through Python's webbrowser module. Also, image_viewer.py uses Features to detect the presence of xdg-open and similar.

[jhpalmieri]

comment:19

Questions and comments:

• I don't know a good way to test whether open (for example) is functional. While xdg-open and cygstart are pretty specific names, I can imagine someone installing an executable called open on their linux box which doesn't do anything what we expect here. If I actually call open to try to open a file, then if successful, it will start an app and open up a window, and that does not seem ideal. Right now there is no is_functional test for these new features.

• the change in latex.py

diff --git a/src/sage/misc/latex.py b/src/sage/misc/latex.py
index 3ec3a3e..5d9ebdb 100644
--- a/src/sage/misc/latex.py
+++ b/src/sage/misc/latex.py
@@ -1859,7 +1859,7 @@ def view(objects, title='Sage', debug=False, sep='', tiny=False,
         sage: with NamedTemporaryFile(mode="w+t", suffix=".tex") as f:  # optional - latex latex_package_tkz_graph
         ....:     _ = f.write(_latex_file_(g))
         ....:     f.flush()
-        ....:     _run_latex_(file, engine="pdflatex")
+        ....:     _run_latex_(f.name, engine="pdflatex")
         'pdf'

is just fixing a bug: file is not defined, whereas f.name is the name of the temporary file being used in with ... as f:. I guess no one runs doctests using latex as an optional flag?

[jhpalmieri]

comment:20

I don't see a way to edit my comments. The first item in the previous comment perhaps should have ended with a question: suggestions for what to do?

[sagetrac-git]

Changed commit from b07ec30 to a99f59d

[sagetrac-git]

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

84107aa	Merge branch 'develop' into t/33931/deprecate-viewer
a99f59d	trac 33931: merge develop and undo `_run_latex_` fix: that is now in #34594.

[sagetrac-git]

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

cb78cbc

trac 33931: undo `_run_latex_` fix: that is now in #34594.

[sagetrac-git]

Changed commit from a99f59d to cb78cbc

[seblabbe]

Reviewer: Sébastien Labbé

[seblabbe]

comment:24

The current branch has a conflict with 9.8.beta1.

[sagetrac-git]

Changed commit from cb78cbc to cb099ed

[sagetrac-git]

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

cb099ed

Merge branch 'develop' into t/33931/deprecate-viewer

[jhpalmieri]

comment:26

Merge conflict fixed.