Ticket #11775 (closed enhancement: fixed)

Opened 21 months ago

Last modified 11 months ago

Make pretty_print take multiple arguments

Reported by: ppurka Owned by: jason
Priority: major Milestone: sage-5.1
Component: misc Keywords: pretty print sd40.5
Cc: Work issues:
Report Upstream: N/A Reviewers: Keshav Kini, William Stein
Authors: Punarbasu Purkayastha Merged in: sage-5.1.beta5
Dependencies: Stopgaps:

Description (last modified by jdemeyer) (diff)

The pretty_print function currently takes only one argument. This somehow makes it difficult to print multiple things in one line, for instance the following is currently not possible: 

A = identity_matrix(2); pretty_print("A = ", A)

with the output being "A = <the matrix A on the same line>"

I am attaching a patch which allows pretty_print to take in multiple arguments. It is based on some code by _leif @ #sagemath (See  http://sagenb.org/home/pub/3111/ ). Also kini @ #sagemath helped in passing the doctests.

The modifications are to 1) the function pretty_print 2) the doctests in pretty_print 3) the doctest in pretty_print_default (whitespace issues)

Apply only trac_11775-pretty_print_multiple_args.5.patch Download to SAGE_ROOT/devel/sage.

Attachments

pretty_print_multiple_args.diff Download (2.7 KB) - added by ppurka 21 months ago.
The patch to pretty_print and pretty_print_default
pretty_print-doctest.txt Download (6.7 KB) - added by ppurka 21 months ago.
Doctest of devel/sage/sage/misc
trac_11775_pretty_print_multiple_args.diff Download (2.8 KB) - added by ppurka 21 months ago.
The patch for ticket #11775
11775-pretty_print_multiple_args.patch Download (2.8 KB) - added by kini 21 months ago.
apply to $SAGE_ROOT/devel/sage
trac_11775-pretty_print_multiple_args.patch Download (2.5 KB) - added by kini 20 months ago.
apply to $SAGE_ROOT/devel/sage
trac_11775-pretty_print_multiple_args.2.patch Download (4.1 KB) - added by kini 20 months ago.
apply to $SAGE_ROOT
trac_11775-pretty_print_multiple_args.3.patch Download (8.7 KB) - added by ppurka 17 months ago.
Latest patch based on sage-4.7.2 (Updated to insert space)
trac_11775-pretty_print_multiple_args_mathjax.patch Download (8.8 KB) - added by ppurka 15 months ago.
Apply only to mathjax-ified sage{nb}
trac_11775-pretty_print_multiple_args.4.patch Download (9.9 KB) - added by ppurka 12 months ago.
Rebased to #12156 (fixed documentation)
trac_11775-pretty_print_multiple_args.5.patch Download (9.9 KB) - added by jdemeyer 11 months ago.
Rebased to sage-5.1.beta4

Change History

Changed 21 months ago by ppurka

The patch to pretty_print and pretty_print_default

Changed 21 months ago by ppurka

Doctest of devel/sage/sage/misc

Changed 21 months ago by ppurka

The patch for ticket #11775

comment:1 Changed 21 months ago by ppurka

I was told to mention the ticket number in my patch. Please consider the patch "trac_11775_pretty_print_multiple_args.diff" instead of the originally uploaded one.

Changed 21 months ago by kini

apply to $SAGE_ROOT/devel/sage

comment:2 Changed 21 months ago by kini

  • Description modified (diff)
  • Authors set to Punarbasu Purkayastha

In particular you need to mention the ticket number in your commit message. I fixed your patch. Doctesting on sage.math now.

comment:3 Changed 20 months ago by ppurka

  • Status changed from new to needs_review

comment:4 Changed 20 months ago by kini

  • Reviewers set to Keshav Kini
  • Summary changed from [with patch] Make pretty_print take multiple arguments to Make pretty_print take multiple arguments

I rebased the patch on 4.7.2.alpha3 (because of #11498 mainly). I got rid of the last chunk since I'm not sure that it's still relevant. Please sanity check! Here's the patch diff:

  • 11775-pretty_print_multiple_args.patch 

    diff --git a/11775-pretty_print_multiple_args.patch b/.hg/patches/11775-pretty_print_multiple_args.patch
index 8d4bdbf..0a9516d 100644
    old new  
    11# HG changeset patch 
    22# User P Purkayastha <ppurka@gmail.com> 
    3 # Date 1315137052 -28800 
     3# Date 1317393015 25200 
    44# Node ID a5526ce127658e2865704fd6c30f8cde603c0a45 
    5 # Parent 2a2abbcad325ccca9399981ceddf5897eb467e64 
     5# Parent 8a0b4f90f1ca76dbdba159897c39209c5da85442 
    66trac #11775: make pretty_print accept multiple arguments 
    77 
    88diff --git a/sage/misc/latex.py b/sage/misc/latex.py 
    99--- a/sage/misc/latex.py 
    1010+++ b/sage/misc/latex.py 
    11 @@ -2077,7 +2077,7 @@ 
     11@@ -2120,7 +2120,7 @@ 
    1212     else: 
    1313         print(object) 
    1414  
    … … diff --git a/sage/misc/latex.py b/sage/misc/latex.py 
    1717     r""" 
    1818     Try to pretty print an object in an intelligent way.  For graphics 
    1919     objects, this returns their default representation.  For other 
    20 @@ -2096,24 +2096,39 @@ 
     20@@ -2139,24 +2139,39 @@ 
    2121  
    2222         sage: from sage.misc.latex import pretty_print 
    2323         sage: pretty_print(ZZ)  # indirect doctest 
    2424-        <html><span class="math">\newcommand{\Bold}[1]{\mathbf{#1}}\Bold{Z}</span></html> 
    25 +        <html><span class="math">\newcommand{\Bold}[1]{\mathbf{#1}}\hbox{$\Bold{Z}$}</span></html> 
     25+        <html><span class="math">\newcommand{\Bold}[1]{\mathbf{#1}}\verb|$\Bold{Z}$|</span></html> 
    2626+        sage: pretty_print("Integers = ", ZZ, ", $\\frac{d(x^3)}{dx} =$ ", derivative(x^3, x)) # test m 
    27 +        <html><span class="math">\newcommand{\Bold}[1]{\mathbf{#1}}\hbox{Integers = $\Bold{Z}$, $\frac{ 
     27+        <html><span class="math">\newcommand{\Bold}[1]{\mathbf{#1}}\verb|Integers|\phantom{x}\verb|=|\p 
    2828     """ 
    2929-    if object is None: 
    3030-        return 
    … … diff --git a/sage/misc/latex.py b/sage/misc/latex.py 
    7373  
    7474 def pretty_print_default(enable=True): 
    7575     r""" 
    76 @@ -2130,7 +2145,7 @@ 
    77   
    78          sage: pretty_print_default(True) 
    79          sage: sys.displayhook 
    80 -        <html><span class="math">...\hbox{ < function pretty_print at ... > }</span></html> 
    81 +        <html><span class="math">...\hbox{<function pretty_print at ...>}</span></html> 
    82          sage: pretty_print_default(False) 
    83          sage: sys.displayhook == sys.__displayhook__ 
    84          True 

Doctesting again now.

Changed 20 months ago by kini

apply to $SAGE_ROOT/devel/sage

comment:5 Changed 20 months ago by kini

  • Description modified (diff)

comment:6 follow-up: ↓ 14 Changed 20 months ago by kini

Breaks a couple doctested examples in the documentation (tutorial). Testing new patch...

Changed 20 months ago by kini

apply to $SAGE_ROOT

comment:7 Changed 20 months ago by kini

  • Description modified (diff)

comment:8 Changed 20 months ago by kini

  • Status changed from needs_review to positive_review

All tests pass! Positive review from me.

comment:9 Changed 20 months ago by jdemeyer

  • Keywords pretty print added; pretty_print removed
  • Description modified (diff)

comment:10 Changed 20 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.7.2.alpha4

comment:11 Changed 19 months ago by jdemeyer

  • Status changed from closed to new
  • Resolution fixed deleted
  • Merged in sage-4.7.2.alpha4 deleted
  • Milestone changed from sage-4.7.2 to sage-4.7.3

As clearly evidenced in the changed doctests, this breaks the notebook "typeset" option.

comment:12 Changed 19 months ago by jdemeyer

  • Status changed from new to needs_review

comment:13 Changed 19 months ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues set to Fix notebook "typeset"

comment:14 in reply to: ↑ 6 ; follow-up: ↓ 18 Changed 19 months ago by jdemeyer

Replying to kini:

Breaks a couple doctested examples in the documentation (tutorial).

For good reason! These doctest failures indicate problems with the patch, not with the doctest so they should not be broken.

comment:15 Changed 19 months ago by ppurka

Investigating...

This \verb| thing in the output seems new to me and it seems it's something I overlooked in the later patches. In the original patch Download, the output didn't contain \verb|. Maybe this is why the pretty_print is breaking this stuff.

comment:16 Changed 19 months ago by jhpalmieri

I would also suggest that if the function is going to accept more than one argument, then the documentation shouldn't say 

Try to pretty print an object in an intelligent way.

but rather something like 

Try to pretty print the arguments in an intelligent way.

The INPUT block should be changed, also.

comment:17 follow-up: ↓ 19 Changed 19 months ago by ppurka

Actually, #11498 fixed a *lot* of latex related output but with it something like the following no longer works: view('$f(x) = x^2$').

It seems to me that the fix for this, if any, will be a lot more invasive and won't be confined to the pretty_print function any more.

comment:18 in reply to: ↑ 14 ; follow-up: ↓ 20 Changed 19 months ago by kini

Are you suggesting to reopen #11498?

comment:19 in reply to: ↑ 17 Changed 19 months ago by jhpalmieri

Replying to ppurka:

Actually, #11498 fixed a *lot* of latex related output but with it something like the following no longer works: view('$f(x) = x^2$').

No, I think technically it does work now, and it didn't work before #11498: running view on a string should just render it verbatim. Of course, the broken behavior was actually pretty nice. I think it would be nice to be able to LaTeX a string easily, but not just using the view function. There might even be a ticket somewhere for this, but I couldn't find it with a quick search.

comment:20 in reply to: ↑ 18 ; follow-up: ↓ 21 Changed 19 months ago by ppurka

Replying to kini:

Are you suggesting to reopen #11498?

No. Please don't reopen #11498. It is a massive cleanup of the code.

@jhpalmieri there is a way. You can simply do html('$f(x) = x^2$'). But it doesn't retain the blue color and it won't work with the viewer=pdf setting. So, modifying view() and thereabouts (maybe list_function or tuple_function too) is probably the only way to implement it now, while still retaining these earlier functionality.

comment:21 in reply to: ↑ 20 Changed 19 months ago by jhpalmieri

Replying to ppurka:

@jhpalmieri there is a way. You can simply do html('$f(x) = x^2$').

You can also put it in an %html block or in a %latex block. These solutions only work in the notebook, though. Anyway, it shouldn't be too hard to write a function which takes a string, passes it through jsMath or LaTeX depending on the context and on other arguments (like viewer=pdf), and produces nice looking output, both from the command-line and the notebook. I think that this should be done by a function other than view, though: view should render a string verbatim, so it can handle names of Python classes, for example. Maybe the right approach is to create such a function and to rewrite view: the new function would render a string in LaTeX, and view would call the new function after preparing the string appropriately.

comment:22 Changed 19 months ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:23 Changed 18 months ago by ppurka

  • Status changed from needs_work to needs_review
  • Work issues Fix notebook "typeset" deleted
  • Description modified (diff)
  • Milestone set to sage-4.8

Updated patch to make it work with Typeset on or off, and also take in multiple arguments. Passes doctests: ../../sage -t -long ./sage/misc.

The view(), tuple_function(), latex() and JSMath() in devel/sage/sage/misc/latex.py have been updated to take in an extra argument combine_all. The way it works is:

  1. if combine_all is True then the string returned by tuple_function does not return a tuple and it instead combines all the elements it receives. It does not collapse a tuple inside a tuple.
  1. if combine_all is False (the default) then everything behaves as earlier.
  1. combine_all is passed as True from pretty_print whenever it calls the view() function.

comment:24 follow-up: ↓ 25 Changed 17 months ago by jhpalmieri

You haven't done anything with my suggestions  above.

comment:25 in reply to: ↑ 24 Changed 17 months ago by ppurka

Replying to jhpalmieri:

You haven't done anything with my suggestions  above.

Sorry! I must have overlooked this post and the documentation of pretty_print itself. I have fixed it and also fixed the call to JSMath from inside pretty_print.

comment:26 Changed 17 months ago by ppurka

I am wondering. Is there anywhere where sage.misc.latex.png() function is used? In all my testing of the notebook I believe it never enters or calls the png() function.

If there are some use cases where the png() function is important, then I believe I should also patch that. Currently, I believe the usage of png() function will still go through and return a tuple, just like we get a tuple back when typeset is on in the notebook and we give multiple commands on the same line like: identity_matrix(2), identity_matrix(2)

comment:27 Changed 17 months ago by jhpalmieri

Proof of concept of typesetting function (to typeset a string using LaTeX): 

# something like this could be in latex.py...
def typeset(s, debug=False, viewer=None, **kwds):
    r"""nodetex
    Typeset a string using LaTeX.
    """
    if EMBEDDED_MODE and viewer is None:
        Latex(density=130, debug=debug, **kwds).eval(s, globals=globals())
        return
    png_file = tmp_filename()
    Latex().eval(s, globals=globals(), filename=png_file, debug=debug, **kwds)
    import sage.misc.viewer
    viewer = sage.misc.viewer.png_viewer()
    if viewer.startswith('sage-native-execute '):
        viewer = viewer.split()[1]
    if debug:
        print 'viewer: "{0}"'.format(viewer)
    subprocess.call(['sage-native-execute', viewer, png_file+'.png'],
                    stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    return

comment:28 Changed 17 months ago by ppurka

  • Status changed from needs_review to needs_work
  • Work issues set to Fix typeset of latex strings

comment:29 Changed 17 months ago by jdemeyer

See also #12156.

comment:30 Changed 17 months ago by jhpalmieri

I think we should defer the typesetting of latex strings to #12156. So perhaps this can be set to "needs review" again?

comment:31 Changed 17 months ago by ppurka

  • Status changed from needs_work to needs_review
  • Description modified (diff)
  • Work issues Fix typeset of latex strings deleted

Thanks for opening #12156. This takes care of the issue of printing latex strings. I am uploaded a version 4 of the patch which is rebased to #12156. You may decide to merge either version 3 or version 4 depending on which ticket is applied first.

comment:32 Changed 17 months ago by ppurka

  • Description modified (diff)

Changed 17 months ago by ppurka

Latest patch based on sage-4.7.2 (Updated to insert space)

comment:33 Changed 17 months ago by ppurka

Updated both patches to include space between tuple elements. This addresses the same issue that cropped up in #12156.

comment:34 Changed 17 months ago by ppurka

  • Description modified (diff)

Updated documentation to be more consistent with the existing format.

Changed 15 months ago by ppurka

Apply only to mathjax-ified sage{nb}

comment:35 Changed 15 months ago by ppurka

  • Description modified (diff)

Updated patch to mathjax based sagenb. Apply new patch Download only after applying patches from #9774.

Otherwise the old patch should apply just fine.

It passes doctests in sage/misc except for the  two (unrelated) failures stemming from the mathjax patch in #9774 and also these (unrelated) ones: 

sage -t  "devel/sage-main-backup/sage/misc/latex_macros.py" 
**********************************************************************
File "/home/punarbasu/Installations/sage-5.0.beta2/devel/sage-main-backup/sage/misc/latex_macros.py", line 121:
    sage: convert_latex_macro_to_mathjax('\\newcommand{\\ZZ}{\\Bold{Z}}')
Expected:
    'ZZ: "\\Bold{Z}"'
Got:
    'ZZ: "\\\\Bold{Z}"'
**********************************************************************
File "/home/punarbasu/Installations/sage-5.0.beta2/devel/sage-main-backup/sage/misc/latex_macros.py", line 123:
    sage: convert_latex_macro_to_mathjax('\\newcommand{\\GF}[1]{\\Bold{F}_{#1}}')
Expected:
    'GF: ["\\Bold{F}_{#1}",1]'
Got:
    'GF: ["\\\\Bold{F}_{#1}",1]'
**********************************************************************
1 items had failures:
   2 of   6 in __main__.example_2
***Test Failed*** 2 failures.
For whitespace errors, see the file /home/punarbasu/.sage//tmp/latex_macros_8554.py


sage -t  "devel/sage-main-backup/sage/misc/hg.py"           
changeset:   16520:7c0237f6509d
tag:         qtip
tag:         tip
tag:         trac_11775-pretty_print_multiple_args_mathjax.patch
user:        Punarbasu Purkayastha <ppurka@gmail.com>
date:        Sun Feb 26 00:58:06 2012 +0800
summary:     make pretty_print accept multiple arguments

trac_11078.patch
trac-11503-jmol-commandline.patch
trac-12229-sagenb-developer-doc.2.patch
trac-12229-manifest.patch
trac-11080-notebook-docs.patch
trac_9774-mathjax-try4.patch
trac_11775-pretty_print_multiple_args_mathjax.patch
trac_11078.patch
trac-11503-jmol-commandline.patch
trac-12229-sagenb-developer-doc.2.patch
trac-12229-manifest.patch
trac-11080-notebook-docs.patch
trac_9774-mathjax-try4.patch
trac_11775-pretty_print_multiple_args_mathjax.patch
**********************************************************************
File "/home/punarbasu/Installations/sage-5.0.beta2/devel/sage-main-backup/sage/misc/hg.py", line 262:
    sage: 'main' in hg_sage.list_branches(print_flag=False)
Expected:
    True
Got:
    False
**********************************************************************
1 items had failures:
   1 of   5 in __main__.example_8
***Test Failed*** 1 failures.
For whitespace errors, see the file /home/punarbasu/.sage//tmp/hg_9014.py
	 [3.5 s]

comment:36 Changed 12 months ago by ppurka

  • Keywords sd40.5 added
  • Description modified (diff)

Updated to sage-5.1beta0. For sd40.5 :)

comment:37 Changed 12 months ago by was

  • Status changed from needs_review to positive_review
  • Reviewers changed from Keshav Kini to Keshav Kini, William Stein

Generally looks good. More Pythonic would be: s += [object] be s.append(object), so maybe you want to make that change. I'm giving this a positive review anyways.

comment:38 Changed 12 months ago by ppurka

Thanks. Only change is the s.append(object) in this new file.

Changed 12 months ago by ppurka

Rebased to #12156 (fixed documentation)

comment:39 Changed 12 months ago by ppurka

patchbot: apply trac_11775-pretty_print_multiple_args.4.patch

Changed 11 months ago by jdemeyer

Rebased to sage-5.1.beta4

comment:40 Changed 11 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Description modified (diff)
  • Merged in set to sage-5.1.beta5
Note: See TracTickets for help on using tickets.