Bug: sympy.factor doesn't work for Poly !!!

[Pilipets]

Minimum reproduction example:

from sympy import symbols, Poly, factor
x = symbols('x')
print(factor(Poly(x ** 4 + 6 * x ** 3 + 4 * x ** 2 - 30 * x - 45, x)))

This outputs Poly(x**4 + 6*x**3 + 4*x**2 - 30*x - 45, x, domain='ZZ'), but I expect factorization into (x+3)(x+3)(x^2−5)

sympy version: 1.7.1
Python version: 3.7.8 (tags/v3.7.8:4b47a5b6ba, Jun 28 2020, 08:53:46) [MSC v.1916 64 bit (AMD64)]

[oscarbenjamin]

The factor function is for Expr rather than Poly so you can use as_expr to factor as Expr or factor_list to factor as Poly:

In [1]: p = Poly(x ** 4 + 6 * x ** 3 + 4 * x ** 2 - 30 * x - 45, x)

In [2]: factor(p.as_expr())
Out[2]: 
       2 ⎛ 2    ⎞
(x + 3) ⋅⎝x  - 5⎠

In [3]: p.factor_list()
Out[3]: (1, [(Poly(x + 3, x, domain='ZZ'), 2), (Poly(x**2 - 5, x, domain='ZZ'), 1)])

Previously factor did work with Poly but that was changed in 75bbeab from #18613.

It was not intentional that that would change factor. I'm not sure how it didn't get picked up but perhaps this is a clue:

In [2]: Poly(x**2 - 1) == (x - 1)*(x + 1)
Out[2]: True

That should return False. It gives True because the expression on the rhs is converted to Poly (which expands everything). I think that Poly should compare unequal to any Expr so that should be fixed.

For the OP issue I'm not sure that it should be expected for factor to work with Poly since it isn't Expr.

[Pilipets]

Thanks, shouldn't it be stated in the factor method docstring?

While instead, it has
"There two modes implemented: symbolic and formal. If f is not an
instance of :class:Poly and generators are not specified, then the
former mode is used. Otherwise, the formal mode is used."

From that, I get a clue that it can be used with Poly.

[oscarbenjamin]

Thanks, shouldn't it be stated in the factor method docstring?

Yes, I guess the doc should be updated.

[Pilipets]

Thanks for your quick response. Am I supposed to close this issue?

[oscarbenjamin]

The issue can stay open until the doc is updated

[oscarbenjamin]

Maybe it should be possible to pass Poly to factor. It seems this also affects sqf_part and maybe other functions. Personally I think it's a mistake to mix Poly and Expr like this but I guess it is convenient in some cases.

Having Poly compare equal to Expr certainly seems like a mistake to me though.