Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

printing bug with list_function() #2584

Closed
craigcitro opened this issue Mar 18, 2008 · 10 comments
Closed

printing bug with list_function() #2584

craigcitro opened this issue Mar 18, 2008 · 10 comments

Comments

@craigcitro
Copy link
Member

This causes a bug when printing:

show([Matrix(ZZ,3,range(9)), Matrix(ZZ,3,range(9))])

Notice the extra ",". A list of one element doesn't have the same problem.

Component: graphics

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

@jasongrout
Copy link
Member

comment:1

The bug is in the latex function:

sage: latex([Matrix(ZZ,3,range(9)), Matrix(ZZ,3,range(9))])
\begin{array}{l}[\left(\begin{array}{rrr}
0 & 1 & 2 \\
3 & 4 & 5 \\
6 & 7 & 8
\end{array}\right),\\
\left(\begin{array}{rrr}
0 & 1 & 2 \\
3 & 4 & 5 \\
6 & 7 & 8
\end{array}\right)],\\
\end{array}

@jasongrout jasongrout changed the title printing bug with show() printing bug with latex() Mar 18, 2008
@jasongrout
Copy link
Member

comment:2

Actually, the bug is in the list_function in latex.py in the case where the list string is too long. I'll post a patch momentarily.

@jasongrout jasongrout changed the title printing bug with latex() printing bug with list_function() Mar 18, 2008
@jasongrout
Copy link
Member

Attachment: list-print-extra-comma.patch.gz

@jasongrout
Copy link
Member

comment:3

I believe the above patch works, but I'm still building 2.10.4, so I haven't tested it yet.

@jasongrout
Copy link
Member

comment:4

The patch fixes the problem.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 18, 2008

comment:5

Replying to @jasongrout:

The patch fixes the problem.

While the patch looks like the fix to this problem shouldn't we also add a doctest that verifies that the bug has been fixed? Not to be pedantic ... well, I am :=)

Great job Jason.

Cheers,

Michael

@mwhansen
Copy link
Contributor

Attachment: 2584.patch.gz

@mwhansen
Copy link
Contributor

comment:6

I added some doctests; only the last patch should be applied. Positive review from me.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 19, 2008

comment:7

Merged 2584.patch in Sage 2.11.alpha0 - thanks Mike for the doctest.

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Mar 19, 2008
@jasongrout
Copy link
Member

comment:8

Thanks, Mike for adding the doctests. The reason I didn't add doctests to test this fix was that the bug was only manifest in the notebook (in EMBEDDED_MODE). However, I should have gone ahead and added doctests for the basic functionality anyway, even if they didn't test that the bug in question is fixed, just to get the doctest score up and do my part towards the 3.0 goals :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants