Opened 10 years ago
Closed 10 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 )
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)
Change History (14)
comment:1 Changed 10 years ago by
- 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 10 years ago by
- Description modified (diff)
comment:3 Changed 10 years ago by
comment:4 Changed 10 years ago by
(Note that the above are all broken, i.e. lose their sign, if one x
is replaced by y
.)
comment:5 Changed 10 years ago by
- Cc was added
comment:6 Changed 10 years ago by
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 10 years ago by
- Priority changed from critical to blocker
This is really embarrassing. I'll fix this tonight.
comment:8 Changed 10 years ago by
Ping.
comment:9 Changed 10 years ago by
It ended up begin an extended night. I'm looking at it right now.
Changed 10 years ago by
comment:10 Changed 10 years ago by
- 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 10 years ago by
- 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 10 years ago by
- Reviewers set to John Cremona
comment:13 Changed 10 years ago by
- Merged in set to sage-4.5.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
Applied
http://sage.math.washington.edu/home/burcin/pynac/pynac-0.2.0.p4.spkg
and
attachment:trac_9314-latex_mul.patch
to sage-4.5.rc1.
These are correct, but don't look that nice: