Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typo in lambert_w._print_latex_() #19336

Closed
rwst opened this issue Oct 3, 2015 · 17 comments
Closed

typo in lambert_w._print_latex_() #19336

rwst opened this issue Oct 3, 2015 · 17 comments

Comments

@rwst
Copy link

rwst commented Oct 3, 2015

From https://groups.google.com/forum/?hl=en#!topic/sage-support/bjrk5270wEE

LaTeX output is defect.

this is probably

diff --git a/src/sage/functions/log.py b/src/sage/functions/log.py
index cc59437..17241ad 100644
--- a/src/sage/functions/log.py
+++ b/src/sage/functions/log.py
@@ -766,8 +766,8 @@ class Function_lambert_w(BuiltinFunction):
             \operatorname{W_{1}}(x)
         """
         if n == 0:
-            return r"\operatorname{W_0}(%s)" % z
+            return r"\operatorname{W_0}({%s})" % z
         else:
-            return r"\operatorname{W_{%s}}(%s)" % (n, z)
+            return r"\operatorname{W_{%s}}({%s})" % (n, z)
 
 lambert_w = Function_lambert_w()

CC: @kcrisman

Component: symbolics

Author: Frédéric Chapoton

Branch/Commit: a87b149

Reviewer: Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/19336

@rwst rwst added this to the sage-6.9 milestone Oct 3, 2015
@videlec
Copy link
Contributor

videlec commented Oct 5, 2015

comment:3

What is the point of having a separate n==0 case?

@fchapoton
Copy link
Contributor

Commit: 6a9b8ea

@fchapoton
Copy link
Contributor

Branch: public/19336

@fchapoton
Copy link
Contributor

New commits:

6a9b8eatrac #19336 fixing custom latex of Lambert W function

@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@nbruin
Copy link
Contributor

nbruin commented Oct 6, 2015

comment:5

Inserting the braces is a nice start, but it doesn't fix the entire problem. Compare:

sage: var("a1,a2")
(a1, a2)
sage:  latex(log( (a1+a1*a2)/a2))
\log\left(\frac{a_{1} a_{2} + a_{1}}{a_{2}}\right)
sage:  latex(lambert_w( (a1+a1*a2)/a2))
\operatorname{W_0}({(a1*a2 + a1)/a2})

As you can see a function like "log" processes its arguments further for latex. Just inserting braces doesn't cut it.

In bessel.py, the latex-ing of the arguments is done explicitly:

    def _print_latex_(self, n, z):
        return r"\operatorname{J_{%s}}(%s)" % (latex(n), latex(z))

That's probably the thing to do for lambert_w too.

@nbruin
Copy link
Contributor

nbruin commented Oct 6, 2015

comment:6

Replying to @videlec:

What is the point of having a separate n==0 case?

There's a case split for normal printing, suppressing a zero argument:

sage: lambert_w(0,x)
lambert_w(x)
sage: lambert_w(1,x)
lambert_w(1, x)

Possibly the same was done for latex at some point. I agree that with the current code, the case split can be removed in the latex code.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Changed commit from 6a9b8ea to 70d9d51

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

b7620a2Merge branch 'public/19336' into 6.9.rc3
70d9d51trac #19336 latex of lambert_w again, plus a few local enhancements

@fchapoton
Copy link
Contributor

comment:8

Here is a version with latex applied to the argument.

I have also taken the opportunity to enhance the link with maxima, which now has the other branches of LambertW.

@fchapoton
Copy link
Contributor

comment:9

ping ?

@rwst
Copy link
Author

rwst commented Oct 12, 2015

Reviewer: Ralf Stephan

@rwst
Copy link
Author

rwst commented Oct 12, 2015

comment:10

Could you please add doctests for the Maxima additions? When done you can set positive since patchbot is already ok with it.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

d7c946eMerge branch 'public/19336' into 6.9
a87b149trac #19336 some more doctest for maxima lambert_w

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2015

Changed commit from 70d9d51 to a87b149

@fchapoton
Copy link
Contributor

comment:12

ok, I have added some doctests, hopefully corresponding to what you asked.

I set to positive review.

@vbraun
Copy link
Member

vbraun commented Oct 14, 2015

Changed branch from public/19336 to a87b149

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants