Skip to content
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

Jump to bottom

Add method to rationalize Rational #43427

Merged
merged 17 commits into from
Jun 14, 2023
Merged

Add method to rationalize Rational #43427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
06691dd
Add Method to Rationalize Rational
tantheta01 Dec 15, 2021
9491c16
Merge branch 'master' of https://github.com/tantheta01/julia
tantheta01 Dec 15, 2021
2c536ea
Merge branch 'JuliaLang:master' into master
tantheta01 Dec 16, 2021
32497af
Update base/rational.jl
tantheta01 Dec 17, 2021
25edfef
Update base/rational.jl
tantheta01 Dec 17, 2021
c06b479
Update base/rational.jl
tantheta01 Dec 19, 2021
109661c
Merge branch 'JuliaLang:master' into master
tantheta01 Dec 19, 2021
850e0de
Rationalize rational, ignore tolerance
tantheta01 Dec 19, 2021
a737e1a
Merge branch 'master' into master
oscardssmith Jan 31, 2023
ab264e6
Merge branch 'master' into master
oscardssmith Jun 8, 2023
4b70c62
more tests
oscardssmith Jun 8, 2023
797f27b
fix overflow and unbound type param.
oscardssmith Jun 9, 2023
af59db9
fix tests
oscardssmith Jun 9, 2023
0f8368d
typo
oscardssmith Jun 12, 2023
e680308
fix test
oscardssmith Jun 12, 2023
ff0e53d
Update rational.jl
oscardssmith Jun 13, 2023
d12ccd0
Merge branch 'master' into master
oscardssmith Jun 13, 2023
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
21 changes: 16 additions & 5 deletions base/rational.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,11 @@ julia> typeof(numerator(a))
BigInt
```
"""
function rationalize(::Type{T}, x::AbstractFloat, tol::Real) where T<:Integer
function rationalize(::Type{T}, x::Union{AbstractFloat, Rational}, tol::Real) where T<:Integer
if tol < 0
throw(ArgumentError("negative tolerance $tol"))
end

Copy link
Member

Choose a reason for hiding this comment

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

Better to avoid spurious whitespace changes like this.

T<:Unsigned && x < 0 && __throw_negate_unsigned()
isnan(x) && return T(x)//one(T)
isinf(x) && return unsafe_rational(x < 0 ? -one(T) : one(T), zero(T))
Expand All @@ -188,7 +189,6 @@ function rationalize(::Type{T}, x::AbstractFloat, tol::Real) where T<:Integer
a = trunc(x)
r = x-a
y = one(x)

tolx = oftype(x, tol)
nt, t, tt = tolx, zero(tolx), tolx
ia = np = nq = zero(T)
Expand Down Expand Up @@ -233,10 +233,21 @@ function rationalize(::Type{T}, x::AbstractFloat, tol::Real) where T<:Integer
return p // q
end
end
rationalize(::Type{T}, x::AbstractFloat; tol::Real = eps(x)) where {T<:Integer} = rationalize(T, x, tol)::Rational{T}
rationalize(::Type{T}, x::AbstractFloat; tol::Real = eps(x)) where {T<:Integer} = rationalize(T, x, tol)
rationalize(x::AbstractFloat; kvs...) = rationalize(Int, x; kvs...)
rationalize(::Type{T}, x::Complex; kvs...) where {T<:Integer} = Complex(rationalize(T, x.re, kvs...)::Rational{T}, rationalize(T, x.im, kvs...)::Rational{T})
Copy link
Member

Choose a reason for hiding this comment

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

Oh wow, this looks like it was actually a bug. Thanks for catching that. Would be good to add a test for it.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe something like:

diff --git a/test/rational.jl b/test/rational.jl
--- test/rational.jl
+++ test/rational.jl
@@ -638,5 +638,11 @@
     @test rationalize(float(pi)im) == 0//1 + 165707065//52746197*im
     @test rationalize(Int8, float(pi)im) == 0//1 + 22//7*im
     @test rationalize(1.192 + 2.233im) == 149//125 + 2233//1000*im
     @test rationalize(Int8, 1.192 + 2.233im) == 118//99 + 67//30*im
+
+    # test: rationalize(x::Complex; kvs...)
+    precise_next = 7205759403792795//72057594037927936
+    @assert Float64(precise_next) == nextfloat(0.1)
+    @test rationalize(nextfloat(0.1) * im; tol=0) == precise_next * im
+    @test rationalize(0.1im; tol=eps(0.1)) == rationalize(0.1im)
 end

rationalize(x::Complex; kvs...) = Complex(rationalize(Int, x.re, kvs...), rationalize(Int, x.im, kvs...))
rationalize(::Type{T}, x::Complex; kvs...) where {T<:Integer} = Complex(rationalize(T, x.re; kvs...), rationalize(T, x.im; kvs...))
rationalize(x::Complex; kvs...) = Complex(rationalize(Int, x.re; kvs...), rationalize(Int, x.im; kvs...))
rationalize(::Type{T}, x::Rational; tol::Real = 0) where {T<:Integer} = rationalize(T, x, tol)
rationalize(x::Rational; kvs...) = x
rationalize(x::Integer; kvs...) = Rational(x)
function rationalize(::Type{T}, x::Integer; kvs...) where {T<:Integer}
if Base.hastypemax(T) # BigInt doesn't
x < typemin(T) && return unsafe_rational(-one(T), zero(T))
x > typemax(T) && return unsafe_rational(one(T), zero(T))
end
return Rational{T}(x)
end


"""
numerator(x)
Expand Down
12 changes: 12 additions & 0 deletions test/rational.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ using Test

@test @inferred(rationalize(Int, 3.0, 0.0)) === 3//1
@test @inferred(rationalize(Int, 3.0, 0)) === 3//1
@test @inferred(rationalize(Int, 33//100; tol=0.1)) === 1//3 # because tol
@test @inferred(rationalize(Int, 3; tol=0.0)) === 3//1
@test @inferred(rationalize(Int8, 1000//333)) === Rational{Int8}(3//1)
@test @inferred(rationalize(Int8, 1000//3)) === Rational{Int8}(1//0)
@test @inferred(rationalize(Int8, 1000)) === Rational{Int8}(1//0)
@test_throws OverflowError rationalize(UInt, -2.0)
@test_throws ArgumentError rationalize(Int, big(3.0), -1.)
# issue 26823
Expand Down Expand Up @@ -727,3 +732,10 @@ end
@test rationalize(1.192 + 2.233im) == 149//125 + 2233//1000*im
@test rationalize(Int8, 1.192 + 2.233im) == 118//99 + 67//30*im
end
@testset "rationalize(Complex) with tol" begin
# test: rationalize(x::Complex; kvs...)
precise_next = 7205759403792795//72057594037927936
@assert Float64(precise_next) == nextfloat(0.1)
@test rationalize(nextfloat(0.1) * im; tol=0) == precise_next * im
@test rationalize(0.1im; tol=eps(0.1)) == rationalize(0.1im)
end