Ticket #4575 (reopened enhancement)

Opened 2 months ago

Last modified 1 month ago

[with patch, needs work] Option to show nested lists as html tables

Reported by: whuss Assigned to: whuss
Priority: major Milestone: sage-3.4
Component: notebook Keywords:
Cc:

Description

The attached patch adds the option table_form to the show() command.

If table_form = True, nested lists are shown in the notebook as nicely formatted html tables.

Attachments

trac_4575.patch (4.4 kB) - added by whuss on 11/21/2008 01:22:45 AM.
trac_4575-taketwo.patch (8.1 kB) - added by mhansen on 11/29/2008 10:39:36 AM.

Change History

11/21/2008 01:22:45 AM changed by whuss

  • attachment trac_4575.patch added.

11/27/2008 10:13:06 PM changed by was

  • summary changed from [with patch, needs review] Option to show nestet lists as html tables to [with patch, positive review] Option to show nestet lists as html tables.

REFEREE REPORT:

Positive review. Code looks solid and the result is really pretty to look at.

1. Line 945 contains a trivial typo: 

935	    Print a nestet list as a html table.

2. The patch isn't a mercurial patch, so... :-(

11/27/2008 10:47:20 PM changed by mabshoff

  • summary changed from [with patch, positive review] Option to show nestet lists as html tables to [with patch, positive review] Option to show nested lists as html tables.

I turned this diff into a patch with credit to Wilfried and fixed the typo. I also fixed the typo in the summary :)

Cheers,

Michael

11/27/2008 10:48:13 PM changed by mabshoff

  • status changed from new to closed.
  • resolution set to fixed.

Merged in Sage 3.2.1.rc0

(follow-up: ↓ 5 ) 11/29/2008 10:11:59 AM changed by mhansen

I don't agree with William's positive review on this for some design reasons. First off, it is only useful from the notebook as it just spits out HTML while all other cases of show work fine from the command-line. Tacking random things and keywords onto show is not a maintainable solution especially since the new functionality is completely orthogonal to what's already there.

Instead this should be some function in a module for creating HTML output of objects.

(in reply to: ↑ 4 ) 11/29/2008 10:15:56 AM changed by mabshoff

Replying to mhansen:

I don't agree with William's positive review on this for some design reasons. First off, it is only useful from the notebook as it just spits out HTML while all other cases of show work fine from the command-line. Tacking random things and keywords onto show is not a maintainable solution especially since the new functionality is completely orthogonal to what's already there. Instead this should be some function in a module for creating HTML output of objects.

Ok, let's take this discussion to sage-devel. I might reverse this patch since Jaap oberved a doctest failure that seems impossible to happen. Another reason to reverse this is that we do not want to ship a show with this option if we are going to break it anyway.

Your idea certainly sounds cleaner.

Cheers,

Michael

11/29/2008 10:39:36 AM changed by mhansen

  • attachment trac_4575-taketwo.patch added.

11/29/2008 10:41:43 AM changed by mhansen

I've attached a patch which implements my idea. I added a comment about html.nested_list, Since the user would have to read the docstring to the previous keyword anyway.

11/29/2008 11:02:38 AM changed by was

I'm ok with revoking my positive review. I like Mike's suggestion better.

11/29/2008 01:14:40 PM changed by mabshoff

Ok, can we then get a review on Mike's patch? Since it applies on to of Wilfried's patch we should attempt to get it into 3.2.1.rc1.

Cheers,

Michael

11/29/2008 09:42:55 PM changed by mabshoff

  • status changed from closed to reopened.
  • resolution deleted.
  • milestone changed from sage-3.2.1 to sage-3.2.2.

Due to the problems with extra mbox{} popping up reported by Jaap, John and also me on some boxen I will revert this patch in 3.2.1.rc1. See also

http://groups.google.com/group/sage-devel/t/3a6575cb49cd61a8

Reopened. Hopefully Mike Hansen's approach will be merged in 3.2.2.

Cheers,

Michael

11/29/2008 09:44:27 PM changed by mabshoff

  • summary changed from [with patch, positive review] Option to show nested lists as html tables to [with patch, needs work] Option to show nested lists as html tables.
sage -t  "devel/sage/sage/misc/functional.py"               
********************************************************************** 
File "/home/jaap/work/downloads/sage-3.2.1.alpha2/devel/sage/sage/misc/functiona l.py", line 891: 
     sage: show([(i, j, i == j) for i in [0..1] for j in [0..1]], table_form = True) 
Expected: 
     <html> 
     <div class="notruncate"> 
     <table class="table_form"> 
     <tbody> 
     <tr class ="row-a"> 
     <td><span class="math">0</span></td> 
     <td><span class="math">0</span></td> 
     <td><span class="math">\mbox{\rm True}</span></td> 
     </tr> 
     <tr class ="row-b"> 
     <td><span class="math">0</span></td> 
     <td><span class="math">1</span></td> 
     <td><span class="math">\mbox{\rm False}</span></td> 
     </tr> 
     <tr class ="row-a"> 
     <td><span class="math">1</span></td> 
     <td><span class="math">0</span></td> 
     <td><span class="math">\mbox{\rm False}</span></td> 
     </tr> 
     <tr class ="row-b"> 
     <td><span class="math">1</span></td> 
     <td><span class="math">1</span></td> 
     <td><span class="math">\mbox{\rm True}</span></td> 
     </tr> 
     </tbody> 
     </table> 
     </div> 
     </html> 
Got: 
     <html> 
     <div class="notruncate"> 
     <table class="table_form"> 
     <tbody> 
     <tr class ="row-a"> 
     <td><span class="math">0</span></td> 
     <td><span class="math">0</span></td> 
     <td><span class="math">True</span></td> 
     </tr> 
     <tr class ="row-b"> 
     <td><span class="math">0</span></td> 
     <td><span class="math">1</span></td> 
     <td><span class="math">False</span></td> 
     </tr> 
     <tr class ="row-a"> 
     <td><span class="math">1</span></td> 
     <td><span class="math">0</span></td> 
     <td><span class="math">False</span></td> 
     </tr> 
     <tr class ="row-b"> 
     <td><span class="math">1</span></td> 
     <td><span class="math">1</span></td> 
     <td><span class="math">True</span></td> 
     </tr> 
     </tbody> 
     </table> 
     </div> 
     </html> 
********************************************************************** 
1 items had failures: 
    1 of   5 in __main__.example_53 
***Test Failed*** 1 failures. 
For whitespace errors, see the file /home/jaap/downloads/sage-3.2.1.alpha2/tmp/.doctest_functional.py 
         [11.5 s] 
exit code: 1024

12/03/2008 12:27:54 PM changed by whuss

Replying to mhansen:

I don't agree with William's positive review on this for some design reasons. First off, it is only useful from the notebook as it just spits out HTML while all other cases of show work fine from the command-line.

I agree, It should also do something useful on the command-line.

Tacking random things and keywords onto show is not a maintainable solution especially since the new functionality is completely orthogonal to what's already there.

Instead this should be some function in a module for creating HTML output of objects.

This is probably cleaner implementation wise, but I don't think html.nested_list() is a good userinterface. Why should the user of the notebook need to know the implementation detail that the output is produced using HTML? And the output of the function is not really html anyway, but html + latex.

It is also inconsistent, since

sage: html(sin(x)/x)

produces acii art, and not html + jsmath.

There are already at least five functions that produce jsmath output in the notebook, which all behave differently:

show():

Produces latex in display mode. And works with graphic objects of course.

view():

Produces latex in inline mode (which is hard to read in the notebook). This has many options that only work on the commandline and with xdvi. For graphic objects it returns a string representation.

typeset():

Same behaviour as view() but has no options.

pretty_print():

produces latex in inline mode.

If used on the graphics objects, it shows it like show(). But it doesn't accept any options, as show() does.

jsmath():

produces latex in display mode. For graphic objects it returns a string representation, but inside latex math-mode.

The docstring says that there is a option mode which changes between display and inline mode.

Unfortunately this only works in doctest mode. In the notebook I get: 

    sage: jsmath(x^2, 'inline')
    Traceback (click to the left for traceback)
    ...
    TypeError: __call__() takes exactly 2 arguments (3 given)

Is there a deeper reason why Sage has all these functions? Or have they just accumulated over the years? A few of these should probably be deprecated.

In my opinion show() is the best of these, because also x.show() works, so it is consistent. It's short and easy to remember. It just needs better documentation.

Would a mode flag for show() like the one for jsmath() be okay? Then it could be extended in the future without adding additional keywords.

Grettings, Wilfried

12/03/2008 02:12:52 PM changed by mabshoff

Wilfried,

please post the summary you made above to sage-devel so we can have some design discussion.

Cheers,

Michael