Remove generic is_power and root methods

[fingolfin]

These method are not useful in practice.

[codecov[bot]]

Codecov Report

Merging #1479 (2c25a15) into master (b5090f4) will increase coverage by 0.04%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1479      +/-   ##
==========================================
+ Coverage   84.76%   84.81%   +0.04%     
==========================================
  Files         110      110              
  Lines       29370    29354      -16     
==========================================
+ Hits        24895    24896       +1     
+ Misses       4475     4458      -17     

Files	Coverage Δ
src/generic/Misc/Rings.jl	100.00% <ø> (+51.61%)	:arrow_up:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[thofma]

I disagree. What is the downside of having the roots => factor fallback?

[fingolfin]

This code currently tries to implement is_power for an element a of an arbitrary ring R::MyRingType by creating a Generic.PolyRing over R, then creating a polynomial x^n-a, and asking for its roots.

For this to be useful, someone (probably whoever implemented R) would have to implement a roots method for Generic.PolyRing{MyRingType}, but not an is_power method. In general, implementing is_power seems to be much simpler than a general roots method.

The code is also never tested right now, and as result does not even work:

julia> Nemo.AbstractAlgebra.Generic.is_power(ZZ(4), 2)
ERROR: MethodError: no method matching AbstractAlgebra.Generic.PolyRing(::ZZRing)

or when trying to use it in Nemo:

julia> AbstractAlgebra.Generic.is_power(ZZ(4), 2)
ERROR: MethodError: no method matching is_power(::BigInt, ::Int64)

julia> AbstractAlgebra.Generic.is_power(GF(5)(2), 2)
ERROR: MethodError: no method matching AbstractAlgebra.Generic.PolyRing(::AbstractAlgebra.GFField{Int64})

But if you think it might some day be useful for someone, we can keep it, but perhaps then at least PolyRing(R) should be changed to polynomial_ring(R) to fix that and error and also so that it might at least work also over rings which have a dedicated polynomial ring type (so e.g. QQRing and QQPolyRing). But that's of course not enough... with this patch:

diff --git a/src/generic/Misc/Rings.jl b/src/generic/Misc/Rings.jl
index 35a79fdd6..21d612a68 100644
--- a/src/generic/Misc/Rings.jl
+++ b/src/generic/Misc/Rings.jl
@@ -12,8 +12,7 @@ function is_power(a::RingElem, n::Int)
         return true, a
     end
     R = parent(a)
-    Rt = PolyRing(R)
-    x = gen(Rt)
+    Rt, x = polynomial_ring(R)
     r = roots(x^n - a)
     if length(r) == 0
         return false, a

I get this in AA:

julia> AbstractAlgebra.Generic.is_power(GF(5)(2), 2)
ERROR: function factor is not implemented for argument
AbstractAlgebra.Generic.Poly{AbstractAlgebra.GFElem{Int64}}: x^2 + 3

[fingolfin]

@fieker @thofma any further thoughts on this?

[thofma]

I will fix it