Repository: sympy/sympy
Pull request: Latex: Fixed long_frac_ratio
Correct output for fractions involving terms
in numerator more than twice the terms in
denominator.

Fixes https://github.com/sympy/sympy/issues/14309

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests .-->


#### Brief description of what is fixed or changed


#### Other comments


Problem statement:
Wrong latex output for fraction with a sqrt
Latex output for some fractions, which involve a square root is wrong:
```
In [2]: x = sympy.symbols('x') 

In [4]: sympy.latex((2*sympy.sqrt(2)*x)/3)
Out[4]: '\\frac{2 x}{3} \\sqrt{2}'
```
The square root is incorrectly taken out of the fraction.
Without the variable x, the output is correct:
```
In [8]: sympy.latex(2*(sympy.sqrt(2))/3)
Out[8]: '\\frac{2 \\sqrt{2}}{3}'
```

 Tested on sympy version 1.1.1.

Hints from issue discussion:
setting `long_frac_ratio = 3` (ratio of number of terms in num to that in denom) should do the trick.
```
>>>latex((2*sqrt(2)*x)/3,  long_frac_ratio=3)
'\\frac{2 \\sqrt{2} x}{3}'
```
Even so, it would be nice for the LaTeX printer and the pretty printer to be consistent here. 

```
>>> pprint((2*sympy.sqrt(2)*x)/3)
2⋅√2⋅x
──────
  3
```
Without above flag, it's not possible to have more than twice the terms in numerator than in denominator.
```
>>> pprint((a*b*c*d)/e)
a⋅b⋅c⋅d
───────
   e   

>>> latex((a*b*c*d)/e)
'\\frac{a b}{e} c d'
```
Should this rule in latex printer be changed, how about we set `long_frac_ratio` to `None` by default and use it if explicitly mentioned?

Commit message: Added test cases

Patch to review:
diff --git a/sympy/printing/latex.py b/sympy/printing/latex.py
index 28e3f1ba530d..65d3e3dd1c2d 100644
--- a/sympy/printing/latex.py
+++ b/sympy/printing/latex.py
@@ -128,7 +128,7 @@ class LatexPrinter(Printer):
         "fold_frac_powers": False,
         "fold_func_brackets": False,
         "fold_short_frac": None,
-        "long_frac_ratio": 2,
+        "long_frac_ratio": None,
         "mul_symbol": None,
         "inv_trig_style": "abbreviated",
         "mat_str": None,
@@ -446,9 +446,17 @@ def convert(expr):
             snumer = convert(numer)
             sdenom = convert(denom)
             ldenom = len(sdenom.split())
-            ratio = self._settings['long_frac_ratio']
+            if self._settings['long_frac_ratio'] == None:
+                exprfind = [x for x in expr.args if \
+                    str(x).startswith('1/')]
+                if len(exprfind) >= 1:
+                    ratio = len(expr.args) - 1
+                else:
+                    ratio = len(expr.args)
+            else:
+                ratio = self._settings['long_frac_ratio']
             if self._settings['fold_short_frac'] \
-                    and ldenom <= 2 and not "^" in sdenom:
+                   and ldenom <= 2 and not "^" in sdenom:
                 # handle short fractions
                 if self._needs_mul_brackets(numer, last=False):
                     tex += r"\left(%s\right) / %s" % (snumer, sdenom)
@@ -2279,7 +2287,8 @@ def latex(expr, **settings):
 
     long_frac_ratio: The allowed ratio of the width of the numerator to the
     width of the denominator before we start breaking off long fractions.
-    The default value is 2.
+    The default value is width of the numerator to the width of the
+    denominator in the expression itself.
 
     >>> print(latex(Integral(r, r)/2/pi, long_frac_ratio=2))
     \frac{\int r\, dr}{2 \pi}
diff --git a/sympy/printing/tests/test_latex.py b/sympy/printing/tests/test_latex.py
index 4f39f4439c6e..989c968dfde3 100644
--- a/sympy/printing/tests/test_latex.py
+++ b/sympy/printing/tests/test_latex.py
@@ -86,6 +86,9 @@ def test_latex_basic():
         r"\frac{1}{2 x} \left(x + y\right)"
     assert latex((x + y)/x) == r"\frac{1}{x} \left(x + y\right)"
     assert latex((x + y)/x, long_frac_ratio=3) == r"\frac{x + y}{x}"
+    assert latex((2*sqrt(2)*x)/3) == r"\frac{2 \sqrt{2} x}{3}"
+    assert latex((2*sqrt(2)*x)/3, long_frac_ratio=2) == \
+        r"\frac{2 x}{3} \sqrt{2}"
 
     assert latex(2*Integral(x, x)/3) == r"\frac{2}{3} \int x\, dx"
     assert latex(2*Integral(x, x)/3, fold_short_frac=True) == \


Code review comments:
diff_hunk: @@ -446,9 +446,17 @@ def convert(expr):
             snumer = convert(numer)
             sdenom = convert(denom)
             ldenom = len(sdenom.split())
-            ratio = self._settings['long_frac_ratio']
+            if self._settings['long_frac_ratio'] == None:
+                exprfind = [x for x in expr.args if \
+                    str(x).startswith('1/')]
path: sympy/printing/latex.py
text: @user1:
This should be tested directly. You shouldn't use the str form of an expression to test that it has some form. 

@author:
The ```startswith``` method or regex functions only support strings. So how can I check if a 'rational' object starts with something without converting into a string?

@user2:
If you really mean an object of class Rational, there is property "p" that holds the numerator. But probably there are other classes involved. Maybe you want to count the number of terms in the numerator or denominator. The method `as_numer_denom` may be helpful then. 

But I don't understand why you are doing this. Was the idea to prevent splitting when ratio is None? Then the place to do that would be a bit below, where the condition `len(snumer.split()) > ratio*ldenom` would be replaced by 
`ratio is not None and len(snumer.split()) > ratio*ldenom`. No need to come up with some value for "ratio" when it's None. 

@author:
@user2 I modified the condition but there seems to be several failed test cases. Can you please tell me what is wrong with my code? I just wanted to change the value of "ratio" according to the number of terms in the numerator or denominator and my code seems to do it just fine.

@user2:
What's wrong was already stated above: "You shouldn't use the str form of an expression to test that it has some form"

@author:
@user2  Yes I understand that but as I have already stated that the ```startswith``` method or regex functions only support strings so how can I check if a 'rational' object starts with something without converting into a string? I need it to check because I need the correct number of terms in numerator and denominator and without it the output might be more than expected if the expression contains constant terms or so.

@user2:
The str test is unacceptable. So you either find an acceptable solution, or find something else to do.