Opened 13 years ago

Closed 13 years ago

LaTeX representation of negative symbolic fractions still broken

Reported by: Owned by: Damm Burcin Erocal blocker sage-4.5 symbolics latex, sign, minus, pynac William Stein sage-4.5.rc1 Burcin Erocal John Cremona N/A

I think #9086 isn't completly fixed:

sage: var('x y')
sage: latex(-x/y)
\frac{x}{y}
sage: latex(x/-y)
\frac{x}{y}


comment:1 Changed 13 years ago by Leif Leonhardy

Component: algebra → symbolics latex sign minus pynac added → sage-4.4.4 changed from Alex Ghitza to Burcin Erocal major → critical

comment:2 Changed 13 years ago by Leif Leonhardy

Description: modified (diff)

comment:3 Changed 13 years ago by Leif Leonhardy

These are correct, but don't look that nice:

sage: latex(-(-x^2/-x^5))
\frac{-1}{x^{3}}
sage: latex(-(x^2/x^5))
\frac{-1}{x^{3}}
sage: latex(-((-x)^2/x^5))
\frac{-1}{x^{3}}
sage: latex(x^2/-x^5)
\frac{-1}{x^{3}}
sage: latex(x^2/(-x)^5)
\frac{-1}{x^{3}}
sage: latex(-(-2*x^2/-x^5))
\frac{-2}{x^{3}}
sage: latex(-(-x^2/(-3*x^5)))
\frac{-1}{3 \, x^{3}}


comment:4 Changed 13 years ago by Leif Leonhardy

(Note that the above are all broken, i.e. lose their sign, if one x is replaced by y.)

comment:6 Changed 13 years ago by Harald Schilly

i just got a report that this is also broken for

sage: var('a b')
sage: latex(-1 * (a/b))


can we make this a blocker?

comment:7 Changed 13 years ago by Burcin Erocal

Priority: critical → blocker

This is really embarrassing. I'll fix this tonight.

Ping.

comment:9 Changed 13 years ago by Burcin Erocal

It ended up begin an extended night. I'm looking at it right now.

comment:10 Changed 13 years ago by Burcin Erocal

Status: new → needs_review

The pynac package at

contains a fix for this. I want to keep this as easy to review as possible, so the only change is the following simple patch:

diff --git a/ginac/mul.cpp b/ginac/mul.cpp
--- a/ginac/mul.cpp
+++ b/ginac/mul.cpp
@@ -268,6 +268,10 @@
}
} else {
if (numer.is_equal(_ex1) || numer.is_equal(_ex_1)) {
+			          const numeric &coeff = ex_to<numeric>(numer);
+				  if (coeff.is_equal(*_num_1_p) && !coeff.is_parent_pos_char()) {
+				        c.s<<"-";
+				  }
mul(others).eval().print(c);
} else {
mul(numer,mul(others).eval()).hold().print(c);


attachment:trac_9314-latex_mul.patch has the doctest fixes for the Sage library.

I will take care of the pretty printing issues from comment:3 later.

comment:11 Changed 13 years ago by John Cremona

Status: needs_review → positive_review

The new spkg installed fine for me on 4.5.rc0 (+patches at #7379), then the patch applied fine and tests pass ( itested the whole library but without -long). So, while I cannot tell whether the four lines of C++ added to ginac/mul.cpp are correct, this is certainly an improvement and enough to make me give it a positive review.

comment:12 Changed 13 years ago by John Cremona

Authors: → Burcin Erocal → John Cremona

comment:13 Changed 13 years ago by Robert Miller

Merged in: → sage-4.5.rc1 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.