Skip to content

Commit

Permalink
Enable factor(::Rational) (#1918)
Browse files Browse the repository at this point in the history
Also enables `factor(ZZ, 28//15)` in Nemo, Hecke, Oscar. Alas, no tests
for this here as we have no `factor` methods for any integer types here
in AA.
  • Loading branch information
fingolfin authored Nov 29, 2024
1 parent 58aa07f commit f301392
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/Fields.jl
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function gcdx(x::T, y::T) where {T <: FieldElem}
end
end

function factor(x::FieldElem)
function factor(x::FieldElement)
@req !is_zero(x) "Element must be non-zero"
return Fac(x, Dict{typeof(x), Int}())
end
2 changes: 1 addition & 1 deletion src/generic/Misc/Poly.jl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function factor(R::Field, f::PolyRingElem)
return factor(f1)
end

function factor(R::Ring, f::FracElem)
function factor(R::Ring, f::Union{FracElem, Rational})
fn = factor(R(numerator(f)))
fd = factor(R(denominator(f)))
fn.unit = divexact(fn.unit, fd.unit)
Expand Down
7 changes: 7 additions & 0 deletions test/generic/Fraction-test.jl
Original file line number Diff line number Diff line change
Expand Up @@ -398,4 +398,11 @@ end
@test length(f) == 0
@test unit(f) == a
@test_throws ArgumentError factor(zero(F))

F = QQ
a = 28//15
f = factor(a)
@test length(f) == 0
@test unit(f) == a
@test_throws ArgumentError factor(zero(F))
end

0 comments on commit f301392

Please sign in to comment.