Opened 9 years ago

Closed 9 years ago

#9314 closed defect (fixed)

LaTeX representation of negative symbolic fractions still broken

Reported by: damm Owned by: burcin
Priority: blocker Milestone: sage-4.5
Component: symbolics Keywords: latex, sign, minus, pynac
Cc: was Merged in: sage-4.5.rc1
Authors: Burcin Erocal Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by leif)

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}

Attachments (1)

trac_9314-latex_mul.patch (902 bytes) - added by burcin 9 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 9 years ago by leif

  • Component changed from algebra to symbolics
  • Keywords latex sign minus pynac added
  • Milestone set to sage-4.4.4
  • Owner changed from AlexGhitza to burcin
  • Priority changed from major to critical

comment:2 Changed 9 years ago by leif

  • Description modified (diff)

comment:3 Changed 9 years ago by leif

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 9 years ago by leif

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

comment:5 Changed 9 years ago by leif

  • Cc was added

comment:6 Changed 9 years ago by 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 9 years ago by burcin

  • Priority changed from critical to blocker

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

comment:8 Changed 9 years ago by mhansen

Ping.

comment:9 Changed 9 years ago by burcin

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

Changed 9 years ago by burcin

comment:10 Changed 9 years ago by burcin

  • Status changed from new to needs_review

The pynac package at

http://sage.math.washington.edu/home/burcin/pynac/pynac-0.2.0.p4.spkg

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 9 years ago by cremona

  • Status changed from needs_review to 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 9 years ago by cremona

  • Authors set to Burcin Erocal
  • Reviewers set to John Cremona

comment:13 Changed 9 years ago by rlm

  • Merged in set to sage-4.5.rc1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.