Repository: sympy/sympy
Pull request: Added a new test case for the issue #7724 in polys
<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### Added a new test case for the issue #7724 in polys
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

Fixes #7724

#### Brief description of what is fixed or changed
For the issue #7724 , I added a new test case.

<!-- 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 -->
NO ENTRY
<!-- END RELEASE NOTES -->

Problem statement:
roots should find the roots of x**4*I + x**2 + I
```
In [34]: roots(x**4*I + x**2 + I, x)
Out[34]: {}
```

surprisingly if we replace `I` with variable coefficients we get an answer.

```
In [35]: roots(x**4*a + x**2 + b, x)
Out[35]: 
{sqrt(2)*sqrt(-sqrt(-4*a*b + 1)/a - 1/a)/2: 1,
 sqrt(2)*sqrt(sqrt(-4*a*b + 1)/a - 1/a)/2: 1,
 -sqrt(2)*sqrt(sqrt(-4*a*b + 1)/a - 1/a)/2: 1,
 -sqrt(2)*sqrt(-sqrt(-4*a*b + 1)/a - 1/a)/2: 1}
```

Hints from issue discussion:
This works:

```
In [3]: Poly(I*x**4 - x**2 + I, x, domain='ZZ[I]')
Out[3]: Poly(I*x**4 - x**2 + I, x, domain='ZZ[I]')

In [4]: len(roots(_,x))
Out[4]: 4
```

I suppose `ZZ` is for the integer domain, because this fails:

```
In [43]: roots(x**2 + I*sqrt(2), x, domain='ZZ[I]')
---------------------------------------------------------------------------
.
. # Removed the call stack to make the comment short
.
CoercionFailed: can't convert sqrt(2)*I of type <class 'sympy.core.mul.Mul'> to ZZ[I]
```

`RR` fails complaining `sqrt(2)*I` isn't a Real number

```
In [44]: roots(x**2 + I*sqrt(2), x, domain='RR')
---------------------------------------------------------------------------
.
.
.
CoercionFailed: expected real number, got sqrt(2)*I
```

`RR[I]` fails with error "expecting a valid domain specification"

```
In [45]: roots(x**2 + I*sqrt(2), x, domain='RR[I]')
---------------------------------------------------------------------------
.
.
.
OptionError: expected a valid domain specification, got RR[I]
```

And `CC` returns the answer in float.

```
In [46]: roots(x**2 + I*sqrt(2), x, domain='CC')
Out[46]: {-1.18920711500272*sqrt(-I): 1, 1.18920711500272*sqrt(-I): 1}
```

The docstring for the `roots` function doesn't contain any example for domains and filters. Maybe @mattpap can help. 

I think you'll need domain=EX

> I suppose ZZ is for the integer domain

Yes.

> The docstring for the roots function doesn't contain any example

The docstring is clear enough for me.  The domain option is a filter to get roots from the specific domain.  In the example above I have used domain option for the Poly constructor.

@asmeurer what does `EX` stands for?

I think it stands for **arbitrary EXpressions**.

The domain parameter only specifies the domain of the coefficients of the polynomial, not the roots. Roots of a polynomial in the domain ZZ might be non-integers. 

See also #12345 and #12421
> example for domains and filters

The filters use the assumption system to filter the results, basically `return [i for i in full_solution if i.is_foo]`. So this is limited by the assumption system's ability to deduce these properties. For complicated expressions it might have problems and get a None.
The original example from this issue now works fine:
```julia
In [9]: roots(x**4*I + x**2 + I, x)                                                                                                            
Out[9]: 
⎧     __________         __________          ____________         ____________   ⎫
⎪    ╱ ⅈ   √5⋅ⅈ         ╱ ⅈ   √5⋅ⅈ          ╱   √5⋅ⅈ   ⅈ         ╱   √5⋅ⅈ   ⅈ    ⎪
⎨-  ╱  ─ + ──── : 1,   ╱  ─ + ──── : 1, -  ╱  - ──── + ─ : 1,   ╱  - ──── + ─ : 1⎬
⎪ ╲╱   2    2        ╲╱   2    2         ╲╱      2     2      ╲╱      2     2    ⎪
⎩                                                                                ⎭
```

I think this can probably be closed now.

Commit message: added a new test case for the issue #7724 in test_polyroots.py

Patch to review:
diff --git a/sympy/polys/tests/test_polyroots.py b/sympy/polys/tests/test_polyroots.py
index 838dc76ef12d..0e24710dc2e5 100644
--- a/sympy/polys/tests/test_polyroots.py
+++ b/sympy/polys/tests/test_polyroots.py
@@ -73,6 +73,13 @@ def test_roots_quadratic():
         roots = roots_quadratic(f)
         assert roots == _nsort(roots)
 
+
+def test_issue_7724():
+    eq = Poly(x**4*I + x**2 + I, x)
+    roots_d = roots(eq)
+    assert len(roots_d) == 4
+
+
 def test_issue_8438():
     p = Poly([1, y, -2, -3], x).as_expr()
     roots = roots_cubic(Poly(p, x), x)


Code review comments:
diff_hunk: @@ -73,6 +73,13 @@ def test_roots_quadratic():
         roots = roots_quadratic(f)
         assert roots == _nsort(roots)
 
+
+def test_issue_7724():
+    eq = Poly(x**4*I + x**2 + I, x)
+    roots_d = roots(eq)
+    assert len(roots_d) == 4
path: sympy/polys/tests/test_polyroots.py
text: @user1:
I think it would be better to assert what the roots are then just how many there are