Opened 3 years ago

Closed 20 months ago

Last modified 19 months ago

#13131 closed enhancement (fixed)

Make a top-level table/Table function

Reported by: kcrisman Owned by: was
Priority: major Milestone: sage-5.10
Component: user interface Keywords:
Cc: jason, jhpalmieri, abmasse, novoselt, andrew.mathas Merged in: sage-5.10.beta0
Authors: John Palmieri Reviewers: Jason Grout, Karl-Dieter Crisman, Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by slabbe)

As the summary says.

See this sage-devel thread for more suggestions, including that we should have a Table class that table calls, which uses something like html.table in the notebook, but also has a _latex_ method, makes ascii art in the command line, etc.

Just a few examples from the doctests in the patch:

        sage: table([['a', 'b', 'c'], [100,2,3], [4,5,60]])
          a     b   c
          100   2   3
          4     5   60
        sage: table([['a', 'b', 'c'], [100,2,3], [4,5,60]], frame=True)
        +-----+---+----+
        | a   | b | c  |
        +-----+---+----+
        | 100 | 2 | 3  |
        +-----+---+----+
        | 4   | 5 | 60 |
        +-----+---+----+

        sage: latex(table([['a', 'b', 'c'], [100,2,3], [4,5,60]], frame=True))
        \begin{tabular}{|l|l|l|} \hline
        a & b & c \\ \hline
        $100$ & $2$ & $3$ \\ \hline
        $4$ & $5$ & $60$ \\ \hline
        \end{tabular}

You can also specify alignments of 'left', 'center', or 'right'.

Apply trac_13131-tables-with-columns.patch, trac_13131_review-sl.patch.

Attachments (3)

trac_13131-tables.patch (29.6 KB) - added by jhpalmieri 22 months ago.
trac_13131-tables-with-columns.patch (31.9 KB) - added by jhpalmieri 21 months ago.
trac_13131_review-sl.patch (3.1 KB) - added by slabbe 21 months ago.
review: Spaces and r""" fixes

Download all attachments as: .zip

Change History (42)

comment:1 Changed 3 years ago by abmasse

  • Cc abmasse added

comment:2 Changed 2 years ago by jhpalmieri

Some random comments and questions:

  • When defining a table, the user should be able to specify position/alignment (left, right, center). Should they be able to do this for each column, and do we like LaTeX notation for this? Should they be able to specify positioning for each individual entry?
  • Is it too much work to try to align numbers at their decimal places? LaTeX and LaTeX ways to do this. html and html ways to do this, but perhaps complicated or messy.
  • Should we refactor the matrix printing code so that it uses this class?
  • html.table expects a nested list as input, and so the entries in [1,2,3] get plotted as separate rows. is that the right thing, or should [1,2,3] get plotted as a single row?
  • We should probably steal code from html.table, matrix._repr_, matrix.str, matrix._latex_
  • Should we allow subtables (like subdivisions in matrices)?

comment:3 Changed 2 years ago by jason

Sure. But minimally, how about: table(list of rows, header=header row). I think we can definitely start simple here.

References: Mathematica's Grid command: http://reference.wolfram.com/mathematica/howto/FormatATableOfData.html, http://reference.wolfram.com/mathematica/ref/Grid.html, and TableForm?: http://reference.wolfram.com/mathematica/ref/TableForm.html

comment:4 Changed 2 years ago by jhpalmieri

  • Authors set to John Palmieri
  • Description modified (diff)
  • Status changed from new to needs_review

Okay, here's a first attempt. For some reason I find the syntax table(array, header=another_row) strange, so this just allows for the header argument to be True or False, and the actual row needs to be the first row of array.

comment:5 Changed 2 years ago by jhpalmieri

To get header_column to have any effect in HTML, we also will need to patch the Sage notebook, to modify the file devel/sagenb/sagenb/data/sage/css/main.css:

  • main.css

    old new  
    445445table.table_form * td { padding-top: 5px; padding-bottom: 5px; padding-left: 15px; padding-right: 15px; } 
    446446/* line 899, ../../../../sass/src/main.scss */ 
    447447table.table_form * th { background: #a6ba4e; height: 29px; padding-left: 15px; padding-right: 15px; padding-top: 5px; padding-bottom: 5px; color: white; text-align: left; } 
     448table.table_form * th.ch { background: #7c8b3a; } 
     449table.table_form * td.ch { background: #a6ba4e; color: white; } 
    448450/* line 909, ../../../../sass/src/main.scss */ 
    449451table.table_form * tr.row-a { background: #f8f8f8; text-align: left; } 
    450452/* line 912, ../../../../sass/src/main.scss */ 

I guess this means a change to devel/sagenb/sass/src/main.scss, but I'm not sure of the syntax in that file. Maybe this:

  • main.scss

    old new  
    895895      padding-top: 5px; 
    896896      padding-bottom: 5px; 
    897897      padding-left: 15px; 
    898       padding-right: 15px; } 
     898      padding-right: 15px; 
     899      &.ch { 
     900        background: #a6ba4e; 
     901        color: white; } } 
    899902    th { 
    900903      background: #a6ba4e; 
    901904      height: 29px; 
     
    904907      padding-top: 5px; 
    905908      padding-bottom: 5px; 
    906909      color: white; 
    907       text-align: left; } 
     910      text-align: left; 
     911      &.ch { 
     912        background: #7c8b3a; } } 
    908913    tr { 
    909914      &.row-a { 
    910915        background: #f8f8f8; 

This is not a big deal and the ticket can be merged independently of such a change. I may get around to putting up a pull request for the notebook, but no guarantees.

comment:6 Changed 2 years ago by jhpalmieri

Oh, one more thing, at least for now: the previously existing technique of using html.table(...) treated a single list as a column, whereas this patch treats a single list as a row instead. This feels more natural to me. Anyone who really wants either a single row or a single column should use nested lists properly anyway.

comment:7 Changed 2 years ago by kcrisman

My refrain of the last few months to your patches has been "I don't have time to give it a proper review, but this looks great on the face of it", so why not repeat it? I look forward to using this, and I know a lot of other people who rely on the equivalent way too much in Mma will love it.

Last edited 2 years ago by kcrisman (previous) (diff)

comment:8 Changed 2 years ago by jason

Right now html.table(data, header=True) treats the first row as the header, but html.table(data, header=['label1', 'label2']) will let you specify the header independently. This is *extremely* useful. Usually I have a table of numbers or something, and being able to just specify the header row separately makes a huge difference in clarity.

comment:9 Changed 2 years ago by jhpalmieri

I think that

table([head] + data, header_row=True)

is just about as clear as

table(data, header_row=head)

especially since head comes first in the first version, consistent with its position in the table. But here's a new patch which implements both, and the same for header_column.

comment:10 Changed 2 years ago by jason

Thanks! I was going to submit a follow-up patch to add that, but thanks for taking care of it!

comment:11 Changed 2 years ago by jhpalmieri

By the way, doing something like

table(data, header_row=['a', 'b', 'c'], header_column=['x', 'y'])

is just strange. I can explain what it would do – first add the header row at the top of the data, then add the header column on the left – but in the docstring I just said that doing this is "not supported". Someone can specify True for one argument and a list for the other, or True for both, but I don't want to document using a list for both.

comment:12 Changed 2 years ago by jhpalmieri

New patch, expanding on the documentation of the options method, and also running a Testsuite.

comment:13 Changed 2 years ago by novoselt

  • Cc novoselt added

comment:14 follow-ups: Changed 22 months ago by kcrisman

  • Reviewers set to Jason Grout, Karl-Dieter Crisman
  • Status changed from needs_review to needs_work

Some dumb questions.

  • elif header_row is not False: why not just elif header_row? Presumably a Python subtlety.
  • Do we need something in html.table() (what remains of it) to change it so that the previous behavior of a single list giving a column and not a row is preserved? I know you removed the example, but technically speaking one should deprecate this...
  • Is there an extra
    self._options['header_column'] = header_column
    
    in there? It should be set already above that.
  • I don't see any error catching. This is particularly important in making sure someone doesn't (contra the doc, but still) try to add in a header row later on via header_row=[1,2,3] and in making sure that the table is, in fact, a rectangle. I think that for sanity, especially with very large tables, this would be helpful to have this error instead of who-knows-what-crazy-error Sage would raise otherwise.
  • Feature request; take a list of lists and just fill in the rest! I had to make my own code for this in some sense, filling in empty slots, if I recall correctly. But presumably this is like the list in comment:2 and for a future ticket.

comment:15 follow-up: Changed 22 months ago by kcrisman

  • Cc andrew.mathas added

I was just speaking with Andrew Mathas of the Sage-combinat group at Sage Days at ICERM and it turns out that he has a pretty impressive (minor bugs remaining) table object as well, which takes any iterable as a header column with indexing and has a slew of formatting options. Although the current functionality is almost ready for showtime, it would be useful to have some dialogue about the relation between these things, especially because some of the formatting stuff is nice. He also has a nice hook to ask for displaying a table in a browser automatically (html gets sent to the browser).

Last edited 22 months ago by kcrisman (previous) (diff)

comment:16 in reply to: ↑ 15 Changed 22 months ago by kcrisman

Replying to kcrisman:

I was just speaking with Andrew Mathas of the Sage-combinat group at Sage Days at ICERM and it turns out that he has a pretty impressive (minor bugs remaining) table object as well, which takes any iterable as a header column with indexing and has a slew of formatting options. Although the current functionality is almost ready for showtime, it would be useful to have some dialogue about the relation between these things, especially because some of the formatting stuff is nice. He also has a nice hook to ask for displaying a table in a browser automatically (html gets sent to the browser).

See #14103.

comment:17 in reply to: ↑ 14 Changed 22 months ago by kcrisman

  • Feature request; take a list of lists and just fill in the rest! I had to make my own code for this in some sense, filling in empty slots, if I recall correctly. But presumably this is like the list in comment:2 and for a future ticket.

This already works. I think I was wanting to have this work better, so that I didn't have to use map(None,...) and then replace with ''.

@interact 
def _(n=7): 
    L = map(None,[p for p in prime_range(n+1) if p%4==1],[p for p in prime_range(n+1) if p%4==3]) 
    L = [['',l[1]] if l[0] is None else l for l in L] 
    T = [['$p\equiv 1\\text{ mod}(4)$','$p\equiv 3\\text{ mod}(4)$']]    
    html.table(T+L,header=True)

But probably that's irrelevant.

comment:18 Changed 22 months ago by jason

Whew. I would probably do that either of these two ways (the second way is more efficient, but really you could even do better by caching the table from run to run)

from itertools import izip_longest
from collections import defaultdict

@interact 
def _(n=7):
    L = list(izip_longest([p for p in prime_range(n+1) if p%4==1],[p for p in prime_range(n+1) if p%4==3],fillvalue=''))
    html.table(L,header=['$p\equiv 1\\text{ mod}(4)$','$p\equiv 3\\text{ mod}(4)$'])


@interact 
def _(n=7):
    m = defaultdict(list)
    for p in prime_range(n+1):
        m[p%4].append(p)
    L = list(izip_longest(m[1], m[3],fillvalue=''))
    html.table(L,header=['$p\equiv 1\\text{ mod}(4)$','$p\equiv 3\\text{ mod}(4)$'])

comment:19 Changed 22 months ago by kcrisman

Hmm, so you're saying it's not really a failing in html.table?

comment:20 Changed 22 months ago by jason

If html.table took a list of columns, then I think you could argue your case. Since it takes a list of rows, you have to pad the columns with the empty cells.

If you formatted your table as two rows, then I think html.table should (but probably doesn't) format different-sized rows.

On the other hand, it seems useful to have a columns option of some sort to the table. Or maybe we can just use pandas (eventually) for more sophisticated formatting.

comment:21 in reply to: ↑ 14 Changed 22 months ago by jhpalmieri

Replying to kcrisman:

Some dumb questions.

  • elif header_row is not False: why not just elif header_row? Presumably a Python subtlety.

I think that can be changed.

  • Do we need something in html.table() (what remains of it) to change it so that the previous behavior of a single list giving a column and not a row is preserved? I know you removed the example, but technically speaking one should deprecate this...

Technically speaking, I think you're right. I don't like the old behavior at all; it doesn't make sense to me. It might actually make sense to just raise an error if you pass a single list, not a nested list. I'm not sure what the right thing to do is.

  • Is there an extra
    self._options['header_column'] = header_column
    
    in there? It should be set already above that.

Yes, you're right.

  • I don't see any error catching. This is particularly important in making sure someone doesn't (contra the doc, but still) try to add in a header row later on via header_row=[1,2,3]

So you're suggesting that

sage: T = table([[1,2,3], [4,5,6]])
sage: T.options(header_row=['x', 'y', 'z'])

should raise an error instead of just silently making [1,2,3] the header? Okay, I guess that makes sense.

and in making sure that the table is, in fact, a rectangle. I think that for sanity, especially with very large tables, this would be helpful to have this error instead of who-knows-what-crazy-error Sage would raise otherwise.

  • Feature request; take a list of lists and just fill in the rest! I had to make my own code for this in some sense, filling in empty slots, if I recall correctly. But presumably this is like the list in comment:2 and for a future ticket.

I'm not sure how these last two items relate to each other: the last one sounds like you want to be able to pass lists of different lengths and have the gaps filled in, and the previous one sounds like you want to raise an error if you pass lists of different lengths. But maybe I don't understand.

Would it be useful to have a transpose method to switch rows and columns? Here's a new patch which adds that, and also addresses all of the issues except the old behavior of html.table() and the last issue about a non-rectangular situation.

Changed 22 months ago by jhpalmieri

comment:22 Changed 22 months ago by jhpalmieri

I also added documentation (in the eval method in html.py) for calling the _html_ method for an object, if it has one. This is analogous to the _latex_ method. I think this could be useful in other situations.

comment:23 Changed 22 months ago by jhpalmieri

  • Status changed from needs_work to needs_review

comment:24 follow-ups: Changed 21 months ago by slabbe

Thanks for clarifying the documentation of html(_) and for making html calling _html_ if it exists.

I have two questions:

  1. Doc of html now says:
In any case, *print* the resulting html string. This method 
always *returns* an empty string. 

What if I want to print the output in a file and not in the shell. What should I do? This is something I came across in the past.

  1. I really like the table function. I am defining such a function in a patch I posted just yesterday at #13069. In my case, the function takes columns as input and not rows. So, if I want to use your code, would you propose an interface that takes columns as input:
sage: columns = [('a', 100, 4), ('b', 2, 5), ('c', 3, 60)] 
sage: table(columns=columns)

or the user should transpose columns into rows himself?

sage: columns = [('a', 100, 4), ('b', 2, 5), ('c', 3, 60)] 
sage: rows = zip(*columns)
sage: rows
[('a', 'b', 'c'), (100, 2, 3), (4, 5, 60)]
sage: table(rows)

comment:25 Changed 21 months ago by novoselt

I would prefer a more straightforward method then going through zipping (which would not immediately occur to me if I was thinking of it).

comment:26 in reply to: ↑ 24 Changed 21 months ago by kcrisman

  1. I really like the table function. I am defining such a function in a patch I posted just yesterday at #13069. In my case, the function takes columns as input and not rows. So, if I want to use your code, would you propose an interface that takes columns as input:

See yet another implementation at #14103.

+1 to being straightforward.

comment:27 in reply to: ↑ 24 Changed 21 months ago by slabbe

Replying to slabbe:

  1. I really like the table function. I am defining such a function in a patch I posted just yesterday at #13069.

To make the reference easier, here is the function I coded at #13069 (the patch is 90 Kb).

def table_from_columns(col1, col2, space=3):
    r"""
    Return a string of a table of two columns.

    This is used by the ``_repr_`` method of DoubleSquare class.

    INPUT:

    - ``col1`` - list
    - ``col2`` - list
    - ``space`` - integer (default ``3``), number of space characters
      between

    EXAMPLES::

        sage: from sage.combinat.double_square_tile import table_from_columns
        sage: col1 = ['ab','asdfasdf', 'adf']
        sage: col2 = ['11', '1313', '131313', '1313']
        sage: print table_from_columns(col1, col2)
        ab         11
        asdfasdf   1313
        adf        131313
                   1313
    """
    from itertools import izip_longest
    width_col1 = max(map(len, col1))
    it = izip_longest(col1,col2, fillvalue='')
    L = [a.ljust(width_col1 + space) + b for (a,b) in it]
    return '\n'.join(L)

comment:28 in reply to: ↑ 24 ; follow-up: Changed 21 months ago by jhpalmieri

Replying to slabbe:

Thanks for clarifying the documentation of html(_) and for making html calling _html_ if it exists.

I have two questions:

  1. Doc of html now says:
In any case, *print* the resulting html string. This method 
always *returns* an empty string. 

What if I want to print the output in a file and not in the shell. What should I do? This is something I came across in the past.

This is just documentation; I haven't changed the code. If we want to change the behavior of html (maybe just adding a str method or something like that), let's do it on another ticket and keep this one focused on tables.

  1. I really like the table function. I am defining such a function in a patch I posted just yesterday at #13069. In my case, the function takes columns as input and not rows. So, if I want to use your code, would you propose an interface that takes columns as input:
sage: columns = [('a', 100, 4), ('b', 2, 5), ('c', 3, 60)] 
sage: table(columns=columns)

I can add this, but what should happen if you do table(array, column=columns)? Raise an error? Or try to combine the two sets of data somehow? I think I prefer to raise an error.

or the user should transpose columns into rows himself?

The easiest thing to do with the code as it stands is table(columns).transpose().

comment:29 in reply to: ↑ 28 Changed 21 months ago by slabbe

Replying to jhpalmieri:

This is just documentation; I haven't changed the code. If we want to change the behavior of html (maybe just adding a str method or something like that), let's do it on another ticket and keep this one focused on tables.

I agree. I just can't close my mouth sometimes!

I can add this, but what should happen if you do table(array, column=columns)? Raise an error? Or try to combine the two sets of data somehow? I think I prefer to raise an error.

or maybe the interface could be :

sage: table(data=rows)
...
sage: table(data=columns, orientation='columns')
...

where orientation would be an optional argument taking 'rows' as default value? This way there can't be inconsistent inputs.

The easiest thing to do with the code as it stands is table(columns).transpose().

ok

comment:30 Changed 21 months ago by jhpalmieri

I think I like

sage: table(rows=...)
...
sage: table(columns=...)
...

because it puts the two formats on equal footing. I've attached a new patch which incorporates this. It also expands on the documentation somewhat.

comment:31 Changed 21 months ago by jhpalmieri

  • Description modified (diff)

Changed 21 months ago by jhpalmieri

comment:32 follow-up: Changed 21 months ago by slabbe

  • Description modified (diff)
  • Reviewers changed from Jason Grout, Karl-Dieter Crisman to Jason Grout, Karl-Dieter Crisman, Sébastien Labbé

Just added a patch which does nothing but removing trailing spaces (on 3 lines) and added the r to the """ to make doc be raw strings. ...sometimes I get doctest errors because I do not put this r. Maybe I am afraid of witches...

Jason, Karl-Dieter, to me this is positive review. Did you have anything else to say?

Sébastien

Changed 21 months ago by slabbe

review: Spaces and r""" fixes

comment:33 Changed 21 months ago by slabbe

Apply trac_13131-tables-with-columns.patch, trac_13131_review-sl.patch

comment:34 in reply to: ↑ 32 Changed 21 months ago by kcrisman

Jason, Karl-Dieter, to me this is positive review. Did you have anything else to say?

If you looked at the latest code carefully, nothing would please me more than for this to get in. I love all the improvements since the very first version. It is probably the number 1 or 2 most-requested basic feature in Sage available at the top level, in my experience, and will be very well received.

comment:35 Changed 21 months ago by jason

I haven't tested it, but I don't see any big red flags in the code either. Thanks!

comment:36 Changed 21 months ago by slabbe

  • Status changed from needs_review to positive_review

comment:37 Changed 21 months ago by jdemeyer

  • Milestone changed from sage-5.9 to sage-5.10

comment:38 Changed 20 months ago by jdemeyer

  • Merged in set to sage-5.10.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:39 Changed 19 months ago by slabbe

see #14601 for a small follow up bug

Note: See TracTickets for help on using tickets.