Repository: sympy/sympy
Pull request: added `fraction` flag in `factor()`
<!-- 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
fixes: #16263 
<!-- 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 . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
passing `fraction=False` directly to `together` seems to disturb few existing tests, like:

```python
In [5]: f = -1 + 1/(x**2 + 2*x + 1/x) 
In [6]: factor(f)
Out[6]: -1 + 1/(x**2 + 2*x + 1/x)
# instead of `-((1 - x + 2*x**2 + x**3)/(1 + 2*x**2 + x**3))`
```

so it would perhaps be better to have an extra flag to `factor`

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
- polys
  - added `fraction` flag in `factor()`
<!-- END RELEASE NOTES -->


Problem statement:
`solve_lambert()` unable to solve in `1.4.dev`
```python
In [3]: import sympy
In [4]: sympy.__version__
Out[4]: '1.4.dev' 
In [5]: x = symbols('x', real=True)                                             
In [6]: f = 5*x - 1 + 3*exp(2 - 7*x)                                            
In [7]: poly = f.as_poly()                                                      
In [8]: gens = _filtered_gens(poly, x)                                          
In [9]: _solve_lambert(f, x, gens)  
NotImplementedError: 5*x + 3*exp(2 - 7*x) - 1 does not appear to have a solution in terms of LambertW
```
This isn't the case in `1.3`, there might have been recent changes (not in `bivariate.py`, as it hasn't been updated in a while) that have caused this error.

```python
In [1]: import sympy                                                            
In [2]: sympy.__version__                                                       
Out[2]: '1.3'
In [7]: x = symbols('x', real=True)                                             
In [8]: f = 5*x - 1 + 3*exp(2 - 7*x)                                            
In [9]: poly = f.as_poly()                                                      
In [10]: gens = _filtered_gens(poly, x)                                         
In [11]: _solve_lambert(f, x, gens)                                             
Out[11]: [1/5 + LambertW(-21*exp(3/5)/5)/7]
```
I wonder why the Travis did not fail any tests.

Hints from issue discussion:
The issue seems with `factor()`

```python
# in 1.3
In [7]: f = 5*x + 3*exp(-7*x + 2)                                                                                                                     
In [8]: factor(f, deep=True)                                                                                                                          
Out[8]: 5*x + 3*exp(2)*exp(-7*x)

# 1.4.dev
In [31]: f = 5*x + 3*exp(-7*x + 2)                                                                                                                    
In [32]: factor(f, deep=True)                                                                                                                         
Out[32]: (5*x*exp(7*x) + 3*exp(2))*exp(-7*x)
```
Since the form gets changed (`Add` type to a `Mul` type) therefore `_solve_lambert` is unable to identify it as a lambert type, hence raises an error.

I am not sure about what has caused this behaviour, simple hack would be exapand it using `expand()`,
but first we need to be sure about what actually should be the expected behaviour- `1.3` or `.dev`.
@asmeurer @smichr 
cf #16240, #6693 (especially, since what happened here is factor returned together)

Might factor reject a return value if it is more complex in counts than the original?
There are varied options in case of `expand` to keep a check on kind of the transformations. I believe such functionalities should also be there with the `factor`.
cf comment [here](https://github.com/sympy/sympy/pull/16277#issuecomment-473536681) regarding how to fix the expression above involving `exp(-7*x)`.

Commit message: removed `class Fraction` as an option for polynomial, instead used `fraction`
flag directly in `factor`;

Patch to review:
diff --git a/sympy/polys/polytools.py b/sympy/polys/polytools.py
index 886627692851..ac432add443a 100644
--- a/sympy/polys/polytools.py
+++ b/sympy/polys/polytools.py
@@ -5961,7 +5961,7 @@ def _symbolic_factor(expr, opt, method):
     if isinstance(expr, Expr) and not expr.is_Relational:
         if hasattr(expr,'_eval_factor'):
             return expr._eval_factor()
-        coeff, factors = _symbolic_factor_list(together(expr), opt, method)
+        coeff, factors = _symbolic_factor_list(together(expr, fraction=opt.get('fraction', True)), opt, method)
         return _keep_coeff(coeff, _factors_product(factors))
     elif hasattr(expr, 'args'):
         return expr.func(*[_symbolic_factor(arg, opt, method) for arg in expr.args])
@@ -6014,8 +6014,10 @@ def _generic_factor_list(expr, gens, args, method):
 
 def _generic_factor(expr, gens, args, method):
     """Helper function for :func:`sqf` and :func:`factor`. """
+    fraction = args.pop('fraction', True)
     options.allowed_flags(args, [])
     opt = options.build_options(gens, args)
+    opt['fraction'] = fraction
     return _symbolic_factor(sympify(expr), opt, method)
 
 
@@ -6274,7 +6276,7 @@ def factor(f, *gens, **args):
     Examples
     ========
 
-    >>> from sympy import factor, sqrt
+    >>> from sympy import factor, sqrt, exp
     >>> from sympy.abc import x, y
 
     >>> factor(2*x**5 + 2*x**4*y + 4*x**3 + 4*x**2*y + 2*x + 2*y)
@@ -6307,6 +6309,14 @@ def factor(f, *gens, **args):
     >>> factor(eq, deep=True)
     2**((x + 1)**2)
 
+    If the ``fraction`` flag is False then rational expressions
+    won't be combined. By default it is True.
+
+    >>> factor(5*x + 3*exp(2 - 7*x), deep=True)
+    (5*x*exp(7*x) + 3*exp(2))*exp(-7*x)
+    >>> factor(5*x + 3*exp(2 - 7*x), deep=True, fraction=False)
+    5*x + 3*exp(2)*exp(-7*x)
+
     See Also
     ========
     sympy.ntheory.factor_.factorint
diff --git a/sympy/polys/rationaltools.py b/sympy/polys/rationaltools.py
index 29862c1d91dc..0c0376fee0e9 100644
--- a/sympy/polys/rationaltools.py
+++ b/sympy/polys/rationaltools.py
@@ -8,7 +8,7 @@
 from sympy.utilities import public
 
 @public
-def together(expr, deep=False):
+def together(expr, deep=False, fraction=True):
     """
     Denest and combine rational expressions using symbolic methods.
 
@@ -65,7 +65,7 @@ def _together(expr):
             if expr.is_Atom or (expr.is_Function and not deep):
                 return expr
             elif expr.is_Add:
-                return gcd_terms(list(map(_together, Add.make_args(expr))))
+                return gcd_terms(list(map(_together, Add.make_args(expr))), fraction=fraction)
             elif expr.is_Pow:
                 base = _together(expr.base)
 
diff --git a/sympy/polys/tests/test_polytools.py b/sympy/polys/tests/test_polytools.py
index 450f20788964..b44bcffb9e8c 100644
--- a/sympy/polys/tests/test_polytools.py
+++ b/sympy/polys/tests/test_polytools.py
@@ -2476,6 +2476,11 @@ def test_factor():
     assert factor(eq, x, deep=True) == (x + 3)*(x + 4)*(y**2 + 11*y + 30)
     assert factor(eq, y, deep=True) == (y + 5)*(y + 6)*(x**2 + 7*x + 12)
 
+    # fraction option
+    f = 5*x + 3*exp(2 - 7*x)
+    assert factor(f, deep=True) == factor(f, deep=True, fraction=True)
+    factor(f, deep=True, fraction=False) == 5*x + 3*exp(2)*exp(-7*x)
+
 
 def test_factor_large():
     f = (x**2 + 4*x + 4)**10000000*(x**2 + 1)*(x**2 + 2*x + 1)**1234567


Code review comments:
diff_hunk: @@ -2476,6 +2476,11 @@ def test_factor():
     assert factor(eq, x, deep=True) == (x + 3)*(x + 4)*(y**2 + 11*y + 30)
     assert factor(eq, y, deep=True) == (y + 5)*(y + 6)*(x**2 + 7*x + 12)
 
+    # fraction option
+    f = 5*x + 3*exp(2 - 7*x)
+    assert factor(f, deep=True) == factor(f, deep=True, fraction=True)
+    factor(f, deep=True, fraction=False) == 5*x + 3*exp(2)*exp(-7*x)
path: sympy/polys/tests/test_polytools.py
text: @user1:
`assert`?

@user2:
This should be part of code quality: a logical `a == b` should be flagged.

@author:
Agreed!