Opened 11 years ago

Closed 11 years ago

# Pretty print LatexExpr directly

Reported by: Owned by: jdemeyer jason major sage-4.8 misc sd35 tdemedts@…, jhpalmieri sage-4.8.alpha6 Jeroen Demeyer, Punarbasu Purkayastha Andrey Novoseltsev, Punarbasu Purkayastha, John Palmieri N/A #12197

An object of type LatexExpr should be pretty-printed as-is, that is

sage: pretty_print(LatexExpr('x^2 + 1'))


should really typeset "x^2 + 1" instead of displaying the string "x^2 + 1".

Also, import LatexExpr globally.

### comment:1 Changed 11 years ago by jdemeyer

• Description modified (diff)

### comment:2 Changed 11 years ago by jdemeyer

• Authors set to Jeroen Demeyer
• Cc tdemedts@… added

### comment:3 Changed 11 years ago by jdemeyer

• Status changed from new to needs_review

### comment:4 Changed 11 years ago by jhpalmieri

• Cc jhpalmieri added

### comment:5 Changed 11 years ago by novoselt

• Reviewers set to Andrey Novoseltsev

I got a bit confused by the description of the class, so I propose the attached change. The main patch has positive review from me.

### comment:6 Changed 11 years ago by jdemeyer

• Description modified (diff)

### comment:7 Changed 11 years ago by jdemeyer

I removed "Strings are wrapped into verbatim environment for typeset output, while LaTeX expressions are left as-is" because I think that's confusing. I added some more documentation changes and examples. Needs review.

### comment:8 Changed 11 years ago by ppurka

There is just one point. It is not possible to combine normal strings with latex strings or LatexExpr? with itself. Something like this fails to produce the right output: LatexExpr(r'\Delta') + LatexExpr(r'\frac{x}2'). This gives the wrong output even though the addition goes through as strings. Proposed patch is attached, though I leave it up to you to decide whether to include it.

Other than this minor point, the patches provided earlier work as advertised. So, positive review from my side.

### comment:9 follow-up: ↓ 10 Changed 11 years ago by novoselt

If we do add the last patch (which is a bit beyond the title of this ticket), then I think that we MUST add a space between concatenated expressions: if you join "x" and "y" into "xy" it may be exactly what you need, but "\alpha" and "x" get joined into "\alphax" which is not a valid code any more (as I have "discovered" at #9478). Plus spaces (or even new lines) help reading the code and adjusting it if necessary.

I also was actually surprised that "\frac{x}2" works, I would put it as "\frac{x}{2}", although it is just a personal preference, I guess ;-)

### comment:10 in reply to: ↑ 9 ; follow-up: ↓ 12 Changed 11 years ago by jdemeyer

If we do add the last patch (which is a bit beyond the title of this ticket), then I think that we MUST add a space between concatenated expressions: if you join "x" and "y" into "xy" it may be exactly what you need, but "\alpha" and "x" get joined into "\alphax" which is not a valid code any more (as I have "discovered" at #9478). Plus spaces (or even new lines) help reading the code and adjusting it if necessary.

The problem is that maybe sometimes you don't want a space. How about adding a "{}" instead which acts like a seperator but otherwise doesn't do much.

### comment:11 Changed 11 years ago by jdemeyer

This breaks the pdf manual:

Runaway argument?
\PYG {g+gp}{sage: }\PYG {n}{LatexExpr}\PYG {p}{(}\PYG {l+s}{r"}\PYG {\ETC.
! Forbidden control sequence found while scanning use of \FancyVerbGetLine.
<inserted text>
\par
l.126604 ...r}\PYG{p}{(}\PYG{l+s}{r"}\PYG{l+s}{^^L
rac\PYGZob{}x\PYGZca{}2 + ...

?
! Emergency stop.
<inserted text>
\par
l.126604 ...r}\PYG{p}{(}\PYG{l+s}{r"}\PYG{l+s}{^^L
rac\PYGZob{}x\PYGZca{}2 + ...

!  ==> Fatal error occurred, no output PDF file produced!
Transcript written on reference.log.


### comment:12 in reply to: ↑ 10 Changed 11 years ago by novoselt

The problem is that maybe sometimes you don't want a space. How about adding a "{}" instead which acts like a seperator but otherwise doesn't do much.

In what situations does the space matter in math mode? It seems to me that if both were valid expressions before, then they can be separated by a space without any harm... As I understand {} will do the same, but it will make the code much uglier, x{}y is not as cool as x y. And I think that readability of the LaTeX output is a valid concern since I quite regularly take some Sage generated code and then do some manual tweaking. For example - take a long polynomial and then typeset it in an align-environment in a paper. In editors that auto-wrap lines on spaces it is also preferable to have just a space separator.

If there are situations when space does not work, which I think are quite rare since I cannot think of any, one always has an option of converting things to strings, doing anything there and then assembling them back. On a related note, it would be nice to have automatic vertical assembling of such expressions as well, but it is definitely beyond this ticket.

### comment:13 follow-up: ↓ 14 Changed 11 years ago by novoselt

Which patch does break the PDF manual? The last one or all of them?

### comment:14 in reply to: ↑ 13 Changed 11 years ago by jdemeyer

• Status changed from needs_review to needs_work
• Work issues set to _add_()

Which patch does break the PDF manual? The last one or all of them?

Problem solved by using r""" instead of """.

Concerning adding: you are right, I did not consider that we are using math mode all the time. But you should use _add_ (single underscores). I don't know why, but Sage does it this way.

### comment:15 Changed 11 years ago by novoselt

I think _add_ is used so that coercion can work properly. But it requires inheriting from SageObject or maybe even Element, so that generic __add__ can do its part of the job. So I doubt it will work for LatexExpr which inherits from str and nothing else.

### comment:16 Changed 11 years ago by ppurka

Actually space seems to work just fine to separate elements. Since we are in math mode, JSMath will collapse the extra spaces automatically.

As mentioned by novoselt, _add_ does not work. I tried it out just now. Updated the patch.

### Changed 11 years ago by jdemeyer

Add to LatexExpr? (added space in between the two)

### comment:17 Changed 11 years ago by jdemeyer

• Description modified (diff)
• Work issues _add_() deleted

### comment:18 Changed 11 years ago by jdemeyer

• Status changed from needs_work to needs_review

Positive review to everything except my extra_doc patch.

### comment:19 Changed 11 years ago by jdemeyer

• Description modified (diff)

### comment:20 Changed 11 years ago by novoselt

• Authors changed from Jeroen Demeyer to Jeroen Demeyer, Punarbasu Purkayastha
• Reviewers changed from Andrey Novoseltsev to Andrey Novoseltsev, Punarbasu Purkayastha
• Status changed from needs_review to positive_review

Positive review to the last patch as well! Why do we have the reviewer field before the author one?..

### comment:21 Changed 11 years ago by jdemeyer

• Status changed from positive_review to needs_work

Unfortunately there are some doctest errors in various unrelated files. I am working on a patch.

### comment:22 Changed 11 years ago by jdemeyer

• Work issues set to sd35

### comment:23 Changed 11 years ago by jdemeyer

• Keywords sd35 added
• Work issues sd35 deleted

### comment:24 Changed 11 years ago by jdemeyer

• Dependencies set to #12197
• Status changed from needs_work to needs_review

### comment:25 Changed 11 years ago by jdemeyer

• Description modified (diff)

### comment:26 Changed 11 years ago by jdemeyer

New patch also allows str + LatexExpr? additions, which needed a few changes in various places inside Sage. I left one more substantial change for differential forms at #12197.

Needs review.

### comment:27 Changed 11 years ago by jhpalmieri

• Description modified (diff)
• Reviewers changed from Andrey Novoseltsev, Punarbasu Purkayastha to Andrey Novoseltsev, Punarbasu Purkayastha, John Palmieri

I think I'll join the patch party: I'm attaching a patch which rewords a few docstrings. Otherwise, everything looks good to me. With the patch at #12197 and all of the patches here, all tests pass.

I'm willing to give everything but my patch a positive review.

### comment:28 Changed 11 years ago by jdemeyer

• Status changed from needs_review to positive_review

Reviewer patch looks fine.

### comment:29 Changed 11 years ago by ppurka

I guess you are patching some 4.8alpha version of sage. I was patching 4.7.2, so I had to modify the patch 12156_fix_latex.patch for sage/modular/dirichlet.py to read as:

• ## sage/modular/dirichlet.py

diff --git a/sage/modular/dirichlet.py b/sage/modular/dirichlet.py
 a for i in range(len(self.values_on_gens())): if i != 0: mapst += ', ' mapst += latex(self.parent().unit_gens()[i]) + ' \mapsto ' + latex(self.values_on_gens()[i]) mapst += self.parent().unit_gens()[i]._latex_() + ' \mapsto ' + self.values_on_gens()[i]._latex_() return r'Dirichlet character modulo %s of conductor %s mapping $%s$'%(self.modulus(), self.conductor(), mapst) def base_ring(self):

This, along with #12197 makes it pass the doctests. So, positive review from me too.

### comment:30 Changed 11 years ago by jdemeyer

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