Skip to content

Make it easier to extend lcm/gcd/gcdx to custom types #34129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 20, 2020
Merged

Make it easier to extend lcm/gcd/gcdx to custom types #34129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3549cd1
Fix array-argument gcd for non-integers
sostock Jan 17, 2020
85eb20f
Make it easier to extend lcm/gcd/gcdx to custom types
sostock Dec 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions base/intfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,17 @@ end

gcd(a::Union{Integer,Rational}) = a
lcm(a::Union{Integer,Rational}) = a
gcd(a::Union{Integer,Rational}, b::Union{Integer,Rational}) = gcd(promote(a,b)...)
lcm(a::Union{Integer,Rational}, b::Union{Integer,Rational}) = lcm(promote(a,b)...)
gcd(a::Union{Integer,Rational}, b::Union{Integer,Rational}...) = gcd(a, gcd(b...))
lcm(a::Union{Integer,Rational}, b::Union{Integer,Rational}...) = lcm(a, lcm(b...))
gcd(a::Real, b::Real) = gcd(promote(a,b)...)
lcm(a::Real, b::Real) = lcm(promote(a,b)...)
gcd(a::Real, b::Real, c::Real...) = gcd(a, gcd(b, c...))
lcm(a::Real, b::Real, c::Real...) = lcm(a, lcm(b, c...))
gcd(a::T, b::T) where T<:Real = throw(MethodError(gcd, (a,b)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually throwing method errors tend to often be a code smell from my experience. What is the reason for it? Is it to avoid an infinite recursion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the reason is to avoid infinite recursion, since some Real types don’t implement gcd/lcm/gcdx (AbstractFloats, for example).

lcm(a::T, b::T) where T<:Real = throw(MethodError(lcm, (a,b)))

lcm(abc::AbstractArray{<:Union{Integer,Rational}}) = reduce(lcm, abc; init=one(eltype(abc)))
gcd(abc::AbstractArray{<:Real}) = reduce(gcd, abc; init=zero(eltype(abc)))
lcm(abc::AbstractArray{<:Real}) = reduce(lcm, abc; init=one(eltype(abc)))

function gcd(abc::AbstractArray{<:Union{Integer,Rational}})
function gcd(abc::AbstractArray{<:Integer})
a = zero(eltype(abc))
for b in abc
a = gcd(a,b)
Expand Down Expand Up @@ -146,7 +149,8 @@ function gcdx(a::T, b::T) where T<:Integer
end
a < 0 ? (-a, -s0, -t0) : (a, s0, t0)
end
gcdx(a::Union{Integer,Rational}, b::Union{Integer,Rational}) = gcdx(promote(a,b)...)
gcdx(a::Real, b::Real) = gcdx(promote(a,b)...)
gcdx(a::T, b::T) where T<:Real = throw(MethodError(gcdx, (a,b)))

# multiplicative inverse of n mod m, error if none

Expand Down
11 changes: 11 additions & 0 deletions test/intfuncs.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,17 @@ end
@test gcdx(5, -12) == (1, 5, 2)
@test gcdx(-25, -4) == (1, -1, 6)
end
@testset "gcd/lcm/gcdx for custom types" begin
struct MyRational <: Real
val::Rational{Int}
end
Base.promote_rule(::Type{MyRational}, T::Type{<:Real}) = promote_type(Rational{Int}, T)
(T::Type{<:Real})(x::MyRational) = T(x.val)

@test gcd(MyRational(2//3), 3) == gcd(2//3, 3) == gcd(Real[MyRational(2//3), 3])
@test lcm(MyRational(2//3), 3) == lcm(2//3, 3) == lcm(Real[MyRational(2//3), 3])
@test gcdx(MyRational(2//3), 3) == gcdx(2//3, 3)
end
@testset "invmod" begin
@test invmod(6, 31) === 26
@test invmod(-1, 3) === 2
Expand Down
2 changes: 2 additions & 0 deletions test/rational.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,5 +399,7 @@ end
@test lcm(1//3, 1) == 1//1
@test lcm(3//1, 1//0) == 3//1
@test lcm(0//1, 1//0) == 0//1

@test gcd([5, 2, 1//2]) == 1//2
end