Opened 11 years ago
Last modified 11 years ago
#12156 closed defect
Pretty print LatexExpr directly — at Version 17
Reported by: | jdemeyer | Owned by: | jason |
---|---|---|---|
Priority: | major | Milestone: | sage-4.8 |
Component: | misc | Keywords: | sd35 |
Cc: | tdemedts@…, jhpalmieri | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Andrey Novoseltsev |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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.
Apply 12156_latex_pretty_print.patch, 12156_reviewer.patch, 12156_add_LatexExpr.patch, 12156_extra_doc.patch
Change History (21)
comment:1 Changed 11 years ago by
- Description modified (diff)
comment:2 Changed 11 years ago by
- Cc tdemedts@… added
Changed 11 years ago by
comment:3 Changed 11 years ago by
- Status changed from new to needs_review
comment:4 Changed 11 years ago by
- Cc jhpalmieri added
Changed 11 years ago by
comment:5 Changed 11 years ago by
- Reviewers set to Andrey Novoseltsev
comment:6 Changed 11 years ago by
- Description modified (diff)
comment:7 Changed 11 years ago by
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
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
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
Replying to 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.
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
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
Replying to jdemeyer:
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
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
- Status changed from needs_review to needs_work
- Work issues set to _add_()
Replying to novoselt:
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
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
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.
comment:17 Changed 11 years ago by
- Description modified (diff)
- Work issues _add_() deleted
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.